Last Comment Bug 666908 - client.mk edits x 2
: client.mk edits x 2
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Build Config (show other bugs)
: unspecified
: All All
: -- normal (vote)
: mozilla7
Assigned To: Joey Armstrong [:joey]
:
: Gregory Szorc [:gps]
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-06-24 05:19 PDT by Joey Armstrong [:joey]
Modified: 2011-07-01 19:36 PDT (History)
3 users (show)
khuey: in‑testsuite-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Removed MAKE:=gmake and stray quoting problem (2.52 KB, patch)
2011-06-24 08:44 PDT, Joey Armstrong [:joey]
khuey: review+
Details | Diff | Splinter Review
original patch + nit [ remove 2011 copyright line ] (2.25 KB, patch)
2011-06-24 12:04 PDT, Joey Armstrong [:joey]
no flags Details | Diff | Splinter Review

Description Joey Armstrong [:joey] 2011-06-24 05:19:35 PDT
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 Ted Mielczarek [:ted.mielczarek] 2011-06-24 07:10:44 PDT
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 :)
Comment 2 Joey Armstrong [:joey] 2011-06-24 07:25:30 PDT
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.
Comment 3 Joey Armstrong [:joey] 2011-06-24 07:27:31 PDT
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 Ted Mielczarek [:ted.mielczarek] 2011-06-24 07:35:37 PDT
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.
Comment 5 Joey Armstrong [:joey] 2011-06-24 08:24:57 PDT
>> 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.
Comment 6 Joey Armstrong [:joey] 2011-06-24 08:44:12 PDT
Created attachment 541692 [details] [diff] [review]
Removed MAKE:=gmake and stray quoting problem
Comment 7 Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary) 2011-06-24 11:24:02 PDT
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.
Comment 8 Joey Armstrong [:joey] 2011-06-24 12:04:32 PDT
Created attachment 541747 [details] [diff] [review]
original patch + nit [ remove 2011 copyright line ]
Comment 9 Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary) 2011-06-30 09:19:41 PDT
http://hg.mozilla.org/projects/build-system/rev/a5870dd2ad7c
Comment 10 Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary) 2011-06-30 18:20:43 PDT
http://hg.mozilla.org/mozilla-central/rev/a5870dd2ad7c
Comment 11 Matt Brubeck (:mbrubeck) 2011-07-01 12:04:03 PDT
Backed out during investigation of Android browser-chrome test failures:
http://hg.mozilla.org/mozilla-central/rev/00bb08972e46
Comment 12 Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary) 2011-07-01 19:36:00 PDT
Relanded
http://hg.mozilla.org/mozilla-central/rev/38e14b78ac76

Note You need to log in before you can comment on or make changes to this bug.