Add llvm-12 and rust-compat tags

[RReverser]

These are human-readable tags for emscripten-core/emscripten#14394 (comment).

I went with rust-compat instead of rust because emsdk install rust might be confusing and suggest that it will install Rust itself too.

[RReverser]

ValueError: invalid literal for int() with base 10: 'rust'

okayyy

[RReverser]

Right, I thought I'd just remove the Rust one for now, but llvm tag fails too:

ValueError: invalid literal for int() with base 10: 'llvm'

I guess we need to change the code to accept identifiers in this position rather than trying to parse them as versions? Or maybe I need to add those to a different file? @sbc100 @kripken @dschuff

[kripken]

Yes, I guess there is code somewhere that assumes those are integers, that needs to be generalized. Fixing that is one option here.

Another might be to include the version number, that is, emit something like x.y.z-rust-a.b.c, x.y.z-llvm-w. The x.y.z version would be the emscripten version. That does seem useful in a sense, as it's close to that emscripten version, just adjusted. But maybe that could be confusing, I'm not sure.

[kripken]

The fix is pretty simple to allow those, but we are missing some info:

diff --git a/emsdk.py b/emsdk.py
index 79ffc55..9356945 100644
--- a/emsdk.py
+++ b/emsdk.py
@@ -2232,7 +2232,13 @@ def get_release_hash(arg, releases_info):
 
 
 def version_key(ver):
-  return tuple(map(int, re.split('[._-]', ver)[:3]))
+  try:
+    return tuple(map(int, re.split('[._-]', ver)[:3]))
+  except:
+    # some tagged releases are not numbered (e.g., "foo" instead of "2.0.x"),
+    # and we have no version for them.
+    return ()
 
 
 # A sort function that is compatible with both Python 2 and Python 3 using a

Without the version number, we don't know if it's upstream or fastcomp. We could perhaps force these all to be "2.0.0+", but that might be an argument for going the other way and including the version number.

[RReverser]

That does seem useful in a sense, as it's close to that emscripten version, just adjusted. But maybe that could be confusing, I'm not sure.

I think that somewhat defeats the purpose, because now the person still needs to look through the list of tags to find the suitable version. I'd rather have a convenience alias that's easy to remember & reuse.

[sbc100]

Can we no just use the numbered released closest to the version we want?

Say, for example, 2.0.6. If we want to be very precise we could add a new tag called, for example, 2.0.6-rust-compat, then have "latest-rust-compat" work just like the "latest" alias above.

I think having it called something like 2.0.6-rust-compat is nice because it places the release in the timeline.

[RReverser]

The idea here (until we automate it on either emsdk or rustup side) to have a tracked tag that works for latest stable Rust and that we can update over time. If we have version tags, it creates possibility of several tags with X.Y.Z-rust-compat which again puts burden on user to find the one that works with latest stable.

It might be relatively easy, but I still think we should make things as simple as

rustup toolchain add stable
rustup target add wasm32-unknown-emscripten
emsdk install rust-compat

where user never has to bother with looking up actual version numbers.

[sbc100]

Totally agree. I was thinking internally though rust-compat could be an alias for X.Y.Z-rust-compat which itself then maps to a release SHA. See #837 for adding better alias system.

[kripken]

The new alias system should make this much easier, nice!

I think my worries about x.y.z-rust etc. possibly being confusing are resolved by that. That is, the user would never need to see the x.y.z, as the alias takes care of it.

[kripken]

rustup toolchain add stable
rustup target add wasm32-unknown-emscripten
emsdk install rust-compat

I think we need a version on the last line, though? The emsdk doesn't know that stable rust was installed. Or are you thinking that we would always assume stable in rust-compat, and maybe add other aliases for nightly etc.?

If so, then we would need to update rust-compat every time stable rust changes - how often is that?

[RReverser]

Or are you thinking that we would always assume stable in rust-compat

Yeah that's what we talked about in the meeting and agreed upon I thought. The expectation is that most users are on latest stable Rust.

update rust-compat every time stable rust changes - how often is that?

Rather, every time they roll new LLVM, which IIRC @tlively said every 6 months.

[RReverser]

I'll leave this PR open, but sounds like we should wait for emscripten-core/emscripten#14394 (comment) (@kripken's comment) to resolve first.

[sbc100]

We should revisit this now that we have support for multiple "aliases".

[RReverser]

The comment above still stands. Looks like we'll need to branch off 2.0.13 and backport or implement fixes for both issues listed in https://github.com/RReverser/emscripten-rust-test, and then use the resulting branch for these tags in emsdk.

[RReverser]

The comment above still stands. Looks like we'll need to branch off 2.0.13 and backport or implement fixes for both issues listed in https://github.com/RReverser/emscripten-rust-test, and then use the resulting branch for these tags in emsdk.

[sbc100]

Sadly we don't have any mechanism for backports or building binaries from anything but the main branch.

If there is no suitable version of the SDK from the past and we do really require a branched/patched SDK we would need to invest in some new tools.

[RReverser]

Sadly we don't have any mechanism for backports or building binaries from anything but the main branch.

If there is no suitable version of the SDK from the past and we do really require a branched/patched SDK we would need to invest in some new tools.

Oh, huh. I thought one-off build would be fairly easy.

[sbc100]

Sadly we don't have any mechanism for backports or building binaries from anything but the main branch.
If there is no suitable version of the SDK from the past and we do really require a branched/patched SDK we would need to invest in some new tools.

Oh, huh. I thought one-off build would be fairly easy.

I mean we have a script that build the binaries.. its just getting it run on mac/linux and windows and having all the binaries uploaded. Not a huge feat, but we don't have any way to tell the bots to do it for us sadly.

[RReverser]

I mean we have a script that build the binaries.. its just getting it run on mac/linux and windows and having all the binaries uploaded. Not a huge feat, but we don't have any way to tell the bots to do it for us sadly.

Hmm, my understanding is that the fixes only need to be done on the Python side, so we probably could just reuse the binaries. But yeah I have no idea how uploading works.

[kripken]

I think that @RReverser 's point is right, that if the only patches needed are to emscripten, we can use the existing binaries. That is, we could add an option for the emsdk to download LLVM and Binaryen binaries for one releases hash, and use an arbitrary emscripten version with them. The emscripten version would be an arbitrary commit in emscripten, which in practice would be branched off from the releases hash for the binaries. We could test that on emscripten using those binaries (basically just tell CI to use them instead of tot).

So I think we would not need any bot changes to do this. Just an emsdk option - if it doesn't have one already? - to install emscripten from a specific emscripten (not releases) commit.

[sbc100]

This would all be much easier if we hadn't fused all the tools into one big download. In the past llvm and binaryen and emscripten could all be pinned independently.

These days there is no tarball that contains llvm but not also emscripten itself :( I mean we can hack it so that we ignore that copy.. but its a bit sad.

[RReverser]

We probably want to add those tags at some point but the ones I used in this PR are pretty outdated by now.