spidermonkey: update to 102 and fix build under macOS #179822

catap · 2022-07-01T17:02:37Z

Description of changes

This PR adds spidermonkey_102 which can be used on every unix which includes macOS. It also fix build of old version under macOS.

Thus, it suppress / consumes / closes #172376 and fixes #91415

And it adds tests for all packages :)

Things done

SuperSandro2000 · 2022-07-01T18:06:00Z

@ofborg eval

pkgs/development/interpreters/spidermonkey/adds-Wno-deprecated-declarations-icu.patch

pkgs/development/interpreters/spidermonkey/102.nix

catap · 2022-07-04T15:43:01Z

@SuperSandro2000 may I ask you to re-review this one also? Thanks!

Some motivation: it should allow to install nixops to macOS! :)

jtojnar · 2022-07-04T16:09:48Z

It would be nice to split the changes into independent commits, for example:

spidermonkey_*: Factor out into a common expression
spidermonkey_*: Fix build under macOS
spidermonkey_102: init at 102.0

That will make it easier to review.

catap · 2022-07-04T16:29:21Z

@jtojnar I've done it in different way:

added 102 with macOS support
added macOS support for 91 and 78;
refactored it.

I may push on this way if it makes things easy to review.

jtojnar · 2022-07-04T18:12:58Z

Having (2) separate from the other changes is the most important. But having the unification first would be the best since it minimizes the number of files needed to compare against each other.

catap · 2022-07-04T18:15:32Z

@jtojnar ok, let me try do it. It shouldn't be something complicated.

catap · 2022-07-04T18:51:44Z

@jtojnar here it is.

I've split it to a bit more steps to make things as clear as I can.

jtojnar

Thanks, this is now much easier to review.

pkgs/development/interpreters/spidermonkey/common.nix

jtojnar · 2022-07-04T20:20:34Z

pkgs/development/interpreters/spidermonkey/common.nix

+    libobjc
+    libiconv


Do these really go here rather than buildInputs?

@jtojnar yes, it is linked against libiconv.dylib on macOS:

√ result % otool -L lib/libmozjs-102.dylib lib/libmozjs-102.dylib: /nix/store/9ld6s0lcdjw9cp26zf4qa0j24m80rwd5-spidermonkey-102.0/lib/libmozjs-102.dylib (compatibility version 1.0.0, current version 1.0.0) /usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1238.60.2) /nix/store/dpfzpp8c7z0fdxxvin21pk5gs135wn39-Libsystem-1238.60.2/lib/libresolv.9.dylib (compatibility version 1.0.0, current version 1.0.0) /nix/store/k56igcml51xccxxakgvj8lai6f7fz05q-zlib-1.2.12/lib/libz.dylib (compatibility version 1.0.0, current version 1.2.12) /nix/store/q4c16zw3kimqdvxcc06njrv49bq68hpf-libiconv-50/lib/libiconv.dylib (compatibility version 7.0.0, current version 7.0.0) /nix/store/nbwbggjf37pxmishd2sqqbswv6f0l3cb-libcxx-11.1.0/lib/libc++.1.0.dylib (compatibility version 1.0.0, current version 1.0.0) @rpath/CoreFoundation.framework/Versions/A/CoreFoundation (compatibility version 150.0.0, current version 1454.90.0) √ result % otool -L bin/js102 bin/js102: /usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1238.60.2) /nix/store/dpfzpp8c7z0fdxxvin21pk5gs135wn39-Libsystem-1238.60.2/lib/libresolv.9.dylib (compatibility version 1.0.0, current version 1.0.0) /nix/store/k56igcml51xccxxakgvj8lai6f7fz05q-zlib-1.2.12/lib/libz.dylib (compatibility version 1.0.0, current version 1.2.12) /nix/store/qgzlayc4acky2pyp56dh6p1bm00d315g-readline-6.3p08/lib/libreadline.6.dylib (compatibility version 6.0.0, current version 6.3.0) /nix/store/q4c16zw3kimqdvxcc06njrv49bq68hpf-libiconv-50/lib/libiconv.dylib (compatibility version 7.0.0, current version 7.0.0) /nix/store/nbwbggjf37pxmishd2sqqbswv6f0l3cb-libcxx-11.1.0/lib/libc++.1.0.dylib (compatibility version 1.0.0, current version 1.0.0) @rpath/CoreFoundation.framework/Versions/A/CoreFoundation (compatibility version 150.0.0, current version 1454.90.0) √ result %

If it is linked against a library it goes into buildInputs.

I think this is the last open thread.

Suggested change

libobjc

libiconv

libobjc

libiconv is already in buildInputs. Can we move libobjc, too?

pkgs/development/interpreters/spidermonkey/common.nix

catap · 2022-07-04T21:17:43Z

@jtojnar seems that I've addressed all your comments. Or at least I hope so.

To make review easy I haven't rebased anything, and keep all changes as two dedicated commits.

nixos-discourse · 2022-08-01T20:07:39Z

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-already-reviewed/2617/567

SuperSandro2000

otherwise LGTM

pkgs/development/interpreters/spidermonkey/common.nix

SuperSandro2000 · 2022-08-01T20:49:56Z

pkgs/development/interpreters/spidermonkey/common.nix

+    libobjc
+    libiconv


If it is linked against a library it goes into buildInputs.

pkgs/development/interpreters/spidermonkey/common.nix

catap · 2022-08-02T22:40:43Z

@SuperSandro2000 may I ask you to re-review this one?

wegank · 2022-08-02T22:53:24Z

@catap This comment has yet to be solved.

wegank · 2022-08-08T20:37:43Z

Just saw that GNOME 43 will depend on SpiderMonkey 102!

catap · 2022-08-08T20:39:31Z

@wegank thanks for the ping. I'll take a look on this on couple of hours.

jtojnar · 2022-08-09T13:39:58Z

pkgs/development/interpreters/spidermonkey/common.nix

+  patches = lib.optionals (lib.versionAtLeast version "102") [
+    # use pkg-config at all systems
+    ./always-check-for-pkg-config.patch
+    ./allow-system-s-nspr-and-icu-on-bootstrapped-sysroot.patch


Suggested change

./allow-system-s-nspr-and-icu-on-bootstrapped-sysroot.patch

./allow-system-s-nspr-and-icu-on-bootstrapped-sysroot.patch

# Patches required by GJS

# https://discourse.gnome.org/t/gnome-43-to-depend-on-spidermonkey-102/10658

# Install ProfilingCategoryList.h

(fetchpatch {

url = "https://hg.mozilla.org/releases/mozilla-esr102/raw-rev/33147b91e42b79f4c6dd3ec11cce96746018407a";

sha256 = "sha256-xJFJZMYJ6P11HQDZbr48GFgybpAeVcu3oLIFEyyMjBI=";

})

# Fix embeder build

(fetchpatch {

url = "https://hg.mozilla.org/releases/mozilla-esr102/raw-rev/1fa20fb474f5d149cc32d98df169dee5e6e6861b";

sha256 = "sha256-eCisKjNxy9SLr9KoEE2UB26BflUknnR7PIvnpezsZeA=";

})

jtojnar · 2022-08-09T13:44:45Z

pkgs/development/interpreters/spidermonkey/102.nix

@@ -0,0 +1,4 @@
+import ./common.nix {
+  version = "102.0";


102.1.0 may have been released according to https://discourse.gnome.org/t/gnome-43-to-depend-on-spidermonkey-102/10658.

jtojnar · 2022-08-10T14:51:32Z

Since this is blocking #182618, I cleaned up it a bit, addressed the remaining review comments and reverted some pointless changes (e.g. exporting configureScript): #185928

ofborg bot added the 6.topic: darwin Running or building packages on Darwin label Jul 1, 2022

catap force-pushed the spidermonkey branch from 3a1efbc to 9ab3e7a Compare July 1, 2022 17:50

ofborg bot added the 8.has: package (new) This PR adds a new package label Jul 1, 2022

ofborg bot requested review from abbradar and lostnet July 1, 2022 18:14

ofborg bot added 11.by: package-maintainer This PR was created by the maintainer of the package it changes 10.rebuild-darwin: 11-100 10.rebuild-linux: 501+ 10.rebuild-linux: 2501-5000 labels Jul 1, 2022

catap force-pushed the spidermonkey branch from 9ab3e7a to de3ccab Compare July 1, 2022 18:38

SuperSandro2000 reviewed Jul 1, 2022

View reviewed changes

pkgs/development/interpreters/spidermonkey/adds-Wno-deprecated-declarations-icu.patch Outdated Show resolved Hide resolved

pkgs/development/interpreters/spidermonkey/102.nix Outdated Show resolved Hide resolved

pkgs/development/interpreters/spidermonkey/102.nix Outdated Show resolved Hide resolved

catap force-pushed the spidermonkey branch from de3ccab to 85769da Compare July 1, 2022 21:05

catap requested a review from SuperSandro2000 July 1, 2022 21:18

catap force-pushed the spidermonkey branch 2 times, most recently from ea521dd to e505c9c Compare July 1, 2022 22:52

lostnet mentioned this pull request Jul 2, 2022

spidermonkey_91: remove platform restriction #172376

Closed

13 tasks

catap force-pushed the spidermonkey branch from e505c9c to 3a9d9a7 Compare July 4, 2022 18:14

catap force-pushed the spidermonkey branch from 3a9d9a7 to 1f64219 Compare July 4, 2022 18:51

catap force-pushed the spidermonkey branch 2 times, most recently from 5f799ce to e8cc360 Compare July 4, 2022 19:40

jtojnar reviewed Jul 4, 2022

View reviewed changes

catap force-pushed the spidermonkey branch from a55c72a to 325e0c7 Compare July 4, 2022 20:50

ofborg bot added 10.rebuild-linux: 501-1000 and removed 10.rebuild-darwin: 11-100 10.rebuild-linux: 2501-5000 labels Aug 1, 2022

SuperSandro2000 reviewed Aug 1, 2022

View reviewed changes

catap added 8 commits August 1, 2022 23:09

spidermonkey_91: factor out into a common expression

993b3dd

spidermonkey_78: factor out into a common expression

593bd36

spidermonkey_*: add tests

2fbcec7

spidermonkey_102: init at 102.0

0fa32a8

spidermonkey_*: add myself as a maintainer

5065c1e

spidermonkey_*: remove useless dependency

b0a1e72

spidermonkey_*: fix build under macOS

d74460a

spidermonkey_*: follow guideline for expression ordering

49694ab

catap force-pushed the spidermonkey branch 2 times, most recently from 7451816 to e8c7d9d Compare August 1, 2022 21:22

spidermonkey_*: follow review remarks

78c990c

catap force-pushed the spidermonkey branch from e8c7d9d to 78c990c Compare August 1, 2022 21:31

catap requested a review from SuperSandro2000 August 1, 2022 21:54

wegank approved these changes Aug 2, 2022

View reviewed changes

jtojnar reviewed Aug 9, 2022

View reviewed changes

jtojnar self-assigned this Aug 10, 2022

jtojnar mentioned this pull request Aug 10, 2022

spidermonkey_102: init at 102.1.0 #185928

Merged

13 tasks

catap closed this Aug 10, 2022

catap deleted the spidermonkey branch August 10, 2022 18:39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

spidermonkey: update to 102 and fix build under macOS #179822

spidermonkey: update to 102 and fix build under macOS #179822

catap commented Jul 1, 2022 •

edited

Loading

SuperSandro2000 commented Jul 1, 2022

catap commented Jul 4, 2022 •

edited

Loading

jtojnar commented Jul 4, 2022

catap commented Jul 4, 2022 •

edited

Loading

jtojnar commented Jul 4, 2022 •

edited

Loading

catap commented Jul 4, 2022

catap commented Jul 4, 2022 •

edited

Loading

jtojnar left a comment

jtojnar Jul 4, 2022

catap Jul 4, 2022

SuperSandro2000 Aug 1, 2022

SuperSandro2000 Aug 2, 2022 •

edited

Loading

catap commented Jul 4, 2022

nixos-discourse commented Aug 1, 2022

SuperSandro2000 left a comment

SuperSandro2000 Aug 1, 2022

catap commented Aug 2, 2022

wegank commented Aug 2, 2022 •

edited

Loading

wegank commented Aug 8, 2022

catap commented Aug 8, 2022

jtojnar Aug 9, 2022

jtojnar Aug 9, 2022

jtojnar commented Aug 10, 2022

-    ./allow-system-s-nspr-and-icu-on-bootstrapped-sysroot.patch
+    ./allow-system-s-nspr-and-icu-on-bootstrapped-sysroot.patch
+    # Patches required by GJS
+    # https://discourse.gnome.org/t/gnome-43-to-depend-on-spidermonkey-102/10658
+    # Install ProfilingCategoryList.h
+    (fetchpatch {
+      url = "https://hg.mozilla.org/releases/mozilla-esr102/raw-rev/33147b91e42b79f4c6dd3ec11cce96746018407a";
+      sha256 = "sha256-xJFJZMYJ6P11HQDZbr48GFgybpAeVcu3oLIFEyyMjBI=";
+    })
+    # Fix embeder build
+    (fetchpatch {
+      url = "https://hg.mozilla.org/releases/mozilla-esr102/raw-rev/1fa20fb474f5d149cc32d98df169dee5e6e6861b";
+      sha256 = "sha256-eCisKjNxy9SLr9KoEE2UB26BflUknnR7PIvnpezsZeA=";
+    })

spidermonkey: update to 102 and fix build under macOS #179822

spidermonkey: update to 102 and fix build under macOS #179822

Conversation

catap commented Jul 1, 2022 • edited Loading

Description of changes

Things done

SuperSandro2000 commented Jul 1, 2022

catap commented Jul 4, 2022 • edited Loading

jtojnar commented Jul 4, 2022

catap commented Jul 4, 2022 • edited Loading

jtojnar commented Jul 4, 2022 • edited Loading

catap commented Jul 4, 2022

catap commented Jul 4, 2022 • edited Loading

jtojnar left a comment

Choose a reason for hiding this comment

jtojnar Jul 4, 2022

Choose a reason for hiding this comment

catap Jul 4, 2022

Choose a reason for hiding this comment

SuperSandro2000 Aug 1, 2022

Choose a reason for hiding this comment

SuperSandro2000 Aug 2, 2022 • edited Loading

Choose a reason for hiding this comment

catap commented Jul 4, 2022

nixos-discourse commented Aug 1, 2022

SuperSandro2000 left a comment

Choose a reason for hiding this comment

SuperSandro2000 Aug 1, 2022

Choose a reason for hiding this comment

catap commented Aug 2, 2022

wegank commented Aug 2, 2022 • edited Loading

wegank commented Aug 8, 2022

catap commented Aug 8, 2022

jtojnar Aug 9, 2022

Choose a reason for hiding this comment

jtojnar Aug 9, 2022

Choose a reason for hiding this comment

jtojnar commented Aug 10, 2022

catap commented Jul 1, 2022 •

edited

Loading

catap commented Jul 4, 2022 •

edited

Loading

catap commented Jul 4, 2022 •

edited

Loading

jtojnar commented Jul 4, 2022 •

edited

Loading

catap commented Jul 4, 2022 •

edited

Loading

SuperSandro2000 Aug 2, 2022 •

edited

Loading

wegank commented Aug 2, 2022 •

edited

Loading