Closed Bug 475654 Opened 12 years ago Closed 11 years ago

Standalone SpiderMonkey should be able to build on Solaris with Sun-supplied NSPR

Categories

(Firefox Build System :: General, defect)

All
Solaris
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
mozilla1.9.2a1

People

(Reporter: wes, Assigned: wes)

References

Details

Attachments

(1 file, 4 obsolete files)

Sun supplies an NSPR shared library and header files with Solaris, yet omits nspr-config. (packages: SUNWpr SUNWprd) 

This means that users building standalone Spidermonkey with --enable-threads cannot use the --with-system-nspr option, unless they have previous built their own NSPR, or use an NSPR distributed with something else.  (Or they go digging through their filesystem and figure out the right incantation for --with-nspr-libs and --with-nspr-cflags)

This patch modifies configure.in to provide three extra "forward symbols" during OS-specific setup to provide hints to nspr.m4 as to what to do when --with-system-nspr is specified but nspr-config cannot be found. These symbols are:

NO_NSPR_CONFIG_SYSTEM_CPPFLAGS     # C preprocessor flags to find headers
NO_NSPR_CONFIG_SYSTEM_LDFLAGS      # ld flags to link in NSPR and dependent libs
NO_NSPR_CONFIG_SYSTEM_VERSION      # NSPR Version number: major, minor, micro

In circumstances where nspr.m4 cannot find nspr-config but needs it, it tests for a non-empty NO_NSPR_CONFIG_SYSTEM_VERSION as a last-ditch way to configure the library. 

Testing the NSPR version number has been generalized so that we now always check NSPR_VERSION_STRING instead of running nspr-config. NSPR_VERSION_STRING will either be ${NO_NSPR_CONFIG_SYSTEM_VERSION}, or `nspr-config --version`.

The symbols provided by configure.in are done by testing OS packages with Solaris' package management tools, and querying them for the version of NSPR.  I have verified that the build fails with version 4.6.3 ("cannot find suitable NSPR") and succeeds with version 4.7 on Solaris 10/sparc (patch 119213-17).

Reviewer, I would appreciate extra keen eyeballs on nspr.m4 where the version testing logic has been modified. It will affect all --enable-threads builds regardless of platform.

This patch depends on, but does not include, the patch from bug 475064. It was tested on a GCC build patched with 475393, but should work fine with Sun Studio. It was generated based on tracemonkey-73bfcdaa1a51. (Jan 23 2009).

This bug is similar to a previous bug, 413164 (need SunOS5.10.mk), however the NSPR-related configuration in there never made it into the autoconf build environment.  Additionally, this patch selects the version of NSPR which is managed by the Solaris package management system (and patched along with NSS and JSS), rather than the one which is "along for the ride" in /usr/sfw.
Attachment #359196 - Flags: review?(jim)
Note: I just noticed that the diff for this patch includes the change for 475393, simply by virtue of being less than 8 lines away. So, please patch with 475393 first in order for all hunks to apply.
Previous patch defective due to word-wrap error on line 60.
Attachment #359196 - Attachment is obsolete: true
Attachment #359206 - Flags: review?(jim)
Attachment #359196 - Flags: review?(jim)
That clause in js/src/configure.in calls pkgconfig, which is a Solaris host environment tool, when all we know is that Solaris is our target; how is that going to play out in a cross-compiling situation?  Would it be better to put the call to pkgconfig inside an 'if solaris host' conditional?
Comment on attachment 359206 [details] [diff] [review]
Patch to allow system NSPR without nspr-config, and also to use Sun-supplied NSPR

On IRC, Wes suggested draping a conditional around the call to pkgconfig.
Attachment #359206 - Flags: review?(jim) → review-
- Rebased to tracemonkey-tip from a couple days ago
- Adjusted to not use local Solaris pkginfo to check NSPR version when test -z "$CROSS_COMPILE".
Assignee: nobody → wes
Attachment #359206 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #364905 - Flags: review?(jim)
Comment on attachment 364905 [details] [diff] [review]
Patch to allow system NSPR without nspr-config, and also to use Sun-supplied NSPR

Nit: in nspr.m4, that first conditional could become a nice neat if-else chain: if $NSPR_CONFIG=yes; else if NO_NSPR_CONFIG_SYSTEM_VERSION is set; else no_nspr.

Nit: NO_NSPR_CONFIG_SYSTEM_CPPFLAGS should be renamed NO_NSPR_CONFIG_SYSTEM_CFLAGS, since it's used to provide a value for NSPR_CFLAGS.
Attachment #364905 - Flags: review?(jim) → review+
What would you think of adding this to the patch to nspr.m4?
Addresses style nits (including nspr.m4 test refactor) and adds doc fix verbatim.

Instead of checking $NSPR_CONFIG = "yes", I chose $NSPR_CONFIG != "no" because I wasn't convinced that no "no" always implied "yes".
Attachment #364905 - Attachment is obsolete: true
Attachment #364930 - Attachment is obsolete: true
Attachment #365025 - Flags: review?(jim)
Comment on attachment 365025 [details] [diff] [review]
Allow system NSPR w/o nspr-config, use Sun-supplied NSPR, doc fix

Looks great --- thanks!
Attachment #365025 - Flags: review?(jim) → review+
Is there more needed to get this landed?  Or do we just need someone to do it?
Keywords: checkin-needed
Adapted to fix build/autoconf/nspr.m4 as well as js/src/build/autoconf/nspr.m4, and landed:
http://hg.mozilla.org/mozilla-central/rev/c3030aee31ab
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Where else do you want this landed?
Flags: in-testsuite-
Target Milestone: --- → mozilla1.9.2a1
Version: unspecified → Trunk
(In reply to comment #12)
> Where else do you want this landed?

It's all good --- thanks.
Keywords: checkin-needed
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.