477358 - Kill NS_USE_NATIVE

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Zack Weinberg (:zwol)]

The Makefile variable NS_USE_NATIVE is labeled as a "temporary hack" -- from 1999.  As best I can tell it's redundant with GNU_CC and it might even not be being used at all anymore - there are a number of files that test it but I can't tell if they're actually part of the build system.

Comment 1

[Serge Gautherie (:sgautherie)]

I could be the one doing the "removal",
but what there actually is to do seems to need to be confirmed first.

Comment 2

[Zack Weinberg (:zwol)]

Here's what I mean by "can't tell if they're actually part of the build system":

$ grep -r NS_USE_NATIVE *
build/unix/nspr_my_config.mk.in:NS_USE_NATIVE=@MOZ_NSPRENV_NS_USE_NATIVE@
config/autoconf.mk.in:# NS_USE_NATIVE to the build system -ramiro.
config/autoconf.mk.in:NS_USE_NATIVE = @NS_USE_NATIVE@
configure:       NS_USE_NATIVE=1
configure:s%@NS_USE_NATIVE@%$NS_USE_NATIVE%g
configure.in:       NS_USE_NATIVE=1
configure.in:AC_SUBST(NS_USE_NATIVE)
js/src/config/autoconf.mk.in:# NS_USE_NATIVE to the build system -ramiro.
js/src/config/autoconf.mk.in:NS_USE_NATIVE = @NS_USE_NATIVE@
js/src/configure.in:       NS_USE_NATIVE=1
js/src/configure.in:AC_SUBST(NS_USE_NATIVE)
js/src/configure:       NS_USE_NATIVE=1
js/src/configure:s%@NS_USE_NATIVE@%$NS_USE_NATIVE%g
js/src/Makefile.ref:#NS_USE_NATIVE = 1
js/src/ref-config/SunOS5.5.mk:ifndef NS_USE_NATIVE
js/src/ref-config/SunOS5.4.mk:ifdef NS_USE_NATIVE
js/src/ref-config/HP-UXB.11.31.mk:ifdef NS_USE_NATIVE
js/src/ref-config/SunOS5.6.mk:ifndef NS_USE_NATIVE
js/src/ref-config/IRIX.mk:ifndef NS_USE_NATIVE
js/src/ref-config/HP-UXB.11.00.mk:ifdef NS_USE_NATIVE
js/src/ref-config/OSF1V5.0.mk:ifndef NS_USE_NATIVE
js/src/ref-config/OSF1V4.0.mk:ifndef NS_USE_NATIVE
js/src/config.mk:NS_USE_NATIVE = 1
security/coreconf/NCR3.0.mk:NS_USE_NATIVE = 1
security/coreconf/NCR3.0.mk:ifdef NS_USE_NATIVE
security/coreconf/SunOS5.mk:	ifndef NS_USE_NATIVE
security/manager/Makefile.in:DEFAULT_GMAKE_FLAGS += NS_USE_GCC=1 NS_USE_NATIVE=
security/manager/Makefile.in:DEFAULT_GMAKE_FLAGS += NS_USE_GCC= NS_USE_NATIVE=1

so there's a lot of setting and passing around of the variable, but the only actual *uses* are these:

js/src/ref-config/SunOS5.5.mk:ifndef NS_USE_NATIVE
js/src/ref-config/SunOS5.4.mk:ifdef NS_USE_NATIVE
js/src/ref-config/HP-UXB.11.31.mk:ifdef NS_USE_NATIVE
js/src/ref-config/SunOS5.6.mk:ifndef NS_USE_NATIVE
js/src/ref-config/IRIX.mk:ifndef NS_USE_NATIVE
js/src/ref-config/HP-UXB.11.00.mk:ifdef NS_USE_NATIVE
js/src/ref-config/OSF1V5.0.mk:ifndef NS_USE_NATIVE
js/src/ref-config/OSF1V4.0.mk:ifndef NS_USE_NATIVE
security/coreconf/NCR3.0.mk:ifdef NS_USE_NATIVE
security/coreconf/SunOS5.mk:	ifndef NS_USE_NATIVE

and it's not clear to me whether any of these files are still relevant to the modern build system.

Comment 3

[Matheus Kerschbaum]

Attachment: patch

Comment 4

[Zack Weinberg (:zwol)]

Comment on attachment 556636 [details] [diff] [review]
patch

Thanks for tackling this, but you don't appear to have completely removed it -- see comment 2 for more places to look.  You're not done till "grep -rw NS_USE_NATIVE *" at the top level of the source tree comes back empty.

Also, judging by the logic in security/manager/Makefile.in, you should get rid of NS_USE_GCC at the same time.

Comment 5

[Matheus Kerschbaum]

Most of the files in comment #2 don't exist anymore, and security/coreconf is a part of NSS so we can't easily change it. Also NS_USE_GCC seems to be used in several places (http://mxr.mozilla.org/mozilla-central/search?string=NS_USE_GCC) so I'm not sure it can be removed.

Comment 6

[Zack Weinberg (:zwol)]

(In reply to Matheus Kerschbaum from comment #5)
> Most of the files in comment #2 don't exist anymore

Good to know.

> and security/coreconf is a part of NSS so we can't easily change it.

I'm inclined to think we should at least *try*.

> Also NS_USE_GCC seems to be
> used in several places
> (http://mxr.mozilla.org/mozilla-central/search?string=NS_USE_GCC) so I'm not
> sure it can be removed.

There's lots of hits, but the only place that *sets* it is security/manager/Makefile.in, so it should be pretty easy to get rid of.  However, it looks like all the uses are in NSPR and NSS, so I retract the statement that it should be done at the same time -- you want to do one patch for nsprpub/ and one patch for security/, independent of your existing patch.  I should think they can all go in this bug, though.

Comment 7

[Zack Weinberg (:zwol)]

ccing NSPR and NSS maintainers.

Comment 8

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Comment on attachment 556636 [details] [diff] [review]
patch

Review of attachment 556636 [details] [diff] [review]:
-----------------------------------------------------------------

Sorry for the delay.  This looks fine to me.  The NSS parts should be done in a separate bug anyways, and review from the NSS peers will be required.

Comment 9

[Wan-Teh Chang]

Comment on attachment 556636 [details] [diff] [review]
patch

r=wtc.

This MXR query shows it is safe to remove NS_USE_NATIVE and
this patch removes it cleanly from mozilla, excluding NSS:
http://mxr.mozilla.org/mozilla-central/search?string=NS_USE_NATIVE

In addition, the removal of NS_USE_NATIVE from NSS's
security/coreconf/SunOS5.mk can be done independent of
this patch because security/manager/Makefile.in passes
NS_USE_GCC to NSS on the make command line, which overrides
the value of NS_USE_GCC in NSS's makefiles.

Comment 10

[Matheus Kerschbaum]

Attachment: patch for checkin

Updated to tip of build-system branch.

Comment 11

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/5f46b0998038

Comment 12

[Marco Bonardo [:mak]]

https://hg.mozilla.org/mozilla-central/rev/5f46b0998038