Closed
Bug 666908
Opened 13 years ago
Closed 13 years ago
client.mk edits x 2
Categories
(Firefox Build System :: General, defect)
Firefox Build System
General
Tracking
(Not tracked)
RESOLVED
FIXED
mozilla7
People
(Reporter: joey, Assigned: joey)
Details
Attachments
(1 file, 1 obsolete file)
2.25 KB,
patch
|
Details | Diff | Splinter Review |
1) cosmetic: escape the quote on line 98 or change to "Cannot find". The stray quote will cause problems for editor colorization: AUTOCONF=$(error Couldn't find autoconf 2.13) 2) MAKE := gmake Most variant/native make programs have fallen by the wayside over time so how about removing this explicit assignment and simply rely on whatever make binary client.mk is being processed by ? The hardcoded assignment will cause problems when using a modified make binary, sub-make calls are hijacked and explicitly redirected to the system gmake command w/o having to include silly arguments: foo -f client.mk MAKE=foo. Use of pymake in the build system would also be affected by this problem.
Comment 1•13 years ago
|
||
The gmake assignment is inside ifndef MAKE, so it's probably not harmful, but it's also probably not useful. Looking at CVS archeology: http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/client.mk&rev=1.396 http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/client.mk&rev=1.19 that line has been there since nearly the beginning of time: http://bonsai.mozilla.org/cvsview2.cgi?diff_mode=context&whitespace_mode=show&root=/cvsroot&subdir=mozilla&command=DIFF_FRAMESET&file=client.mk&rev2=1.12&rev1=1.11 1998-11-13 :)
Assignee | ||
Comment 2•13 years ago
|
||
argh overlooked that ifdef... I'll keep digging because 'gmake' is creeping into the build somewhere along the way a level or two down from the top call.
Assignee | ||
Comment 3•13 years ago
|
||
Is there any significance for shebang path being embedded within the makefiles or is this simply odd syntax to enable editor colorization ? ./js/src/xpconnect/sample/Makefile.in:#!gmake ./js/jsd/Makefile.in:#!gmake ./tools/jprof/Makefile.in:#! gmake ./tools/jprof/stub/Makefile.in:#! gmake ./tools/leaky/Makefile.in:#! gmake ./tools/leaky/Makefile.linux:#! gmake
Comment 4•13 years ago
|
||
No idea, probably bizarre ancient history. I can't imagine that actually does anything useful, unless someone wants to chmod +x Makefile; ./Makefile, which seems insane. There is a little bit of configure silliness to find "gmake" because we explicitly build NSS with GNU make when building with pymake, because NSS' build system is separate and a PITA to patch.
Assignee | ||
Comment 5•13 years ago
|
||
>> chmod +x Makefile; ./Makefile, which seems insane. We have inners: ~40 shebang makefiles could be launched as a command ============================================================================== 39726016 4 -rwxr-xr-x ./security/nss/cmd/libpkix/pkix_pl/Makefile 39726021 4 -rwxr-xr-x ./security/nss/cmd/libpkix/pkix_pl/system/Makefile 39981001 4 -rwxr-xr-x ./security/nss/cmd/pk11mode/Makefile 39981034 4 -rwxr-xr-x ./security/nss/cmd/fipstest/Makefile >> little bit of configure silliness because we explicitly build NSS It may be happening in a few sandbox directories, configure scripts beneath js/ are playing similar games.
Assignee | ||
Comment 6•13 years ago
|
||
Attachment #541692 -
Flags: review?(khuey)
Comment on attachment 541692 [details] [diff] [review] Removed MAKE:=gmake and stray quoting problem >@@ -11,23 +13,25 @@ >+# Modified by Mozilla Corporation Copyright (C) 2011 Don't add this. Also, for future reference, copyright on works produced by Mozilla Corporation employees accrues to the Mozilla *Foundation*, not the Corporation. r=me.
Attachment #541692 -
Flags: review?(khuey) → review+
Assignee | ||
Comment 8•13 years ago
|
||
Attachment #541692 -
Attachment is obsolete: true
http://hg.mozilla.org/projects/build-system/rev/a5870dd2ad7c
Whiteboard: fixed-in-bs
http://hg.mozilla.org/mozilla-central/rev/a5870dd2ad7c
Assignee: nobody → joey
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla7
Comment 11•13 years ago
|
||
Backed out during investigation of Android browser-chrome test failures: http://hg.mozilla.org/mozilla-central/rev/00bb08972e46
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Whiteboard: fixed-in-bs
Target Milestone: mozilla7 → ---
Relanded http://hg.mozilla.org/mozilla-central/rev/38e14b78ac76
Flags: in-testsuite-
Target Milestone: --- → mozilla7
Status: REOPENED → RESOLVED
Closed: 13 years ago → 13 years ago
Resolution: --- → FIXED
Updated•6 years ago
|
Product: Core → Firefox Build System
You need to log in
before you can comment on or make changes to this bug.
Description
•