Closed
Bug 1379539
Opened 7 years ago
Closed 7 years ago
Standalone SpiderMonkey should not require NSPR in pkg-config file when compiled with --enable-posix-nspr-emulation
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
RESOLVED
FIXED
mozilla57
People
(Reporter: ptomato, Assigned: ptomato)
References
(Blocks 1 open bug)
Details
Attachments
(1 file, 2 obsolete files)
1.94 KB,
patch
|
glandium
:
review+
jcristau
:
approval-mozilla-esr52+
|
Details | Diff | Splinter Review |
User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_12_5) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/58.0.3029.110 Safari/537.36 Steps to reproduce: When configuring standalone SpiderMonkey with --enable-posix-nspr-emulation, it should have no dependency on NSPR. However, it requires NSPR in its pkg-config file. Actual results: Building any programs that use pkg-config to link to SpiderMonkey will fail if NSPR is not installed. Expected results: NSPR should not be necessary.
Assignee | ||
Comment 1•7 years ago
|
||
Here's a patch that worked for me.
Assignee | ||
Updated•7 years ago
|
Blocks: sm-embedding
Updated•7 years ago
|
Attachment #8884718 -
Flags: review?(mh+mozilla)
Comment 2•7 years ago
|
||
Comment on attachment 8884718 [details] [diff] [review] 0006-build-Remove-unnecessary-NSPR-dependency.patch Review of attachment 8884718 [details] [diff] [review]: ----------------------------------------------------------------- You may want to add a ifdef in js.pc.in so that the Requires.private line doesn't appear at all in that case.
Attachment #8884718 -
Flags: review?(mh+mozilla)
Assignee | ||
Comment 3•7 years ago
|
||
The pkg-config file should not require NSPR if compiled with --enable-posix-nspr-emulation.
Assignee | ||
Comment 4•7 years ago
|
||
There's no ifdef in configure substitution, but this ought to do the trick.
Assignee | ||
Updated•7 years ago
|
Attachment #8884718 -
Attachment is obsolete: true
Assignee | ||
Updated•7 years ago
|
Attachment #8889004 -
Flags: review?(mh+mozilla)
Comment 5•7 years ago
|
||
Comment on attachment 8889004 [details] [diff] [review] Remove unnecessary NSPR dependency Review of attachment 8889004 [details] [diff] [review]: ----------------------------------------------------------------- ::: build/autoconf/nspr-build.m4 @@ +192,5 @@ > AC_MSG_ERROR([system NSPR does not support PR_STATIC_ASSERT])) > CFLAGS=$_SAVE_CFLAGS > + PKGCONF_REQUIRES_PRIVATE="Requires.private: $NSPR_PKGCONF_CHECK" > +elif test -n "$JS_POSIX_NSPR"; then > + PKGCONF_REQUIRES_PRIVATE= You're removing the line in the !MOZ_SYSTEM_NSPR case. ::: js/src/js.pc.in @@ +6,4 @@ > Name: SpiderMonkey @MOZILLA_VERSION@ > Description: The Mozilla library for JavaScript > Version: @MOZILLA_VERSION@ > +@PKGCONF_REQUIRES_PRIVATE@ Note this file is in js/src/build/ since bug 1262241. Please create patches against mozilla-central.
Attachment #8889004 -
Flags: review?(mh+mozilla)
Assignee | ||
Comment 6•7 years ago
|
||
Note that for backporting to esr52, js/src/js.pc.in has to be patched instead of js/src/build/js.pc.in.
Assignee | ||
Updated•7 years ago
|
Attachment #8889004 -
Attachment is obsolete: true
Assignee | ||
Updated•7 years ago
|
Attachment #8890070 -
Flags: review?(mh+mozilla)
Updated•7 years ago
|
Attachment #8890070 -
Flags: review?(mh+mozilla) → review+
Assignee | ||
Comment 7•7 years ago
|
||
Try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=2184676e9db86043144606242f5417dd975da199&selectedJob=125785514 Looks like the Windows failure is a known intermittent build issue, bug 1318172, so I'm assuming this is OK.
Keywords: checkin-needed
Updated•7 years ago
|
Assignee: nobody → philip.chimento
Pushed by ryanvm@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/8fae85aac9b9 Remove unnecessary NSPR dependency. r=glandium
Keywords: checkin-needed
Assignee | ||
Comment 9•7 years ago
|
||
Comment on attachment 8890070 [details] [diff] [review] Remove unnecessary NSPR dependency [Approval Request Comment] If this is not a sec:{high,crit} bug, please state case for ESR consideration: Fixes a dependency issue for SpiderMonkey embedders that would otherwise cause them to have an unnecessary dependency when compiling and packaging SpiderMonkey. User impact if declined: None to Firefox users, but Linux distributions will have to patch out the dependency Fix Landed on Version: 57 Risk to taking this patch (and alternatives if risky): Negligible risk, as to my knowledge the pkg-config file isn't used in the Firefox build process. String or UUID changes made by this patch: None See https://wiki.mozilla.org/Release_Management/ESR_Landing_Process for more info.
Attachment #8890070 -
Flags: approval-mozilla-esr52?
Assignee | ||
Comment 10•7 years ago
|
||
(Note that for backporting to esr52, js/src/js.pc.in has to be patched instead of js/src/build/js.pc.in.)
Comment 11•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/8fae85aac9b9
Status: UNCONFIRMED → RESOLVED
Closed: 7 years ago
status-firefox57:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
Comment 12•7 years ago
|
||
Comment on attachment 8890070 [details] [diff] [review] Remove unnecessary NSPR dependency build tweak for tier3 builds, esr52.4+
Attachment #8890070 -
Flags: approval-mozilla-esr52? → approval-mozilla-esr52+
Comment 13•7 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-esr52/rev/5d95b2833b30
status-firefox-esr52:
--- → fixed
You need to log in
before you can comment on or make changes to this bug.
Description
•