Skip to content

Allow the install of any emscripten-releases build#732

Merged
sbc100 merged 1 commit intomasterfrom
install_any_release
Mar 2, 2021
Merged

Allow the install of any emscripten-releases build#732
sbc100 merged 1 commit intomasterfrom
install_any_release

Conversation

@sbc100
Copy link
Copy Markdown
Collaborator

@sbc100 sbc100 commented Feb 26, 2021

This is done by detecting the 40 character git sha in the SDK version
and creating a new set of tools based on this SHA. This works for any
command that uses expand_sdk_name (i.e. install activate and update) but
it wont show up in the output of list.

For example:

./emsdk install sdk-releases-upstream-b0cfdb236483b6828ee2e3f263fd94f011ed1863-64bit

Or just:

./emsdk install releases-upstream-b0cfdb236483b6828ee2e3f263fd94f011ed1863

Fixes: #732

@sbc100 sbc100 requested review from dschuff and kripken February 26, 2021 01:55
@sbc100
Copy link
Copy Markdown
Collaborator Author

sbc100 commented Feb 26, 2021

I can probably re-factor this to make the diff more readable ...

@sbc100 sbc100 mentioned this pull request Feb 26, 2021
@sbc100 sbc100 force-pushed the install_any_release branch 2 times, most recently from 755c9ef to bb607a6 Compare February 26, 2021 15:40
Base automatically changed from fix_fastcomp_crash to master February 26, 2021 16:48
@dschuff
Copy link
Copy Markdown
Member

dschuff commented Feb 26, 2021

would it be possible to stop using the "-64-bit" suffix? We only release 64-bit builds now

@dschuff
Copy link
Copy Markdown
Member

dschuff commented Feb 26, 2021

Although actually before long we will need to start releasing ARM64 mac builds too. And if we don't want to distribute fat binaries, then emsdk will still need some way to specify architecture.

Copy link
Copy Markdown
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.

this looks fine for now though, since it just tweaks the existing framework

@sbc100
Copy link
Copy Markdown
Collaborator Author

sbc100 commented Feb 26, 2021

would it be possible to stop using the "-64-bit" suffix? We only release 64-bit builds now

The suffix is not needed on the command line. Its just part of the full internal name.

@sbc100 sbc100 force-pushed the install_any_release branch from bb607a6 to bdaa2f2 Compare February 26, 2021 19:23
Copy link
Copy Markdown
Member

@kripken kripken left a comment

Choose a reason for hiding this comment

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

lgtm with a test + docs.

In addition to docs in --help here, please update https://emscripten.org/docs/contributing/developers_guide.html#bisecting which can be much simpler thanks to this nice change.

@sbc100 sbc100 force-pushed the install_any_release branch 2 times, most recently from 5c6f18f to 99b0e07 Compare March 1, 2021 21:14
@sbc100
Copy link
Copy Markdown
Collaborator Author

sbc100 commented Mar 1, 2021

Added a test based on the new unit test framework.

Copy link
Copy Markdown
Member

@kripken kripken left a comment

Choose a reason for hiding this comment

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

lgtm with an update to --help

@sbc100 sbc100 force-pushed the install_any_release branch from 99b0e07 to d55e5e8 Compare March 1, 2021 21:21
@sbc100
Copy link
Copy Markdown
Collaborator Author

sbc100 commented Mar 1, 2021

Do you think its worth adding something specific to --help.. this seems like kind of an advanced feature. Perhaps it would be enough to add to the FAQ and/or bisect instructions?

@sbc100
Copy link
Copy Markdown
Collaborator Author

sbc100 commented Mar 1, 2021

I guess if its added anywhere it would want to be under --list since it comes under "what things can I install".

@sbc100 sbc100 force-pushed the install_any_release branch from d55e5e8 to 1d17d66 Compare March 1, 2021 21:47
@dschuff
Copy link
Copy Markdown
Member

dschuff commented Mar 1, 2021

the question of "what does a version hash mean" is somewhat complex. Does the emsdk documentation says anything about emscripten-releases versions? or is it just internal?

@sbc100
Copy link
Copy Markdown
Collaborator Author

sbc100 commented Mar 2, 2021

We certainly talk about it here: https://emscripten.org/docs/contributing/developers_guide.html#bisecting

I'm not sure we want to talk about it in --list. If its OK with both you we can land this as is and followup with a change to that above guide?

@dschuff
Copy link
Copy Markdown
Member

dschuff commented Mar 2, 2021

sounds good to me.

This is done by detecting the 40 character git sha in the SDK version
and creating a new set of tools based on this SHA.  This works for any
command that uses expand_sdk_name (i.e. install activate and update) but
it wont show up in the output of `list`.

For example:

./emsdk install sdk-releases-upstream-b0cfdb236483b6828ee2e3f263fd94f011ed1863-64bit

Or just:

./emsdk install releases-upstream-b0cfdb236483b6828ee2e3f263fd94f011ed1863
@sbc100 sbc100 force-pushed the install_any_release branch from 1d17d66 to a145d28 Compare March 2, 2021 01:10
@sbc100 sbc100 enabled auto-merge (squash) March 2, 2021 01:15
@kripken
Copy link
Copy Markdown
Member

kripken commented Mar 2, 2021

sgtm

@sbc100 sbc100 merged commit 53eacf4 into master Mar 2, 2021
auto-merge was automatically disabled March 2, 2021 01:32

Pull request was closed

@sbc100 sbc100 deleted the install_any_release branch March 2, 2021 01:32
@sbc100 sbc100 mentioned this pull request Mar 4, 2021
sbc100 added a commit that referenced this pull request Mar 8, 2021
Since #732 we lookup tot release dynamically, but we don't
necessarily want to do this for the `activate` command otherwise
`install` followed by `activate` a can fail if a new build was
produced inbetween.
sbc100 added a commit that referenced this pull request Mar 8, 2021
Since #732 we lookup tot release dynamically, but we don't
necessarily want to do this for the `activate` command otherwise
`install` followed by `activate` a can fail if a new build was
produced inbetween.
akoeplinger pushed a commit to akoeplinger/emsdk that referenced this pull request Dec 13, 2024
…pten-core#732)

runtime.linux-arm64.Microsoft.NETCore.Runtime.Wasm.Node.Transport , runtime.linux-musl-arm64.Microsoft.NETCore.Runtime.Wasm.Node.Transport , runtime.linux-musl-x64.Microsoft.NETCore.Runtime.Wasm.Node.Transport , runtime.linux-x64.Microsoft.NETCore.Runtime.Wasm.Node.Transport , runtime.osx-arm64.Microsoft.NETCore.Runtime.Wasm.Node.Transport , runtime.osx-x64.Microsoft.NETCore.Runtime.Wasm.Node.Transport , runtime.win-arm64.Microsoft.NETCore.Runtime.Wasm.Node.Transport , runtime.win-x64.Microsoft.NETCore.Runtime.Wasm.Node.Transport
 From Version 9.0.0-alpha.1.24172.1 -> To Version 9.0.0-alpha.1.24174.1

Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com>
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.

3 participants