The default bug view has changed. See this FAQ.

Kill NS_USE_NATIVE

RESOLVED FIXED in mozilla10

Status

()

Core
Build Config
RESOLVED FIXED
8 years ago
6 years ago

People

(Reporter: zwol, Assigned: Matheus Kerschbaum)

Tracking

Trunk
mozilla10
Points:
---
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(1 attachment, 1 obsolete attachment)

5.40 KB, patch
Matheus Kerschbaum
: review+
Details | Diff | Splinter Review
(Reporter)

Description

8 years ago
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.
(Reporter)

Updated

8 years ago
Assignee: ted.mielczarek → nobody
I could be the one doing the "removal",
but what there actually is to do seems to need to be confirmed first.
(Reporter)

Comment 2

8 years ago
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.
(Assignee)

Updated

6 years ago
Assignee: nobody → matjk7
Status: NEW → ASSIGNED
Flags: in-testsuite-
(Assignee)

Comment 3

6 years ago
Created attachment 556636 [details] [diff] [review]
patch
Attachment #556636 - Flags: review?(khuey)
(Reporter)

Comment 4

6 years ago
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.
Attachment #556636 - Flags: feedback-
(Assignee)

Comment 5

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

Comment 6

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

Comment 7

6 years ago
ccing NSPR and NSS maintainers.
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.
Attachment #556636 - Flags: review?(khuey) → review+

Comment 9

6 years ago
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.
Attachment #556636 - Flags: review+
(Assignee)

Comment 10

6 years ago
Created attachment 566083 [details] [diff] [review]
patch for checkin

Updated to tip of build-system branch.
Attachment #556636 - Attachment is obsolete: true
Attachment #566083 - Flags: review+
(Assignee)

Updated

6 years ago
Keywords: checkin-needed
https://hg.mozilla.org/integration/mozilla-inbound/rev/5f46b0998038
Keywords: checkin-needed
OS: Linux → All
Hardware: x86 → All
Target Milestone: --- → mozilla10
https://hg.mozilla.org/mozilla-central/rev/5f46b0998038
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.