Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Switch to .xz by default for SDK downloads #1281

Merged
merged 1 commit into from
Sep 21, 2023
Merged

Switch to .xz by default for SDK downloads #1281

merged 1 commit into from
Sep 21, 2023

Conversation

sbc100
Copy link
Collaborator

@sbc100 sbc100 commented Sep 21, 2023

This is a bit of a hack but I can't think of another way to do it. Basically when downloading SDKs, we first try the new .xz extension. If that fails, we fall back to the old .tbz2. Both these first two download attempts we run in "silent" mode. If both of them fail we re-run the original request in non-silent mode so that the error message will always contain the original .xz extension.

See #1235

@sbc100 sbc100 requested a review from dschuff September 21, 2023 00:21
@sbc100
Copy link
Collaborator Author

sbc100 commented Sep 21, 2023

I manually uploaded an .xz version the 3.1.46 linux binaries so that test-linux bot here will use that version over the tbz2

@sbc100
Copy link
Collaborator Author

sbc100 commented Sep 21, 2023

This is a bit of a hack but I can't think of another way to do it.
Basically when downloading SDKs, we first try the new `.xz` extension.
If that fails, we fall back to the old `.tbz2`.  Both these first two
download attempts we run in "silent" mode.  If both of them fail we
re-run the original request in non-silent mode so that the error message
will always contain the original `.xz` extension.

See #1235
@@ -34,8 +34,8 @@
"version": "%releases-tag%",
"bitness": 64,
"arch": "x86_64",
"linux_url": "https://storage.googleapis.com/webassembly/emscripten-releases-builds/linux/%releases-tag%/wasm-binaries.tbz2",
"macos_url": "https://storage.googleapis.com/webassembly/emscripten-releases-builds/mac/%releases-tag%/wasm-binaries.tbz2",
"linux_url": "https://storage.googleapis.com/webassembly/emscripten-releases-builds/linux/%releases-tag%/wasm-binaries.tar.xz",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if it's worth leaving a comment here that the .tbz2 equivalents to these URLs are also tried if xz is not fuond.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

json doesn't support comments 😭

Copy link
Member

@dschuff dschuff Sep 21, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh right, I always forget that 😿

# `.xz`, but we can't tell from the version/url which one to use, so
# try one and then fall back to the other.
success = False
if 'wasm-binaries' in archive and os.path.splitext(archive)[1] == '.xz':
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess this will also cause a retry for any other kind of download failure (first a silent failure, then a retry with a potentially loud failure), but in the successful case of fetching an xz, there will be only one request, right?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

right

@dschuff
Copy link
Member

dschuff commented Sep 21, 2023

Relatedly, I wonder if we can do any better on Windows. I don't know of any other archive formats natively supported by Windows, do you? I wonder if it's worth bundling something with emsdk, it makes a pretty big compression difference.

Copy link
Member

@dschuff dschuff left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess this is fine. I don't really have any better ideas that don't involve more advanced networking libraries like requests, which may not be available on users' machines and maybe we don't want to bother bundling.

@sbc100 sbc100 merged commit d42c740 into main Sep 21, 2023
10 checks passed
@sbc100 sbc100 deleted the use_xz branch September 21, 2023 22:16
@dschuff
Copy link
Member

dschuff commented Sep 21, 2023

(oh, maybe you should mention that this only affects Mac and Linux. If we could do something for windows too, it could still have a big impact).

@sbc100
Copy link
Collaborator Author

sbc100 commented Sep 21, 2023

(oh, maybe you should mention that this only affects Mac and Linux. If we could do something for windows too, it could still have a big impact).

Mention where?

@dschuff
Copy link
Member

dschuff commented Sep 21, 2023

I guess I meant in the commit description; but as mentioned this probably warrants a changelog entry and maybe an announcement to emscripten-discuss (the fact that most users need to update emsdk to see new versions makes this less of a big deal I guess, but still maybe worth doing).

@sbc100
Copy link
Collaborator Author

sbc100 commented Sep 21, 2023

Will do

shlomif pushed a commit to shlomif/emsdk that referenced this pull request Sep 29, 2023
This is a bit of a hack but I can't think of another way to do it.
Basically when downloading SDKs, we first try the new `.xz` extension.
If that fails, we fall back to the old `.tbz2`.  Both these first two
download attempts we run in "silent" mode.  If both of them fail we
re-run the original request in non-silent mode so that the error message
will always contain the original `.xz` extension.

See emscripten-core#1235
@emscripten-core emscripten-core deleted a comment Oct 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants