Standalone SpiderMonkey should copy headers on `make install` instead of symlinking

RESOLVED FIXED in Firefox -esr52

Status

()

Core
JavaScript Engine
RESOLVED FIXED
3 months ago
3 months ago

People

(Reporter: ptomato, Assigned: ptomato)

Tracking

(Blocks: 2 bugs)

52 Branch
mozilla56
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox-esr52 fixed, firefox56 fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

3 months ago
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 building SpiderMonkey standalone, `make install` symlinks the header files into the install directories. This is incompatible with the way that GNOME and its various downstream packagers will need to build SpiderMonkey. Instead, the header files should be copied.


Actual results:

Installed files in $prefix/include/mozjs-52/ are all symlinks.


Expected results:

Installed files in $prefix/include/mozjs-52/ should not be symlinks.
(Assignee)

Comment 1

3 months ago
Created attachment 8884715 [details] [diff] [review]
build: Copy headers on install instead of symlinking

Here's a patch that works for me.
(Assignee)

Updated

3 months ago
Blocks: 1379541
(Assignee)

Updated

3 months ago
Blocks: 837921
Comment on attachment 8884715 [details] [diff] [review]
build: Copy headers on install instead of symlinking

Review of attachment 8884715 [details] [diff] [review]:
-----------------------------------------------------------------

Does this seem reasonable, or is something else required?
Attachment #8884715 - Flags: feedback?(mh+mozilla)
Comment on attachment 8884715 [details] [diff] [review]
build: Copy headers on install instead of symlinking

Review of attachment 8884715 [details] [diff] [review]:
-----------------------------------------------------------------

This changes more than advertized. As in the other bug, what you want is to change the behavior of make install, without touching what the normal build does.
Attachment #8884715 - Flags: feedback?(mh+mozilla) → feedback-
(Assignee)

Comment 4

3 months ago
OK, found it - make install already does specify --no-symlinks to process_install_manifest.py, but the InstallManifestNoSymlinks class doesn't do the right thing for add_pattern_symlink().
(Assignee)

Comment 5

3 months ago
Created attachment 8889015 [details] [diff] [review]
Fix InstallManifestNoSymlinks to not install symlinks
(Assignee)

Updated

3 months ago
Attachment #8884715 - Attachment is obsolete: true
(Assignee)

Updated

3 months ago
Attachment #8889015 - Flags: review?(mh+mozilla)

Updated

3 months ago
Attachment #8889015 - Flags: review?(mh+mozilla) → review+
Try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=cb488da4179f2b27f87cbdce66f1fdb872868f88

Updated

3 months ago
Assignee: nobody → philip.chimento
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
(Assignee)

Updated

3 months ago
Keywords: checkin-needed

Comment 7

3 months ago
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/bbac1c606f3d
Fix InstallManifestNoSymlinks to not install symlinks. r=glandium
Keywords: checkin-needed

Comment 8

3 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/bbac1c606f3d
Status: ASSIGNED → RESOLVED
Last Resolved: 3 months ago
status-firefox56: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
(Assignee)

Comment 9

3 months ago
Comment on attachment 8889015 [details] [diff] [review]
Fix InstallManifestNoSymlinks to not install symlinks

[Approval Request Comment]
If this is not a sec:{high,crit} bug, please state case for ESR consideration: Fixes a build problem that affects downstream packagers (Linux distributions) of SpiderMonkey ESR releases.
User impact if declined: None to Firefox users, but the SpiderMonkey standalone release will be less usable for embedders, and downstream packagers will have to maintain this patch themselves
Fix Landed on Version: 56
Risk to taking this patch (and alternatives if risky): None, it is my understanding that SpiderMonkey's `make install` is not used in Firefox's build process, only by SpiderMonkey embedders
String or UUID changes made by this patch: None 

See https://wiki.mozilla.org/Release_Management/ESR_Landing_Process for more info.
Attachment #8889015 - Flags: approval-mozilla-esr52?
status-firefox-esr52: --- → affected
Comment on attachment 8889015 [details] [diff] [review]
Fix InstallManifestNoSymlinks to not install symlinks

Needed for spidermonkey. Let's uplift it to ESR52.3.
Attachment #8889015 - Flags: approval-mozilla-esr52? → approval-mozilla-esr52+

Comment 11

3 months ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-esr52/rev/fd052b08fc3e
status-firefox-esr52: affected → fixed
You need to log in before you can comment on or make changes to this bug.