Closed Bug 1384308 Opened 7 years ago Closed 7 years ago

Standalone SpiderMonkey should not install shared library twice

Categories

(Core :: JavaScript Engine, defect)

52 Branch
defect
Not set
normal

Tracking

()

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

People

(Reporter: ptomato, Assigned: ptomato)

References

Details

Attachments

(1 file)

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_12_6) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/59.0.3071.115 Safari/537.36

Steps to reproduce:

Looks like the libmozjs-52 shared library is getting installed twice when building standalone SpiderMonkey on macOS. It is getting installed once as $(SHARED_LIBRARY) and once as $(IMPORT_LIBRARY). It's my understanding that $(IMPORT_LIBRARY) should be empty on non-Windows systems, but it is instead set to the same value as $(SHARED_LIBRARY).


Actual results:

Relevant log from "make install":

../../config/nsinstall -t libmozjs-52.dylib /Users/ptomato/jhbuild/install/_jhbuild/root-mozjs52/Users/ptomato/jhbuild/install/lib
../../config/nsinstall -t libmozjs-52.dylib /Users/ptomato/jhbuild/install/_jhbuild/root-mozjs52/Users/ptomato/jhbuild/install/lib


Expected results:

The library should only have been installed once.
Blocks: 1379536
Would the correct approach be to change the makefile code generation at http://searchfox.org/mozilla-central/source/python/mozbuild/mozbuild/backend/recursivemake.py#1232 to emit, say, "ifeq ($(OS_ARCH),WINNT)" around the IMPORT_LIBRARY definition and otherwise leave the variable blank?

It seems like BaseLibrary.import_name is used for a lot of other things so I don't really want to mess with it.
Flags: needinfo?(mh+mozilla)
I think a simpler approach would be to skip the import lib install when it's the same as the shared library, in js/src/build/Makefile.in.
Flags: needinfo?(mh+mozilla)
When backporting to esr52, the change should be applied to
js/src/Makefile.in instead of js/src/build/Makefile.in.
Attachment #8902522 - Flags: review?(mh+mozilla)
Attachment #8902522 - Flags: review?(mh+mozilla) → review+
Assignee: nobody → philip.chimento
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/ec954e9a0e55
Don't install import library if same as shared library. r=glandium
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/ec954e9a0e55
Status: UNCONFIRMED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
Comment on attachment 8902522 [details] [diff] [review]
Don't install import library if same as shared library

[Approval Request Comment]
If this is not a sec:{high,crit} bug, please state case for ESR consideration: This is a build fix for standalone SpiderMonkey on macOS.
User impact if declined: None to Firefox users, but embedders will need to patch the ESR tarball for compiling on macOS.
Fix Landed on Version: 57
Risk to taking this patch (and alternatives if risky): Very low risk, only affects "make install"
String or UUID changes made by this patch: None

See https://wiki.mozilla.org/Release_Management/ESR_Landing_Process for more info.
Attachment #8902522 - Flags: approval-mozilla-esr52?
Comment on attachment 8902522 [details] [diff] [review]
Don't install import library if same as shared library

packaging fix for spidermonkey, esr52.4+
Attachment #8902522 - Flags: approval-mozilla-esr52? → approval-mozilla-esr52+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: