Summary: | Hardcoding of /usr/bin/gcc in perl build scripts | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Fred Ollinger <Fred_Ollinger> | ||||||||||
Component: | New Bugs | Assignee: | Nobody <webkit-unassigned> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | commit-queue, darin, ddkilzer, dglazkov, eric, gustavo, lucas.de.marchi, webkit.review.bot, xan.lopez | ||||||||||
Priority: | P2 | ||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||
Hardware: | PC | ||||||||||||
OS: | OS X 10.5 | ||||||||||||
Attachments: |
|
Description
Fred Ollinger
2010-03-01 15:39:01 PST
Comment on attachment 49760 [details] Patch to change /usr/bin/gcc to $ENV{CC} in perl build scripts. Rejecting patch 49760 from review queue. Fred_Ollinger@seektech.com does not have reviewer permissions according to http://trac.webkit.org/browser/trunk/WebKitTools/Scripts/webkitpy/committers.py. - If you do not have reviewer rights please read http://webkit.org/coding/contributing.html for instructions on how to use bugzilla flags. - If you have reviewer rights please correct the error in WebKitTools/Scripts/webkitpy/committers.py by adding yourself to the file (no review needed). Due to bug 30084 the commit-queue will require a restart after your change. Please contact eseidel@chromium.org to request a commit-queue restart. After restart the commit-queue will correctly respect your reviewer rights. Comment on attachment 49760 [details] Patch to change /usr/bin/gcc to $ENV{CC} in perl build scripts. Rejecting patch 49760 from commit-queue. Fred_Ollinger@seektech.com does not have committer permissions according to http://trac.webkit.org/browser/trunk/WebKitTools/Scripts/webkitpy/committers.py. - If you do not have committer rights please read http://webkit.org/coding/contributing.html for instructions on how to use bugzilla flags. - If you have committer rights please correct the error in WebKitTools/Scripts/webkitpy/committers.py by adding yourself to the file (no review needed). Due to bug 30084 the commit-queue will require a restart after your change. Please contact eseidel@chromium.org to request a commit-queue restart. After restart the commit-queue will correctly respect your committer rights. I assume you may have meant to set these r? Please see http://webkit.org/coding/contributing.html Created attachment 57186 [details]
remove hardcoded refs to gcc's path
Updated version of this old patch, fulfilling requirements for patches sent to WebKit.
Attachment 57186 [details] did not build on gtk: Build output: http://webkit-commit-queue.appspot.com/results/2521049 Comment on attachment 57186 [details]
remove hardcoded refs to gcc's path
This will not work on my computers, for example. I don't think that most OS X users have an environment variable named "CC" set in their environment. Perhaps you could use the environment variable if it exists, and otherwise use the hardcoded path.
Attachment 57186 [details] did not build on chromium: Build output: http://webkit-commit-queue.appspot.com/results/2491048 (In reply to comment #6) > (From update of attachment 57186 [details]) > This will not work on my computers, for example. I don't think that most OS X users have an environment variable named "CC" set in their environment. Perhaps you could use the environment variable if it exists, and otherwise use the hardcoded path. Why are we hard-coding the path? Isn't it better to use the CC environment var and fall-back to the default path? Created attachment 57197 [details]
remove hardcoded refs to gcc's path
Is this good for MacOS?
Comment on attachment 57197 [details]
remove hardcoded refs to gcc's path
This patch doesn't do this "Perhaps you could use the environment variable if it exists, and otherwise use the hardcoded path" as suggested in the comments.
Also, the variable name change is superfluous (and without that change, less lines of code are changed).
(In reply to comment #10) > (From update of attachment 57197 [details]) > This patch doesn't do this "Perhaps you could use the environment variable if it exists, and otherwise use the hardcoded path" as suggested in the comments. > I know. Did you see my comment above? What's the point in using the complete path when the PATH environment variable can help? > Also, the variable name change is superfluous (and without that change, less lines of code are changed). More lines of code are changed, but the variable name will make sense since with this patch it will not necessarily contain the path. I could do what was suggested, but I'm still convinced it's not the right thing to do. Created attachment 57409 [details]
Patch
Here is the patch as suggested Darin. I'd rather take the previous patch I sent since it seems to be a better idea. But, anyway, here is the other patch.
We definitely don't want to do anything that relies on PATH, so I do not think the previous patch was a better idea. Comment on attachment 57409 [details] Patch Clearing flags on attachment: 57409 Committed r60413: <http://trac.webkit.org/changeset/60413> All reviewed patches have been landed. Closing bug. *** Bug 24454 has been marked as a duplicate of this bug. *** |