Last Comment Bug 477358 - Kill NS_USE_NATIVE
: Kill NS_USE_NATIVE
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Build Config (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla10
Assigned To: Matheus Kerschbaum
:
: Gregory Szorc [:gps]
Mentors:
http://bonsai.mozilla.org/cvsblame.cg...
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2009-02-06 21:40 PST by Zack Weinberg (:zwol)
Modified: 2011-10-13 07:22 PDT (History)
11 users (show)
matjk7: in‑testsuite-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch (5.41 KB, patch)
2011-08-29 12:00 PDT, Matheus Kerschbaum
khuey: review+
wtc: review+
zackw: feedback-
Details | Diff | Splinter Review
patch for checkin (5.40 KB, patch)
2011-10-10 17:05 PDT, Matheus Kerschbaum
matjk7: review+
Details | Diff | Splinter Review

Description Zack Weinberg (:zwol) 2009-02-06 21:40:27 PST
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) 2009-07-25 21:13:02 PDT
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) 2009-07-26 21:30:17 PDT
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 2011-08-29 12:00:25 PDT
Created attachment 556636 [details] [diff] [review]
patch
Comment 4 Zack Weinberg (:zwol) 2011-08-29 15:03:00 PDT
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 2011-08-29 15:17:07 PDT
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) 2011-08-29 15:52:21 PDT
(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) 2011-08-29 15:53:43 PDT
ccing NSPR and NSS maintainers.
Comment 8 Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary) 2011-10-10 04:43:52 PDT
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 2011-10-10 11:20:33 PDT
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 2011-10-10 17:05:46 PDT
Created attachment 566083 [details] [diff] [review]
patch for checkin

Updated to tip of build-system branch.
Comment 12 Marco Bonardo [::mak] 2011-10-13 07:22:00 PDT
https://hg.mozilla.org/mozilla-central/rev/5f46b0998038

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