Bug 35551

Summary: Hardcoding of /usr/bin/gcc in perl build scripts
Product: WebKit Reporter: Fred Ollinger <Fred_Ollinger>
Component: New BugsAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, darin, ddkilzer, dglazkov, eric, gns, lucas.de.marchi, webkit.review.bot, xan.lopez
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: OS X 10.5   
Attachments:
Description Flags
Patch to change /usr/bin/gcc to $ENV{CC} in perl build scripts.
commit-queue: review-, commit-queue: commit-queue-
remove hardcoded refs to gcc's path
darin: review-, darin: commit-queue-
remove hardcoded refs to gcc's path
none
Patch none

Description Fred Ollinger 2010-03-01 15:39:01 PST
Created attachment 49760 [details]
Patch to change /usr/bin/gcc to $ENV{CC} in perl build scripts.

Recently when trying to cross the nightly build: r55354, I noticed that several Perl scripts hardcoded the path to /usr/bin/gcc. Obviously, this completely breaks a cross compile as a cross compiler should not be installed in /usr/bin/gcc; that's where the native compiler is NOT the cross compiler. 

There are four perl build scripts that hard code /usr/bin/gcc as the default compiler. This is true even if one has chosen to cross-compile. This should effectively break cross-compiling as there is no way to tell ./configure that gcc is located in an alternate location. This bug will also break use of another compiler other than gcc. It will also break using a different version of gcc if it is not installed in the default location of /usr/bin/gcc.

To fix this, I first isolated the areas where /usr/bin/gcc was called from Perl. Then I changed references from "/usr/bin/gcc" to $ENV{CC} which will allow a user to use any version of gcc that they wish. Patch is attached.


1. bindings/scripts/CodeGeneratorObjC.pm   

2. bindings/scripts/IDLParser.pm        

3. css/make-css-file-arrays.pl

4. dom/make_names.pl

Fred Ollinger
Embedded Software Engineer
Seektech, Incorporated
http://www.seektechinc.com/
fred_ollinger@seektech.com
Comment 1 WebKit Commit Bot 2010-03-01 15:49:31 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 2 WebKit Commit Bot 2010-03-01 16:05:58 PST
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.
Comment 3 Eric Seidel (no email) 2010-03-01 17:33:54 PST
I assume you may have meant to set these r?  Please see http://webkit.org/coding/contributing.html
Comment 4 Lucas De Marchi 2010-05-26 19:19:52 PDT
Created attachment 57186 [details]
remove hardcoded refs to gcc's path

Updated version of this old patch, fulfilling requirements for patches sent to WebKit.
Comment 5 WebKit Review Bot 2010-05-26 20:34:10 PDT
Attachment 57186 [details] did not build on gtk:
Build output: http://webkit-commit-queue.appspot.com/results/2521049
Comment 6 Darin Adler 2010-05-26 20:39:50 PDT
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.
Comment 7 WebKit Review Bot 2010-05-26 21:07:16 PDT
Attachment 57186 [details] did not build on chromium:
Build output: http://webkit-commit-queue.appspot.com/results/2491048
Comment 8 Lucas De Marchi 2010-05-26 22:23:06 PDT
(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?
Comment 9 Lucas De Marchi 2010-05-26 23:24:30 PDT
Created attachment 57197 [details]
remove hardcoded refs to gcc's path

Is this good for MacOS?
Comment 10 David Levin 2010-05-27 12:55:02 PDT
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).
Comment 11 Lucas De Marchi 2010-05-27 17:20:59 PDT
(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.
Comment 12 Lucas De Marchi 2010-05-29 06:29:24 PDT
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.
Comment 13 Darin Adler 2010-05-29 20:25:31 PDT
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 14 WebKit Commit Bot 2010-05-29 20:49:23 PDT
Comment on attachment 57409 [details]
Patch

Clearing flags on attachment: 57409

Committed r60413: <http://trac.webkit.org/changeset/60413>
Comment 15 WebKit Commit Bot 2010-05-29 20:49:30 PDT
All reviewed patches have been landed.  Closing bug.
Comment 16 Lucas De Marchi 2010-06-02 17:21:11 PDT
*** Bug 24454 has been marked as a duplicate of this bug. ***