client.mk edits x 2

RESOLVED FIXED in mozilla7

Status

()

Core
Build Config
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: joey, Assigned: joey)

Tracking

unspecified
mozilla7
Points:
---
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

6 years ago
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.
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

6 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

6 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
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

6 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

6 years ago
Created attachment 541692 [details] [diff] [review]
Removed MAKE:=gmake and stray quoting problem
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

6 years ago
Created attachment 541747 [details] [diff] [review]
original patch + nit [ remove 2011 copyright line ]
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
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla7
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
Last Resolved: 6 years ago6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.