Closed Bug 1379539 Opened 3 years ago Closed 2 years ago

Standalone SpiderMonkey should not require NSPR in pkg-config file when compiled with --enable-posix-nspr-emulation

Categories

(Core :: JavaScript Engine, defect)

52 Branch
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla57
Tracking Status
firefox-esr52 --- fixed
firefox57 --- fixed

People

(Reporter: ptomato, Assigned: ptomato)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 2 obsolete files)

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.
Here's a patch that worked for me.
Blocks: 1379541
Blocks: sm.embedding
Attachment #8884718 - Flags: review?(mh+mozilla)
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)
The pkg-config file should not require NSPR if compiled with
--enable-posix-nspr-emulation.
There's no ifdef in configure substitution, but this ought to do the trick.
Attachment #8884718 - Attachment is obsolete: true
Attachment #8889004 - Flags: review?(mh+mozilla)
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)
Note that for backporting to esr52, js/src/js.pc.in has to be patched
instead of js/src/build/js.pc.in.
Attachment #8889004 - Attachment is obsolete: true
Attachment #8890070 - Flags: review?(mh+mozilla)
Attachment #8890070 - Flags: review?(mh+mozilla) → review+
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
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
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?
(Note that for backporting to esr52, js/src/js.pc.in has to be patched instead of js/src/build/js.pc.in.)
https://hg.mozilla.org/mozilla-central/rev/8fae85aac9b9
Status: UNCONFIRMED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
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+
You need to log in before you can comment on or make changes to this bug.