Closed Bug 1143421 Opened 5 years ago Closed 3 years ago

SpiderMonkey-31: install include copies instead of symlinks

Categories

(Core :: JavaScript Engine, defect)

31 Branch
x86_64
Linux
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla52
Tracking Status
firefox52 --- fixed

People

(Reporter: tolga.dalman, Assigned: yan12125)

References

Details

Attachments

(1 file, 3 obsolete files)

User Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/41.0.2272.76 Safari/537.36

Steps to reproduce:

Invoke make install in js/src


Actual results:

Symlinks instead of file copies are placed into the destination directory.


Expected results:

Update dist_include manifest with prefix "2" instead of "1".

See https://bugs.gentoo.org/show_bug.cgi?id=540876
Duplicate of this bug: 1296289
Attached patch bug1143421.patch (obsolete) — Splinter Review
Comment on attachment 8782838 [details] [diff] [review]
bug1143421.patch

Hello Mike could you review this? It's a Spidermonkey bug while relevant codes are in /python/mozbuild/
Attachment #8782838 - Flags: review?(mh+mozilla)
Comment on attachment 8782838 [details] [diff] [review]
bug1143421.patch

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

::: python/mozbuild/mozbuild/action/process_install_manifest.py
@@ +59,4 @@
>          manifest |= InstallManifest(path=path)
>  
>      copier = FileCopier()
> +    manifest.populate_registry(copier, defines_override=defines, no_symlinks=no_symlinks)

Instead of increasing the API surface for InstallManifest, I'd create a subclass here with an overridden add_symlink that just calls add_copy, then use that subclass instead of InstallManifest when no_symlinks is true.
Attachment #8782838 - Flags: review?(mh+mozilla)
Attached patch bug1143421_ver2.patch (obsolete) — Splinter Review
Sorry for being late. I just messed up my build system and I pulled from HG again. Is this patch OK?
Attachment #8783846 - Flags: review?(mh+mozilla)
Attachment #8782838 - Attachment is obsolete: true
Comment on attachment 8783846 [details] [diff] [review]
bug1143421_ver2.patch

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

Please update the patch as per the comment below, as well as adding some information for checkin:
https://developer.mozilla.org/en-US/docs/Mercurial/Using_Mercurial#How_can_I_generate_a_patch_for_somebody_else_to_check-in_for_me.3F

::: python/mozbuild/mozpack/manifests.py
@@ +400,5 @@
> +
> +
> +class InstallManifestNoSymlinks(InstallManifest):
> +    """Like InstallManifest, but files are never installed as symbolic links.
> +    Instead, they are always copied. This is useful for distribution packagers.

The last sentence is not necessary.
Attachment #8783846 - Flags: review?(mh+mozilla) → review+
Attached patch bug1143421_ver3.patch (obsolete) — Splinter Review
Updated. Thanks for all kind instructions!
Attachment #8783846 - Attachment is obsolete: true
Re-generate the patch with `hg export qtip`. Sorry for the previous noise.
Attachment #8784718 - Attachment is obsolete: true
Hi Mike, is it OK to move on?
Flags: needinfo?(mh+mozilla)
Comment on attachment 8784721 [details] [diff] [review]
bug1143421_ver4.patch

Carrying over r+
Flags: needinfo?(mh+mozilla)
Attachment #8784721 - Flags: review+
Assignee: nobody → yan12125
Keywords: checkin-needed
Pushed by cbook@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/dd4cbc33bad4
install file copies instead of symlinks for Spidermonkey. r=glandium
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/dd4cbc33bad4
Status: UNCONFIRMED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
You need to log in before you can comment on or make changes to this bug.