Update node, 15.14.0 -> 16.20.0 #1232

dschuff · 2023-06-14T00:14:52Z

This allows us to use the native aarch64 version on MacOS

sbc100

lgtm if tests pass

dschuff · 2023-06-14T00:40:06Z

This seems to be working other than the fact that the tests use some versions that aren't supported on arm64 (e.g. 1.39.15 in test.sh). How important is it to test such an old version? Should we keep the very old versions (and just skip that test on M1) or update to a more recent version?

sbc100 · 2023-06-14T16:50:19Z

This seems to be working other than the fact that the tests use some versions that aren't supported on arm64 (e.g. 1.39.15 in test.sh). How important is it to test such an old version? Should we keep the very old versions (and just skip that test on M1) or update to a more recent version?

I think just skip that part of the test when running on arm64

sbc100 · 2023-06-14T17:23:01Z

Hopefully this is a step towards #1173

sbc100 · 2023-06-14T18:20:02Z

I wonder if we should announce this change somewhere? I guess its mostly internal facing, but some folks might be using the emsdk version of node directly I guess?

dschuff · 2023-06-14T18:28:12Z

Yeah, it's probably worth mentioning. Maybe someone would want to disable Rosetta or something and we are the only thing holding them back. In any case it seems worth announcing the node update, and the architecture update would go with that.

sbc100 · 2023-06-14T18:32:39Z

Yeah, it's probably worth mentioning. Maybe someone would want to disable Rosetta or something and we are the only thing holding them back. In any case it seems worth announcing the node update, and the architecture update would go with that.

I'm just not sure we have a good place to announce such changes.

The other thing that I hadn't previously considered is that this change retro-actively upgrade node for all old versions of emsdk.. so there is a risk we could break old things that used to work. We should be a little careful here.. its a shame we don't have a good mechanism for using this new version only with newer versions of the sdk.

dschuff · 2023-06-14T18:38:09Z

Hm, yeah. I guess this could only happen if someone uses a new version of emsdk to download an old version of emscripten, and it in theory should only affect the code that emscripten itself runs, right? Hopefully not too many users are using emsdk's node for running their own tests or tooling or something?

dschuff · 2023-06-14T18:40:12Z

test/test.py

@@ -220,7 +226,7 @@ def test_binaryen_from_source(self):
  def test_no_32bit(self):
    print('test 32-bit error')
    emsdk_hacked = hack_emsdk('not is_os_64bit()', 'True')
-    failing_call_with_output('python %s install latest' % emsdk_hacked, 'this tool is only provided for 64-bit OSes')
+    failing_call_with_output('python3 %s install latest' % emsdk_hacked, 'this tool is only provided for 64-bit OSes')


Question about this bit: I updated this to python3 because just 'python' doesn't work on mac when updating to the newer XCode. But 'python3' doesn't work on windows.

what is this test trying to check? Is there a better way than this?

I don't think that anything currently exists.. maybe probe using shutil.which to see which one to use (`https://docs.python.org/3/library/shutil.html#shutil.which)

dschuff · 2023-06-14T20:25:10Z

I just realized that test.py runs under python2 on Windows, and we flake8 on both py2 and py3. Now that emscripten requires python3, can we drop py2?
(I would make that separate from this change, but just wondering if that was intentional).

sbc100

We need to support python2 in emsdk still.. at least until we can somehow bootstrap using our own python3.

dschuff · 2023-06-14T21:59:49Z

OK, all the tests are passing now!
given the above comment, do you still think this is a good idea?

sbc100 · 2023-06-14T22:26:37Z

Which comment are you talking about?

I do think this is a good idea yes. If it proves problematic to use node 16 with older versions of emsdk we can figure out a way to install node 15 for older versions.. but in the past when we have done these upgrades it hasn't been a problem (IIRC).

sbc100

@juj just in case ..

sbc100 · 2023-06-14T23:56:33Z

.circleci/config.yml

      # brew itself which takes more than 4 minutes.
      HOMEBREW_NO_AUTO_UPDATE: "1"
    macos:
-      xcode: "12.5.1"
+      xcode: "13.4.1"


Actually does this dermine the base image to use.. or just the xcode version that is installed in the image? If it changes the base image I think we should revert this line.

I'm not sure. The reason I changed it though is because there is no 12.5.1 for M1, and I figured it would be good for them to match.

It looks like it does map to the OS version: https://circleci.com/docs/using-macos/#supported-xcode-versions-intel.. so I think we should revert this line.

ok. so you're ok with the version not matching between the different hardware?

Sure. I think the important thing is that we are testing on older versions of macOS where possible

sbc100 · 2023-06-20T15:55:25Z

I think we can land this now.

juj · 2023-06-21T07:16:05Z

Thanks for the ping. Looks good, and we should be good on this front overall as well.

Btw, I will be on vacation until early August, so my response time will be delayed up a bit until then.

) This version of node supports wasm-bigint by default, so it will help #19156 Node 16 is over 2 years old, and the node website already provides newer versions both for latest and for LTS. This is possible after emscripten-core/emsdk#829 made the emsdk install a 15.x version by default, and then emscripten-core/emsdk#1232 switched to 16.x.

This allows us to use the native ARM64 version on MacOS. Also update the test scripts to work on ARM64 mac, and skip tests that aren't relevant.

dschuff added 3 commits June 13, 2023 17:13

Update node, 15.14.0 -> 16.20.0

4b5a2e6

This allows us to use the native aarch64 version on MacOS

Try adding M1 mac on CI

ce3d2bc

Add workflow

6185b97

sbc100 approved these changes Jun 14, 2023

View reviewed changes

dschuff added 3 commits June 13, 2023 17:23

Use a valid xcode

320c30a

Use a valid xcode

5adc1bb

udpate test to use an emscripten-releases hash that has arm64 binaries

2cd2f9a

sbc100 mentioned this pull request Jun 14, 2023

Update default minimum Node.js version to 16.0.0 emscripten-core/emscripten#19192

Merged

conditionalize install test of 1.39.15 to x86-64

d2d18ad

sbc100 approved these changes Jun 14, 2023

View reviewed changes

dschuff added 3 commits June 14, 2023 11:02

factor macos CI steps into shared commands

567fdac

skip specific version tests on arm64, use python3 instead of python

87da54d

fix python variable

db3b675

dschuff commented Jun 14, 2023

View reviewed changes

use sys.executable, fix quotes

f19a260

go back to py2

27bf931

sbc100 reviewed Jun 14, 2023

View reviewed changes

sbc100 approved these changes Jun 14, 2023

View reviewed changes

sbc100 reviewed Jun 14, 2023

View reviewed changes

jeswr mentioned this pull request Jun 15, 2023

remove node version override in Dockerfile SWI-Prolog/npm-swipl-wasm#235

Closed

go back to previous xcode version

e828415

dschuff enabled auto-merge (squash) June 20, 2023 18:37

dschuff merged commit d7327b4 into main Jun 20, 2023

dschuff deleted the update-node branch June 20, 2023 18:57

sbc100 mentioned this pull request Nov 27, 2023

Please bump Node version on emscripten/emsdk #1303

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update node, 15.14.0 -> 16.20.0 #1232

Update node, 15.14.0 -> 16.20.0 #1232

dschuff commented Jun 14, 2023

sbc100 left a comment

dschuff commented Jun 14, 2023

sbc100 commented Jun 14, 2023

sbc100 commented Jun 14, 2023

sbc100 commented Jun 14, 2023

dschuff commented Jun 14, 2023

sbc100 commented Jun 14, 2023

dschuff commented Jun 14, 2023

dschuff Jun 14, 2023

sbc100 Jun 14, 2023 •

edited

Loading

dschuff commented Jun 14, 2023

sbc100 left a comment

dschuff commented Jun 14, 2023

sbc100 commented Jun 14, 2023

sbc100 left a comment

sbc100 Jun 14, 2023

dschuff Jun 15, 2023

sbc100 Jun 20, 2023

dschuff Jun 20, 2023

sbc100 Jun 20, 2023

sbc100 commented Jun 20, 2023

juj commented Jun 21, 2023

Update node, 15.14.0 -> 16.20.0 #1232

Update node, 15.14.0 -> 16.20.0 #1232

Conversation

dschuff commented Jun 14, 2023

sbc100 left a comment

Choose a reason for hiding this comment

dschuff commented Jun 14, 2023

sbc100 commented Jun 14, 2023

sbc100 commented Jun 14, 2023

sbc100 commented Jun 14, 2023

dschuff commented Jun 14, 2023

sbc100 commented Jun 14, 2023

dschuff commented Jun 14, 2023

dschuff Jun 14, 2023

Choose a reason for hiding this comment

sbc100 Jun 14, 2023 • edited Loading

Choose a reason for hiding this comment

dschuff commented Jun 14, 2023

sbc100 left a comment

Choose a reason for hiding this comment

dschuff commented Jun 14, 2023

sbc100 commented Jun 14, 2023

sbc100 left a comment

Choose a reason for hiding this comment

sbc100 Jun 14, 2023

Choose a reason for hiding this comment

dschuff Jun 15, 2023

Choose a reason for hiding this comment

sbc100 Jun 20, 2023

Choose a reason for hiding this comment

dschuff Jun 20, 2023

Choose a reason for hiding this comment

sbc100 Jun 20, 2023

Choose a reason for hiding this comment

sbc100 commented Jun 20, 2023

juj commented Jun 21, 2023

sbc100 Jun 14, 2023 •

edited

Loading