-
Notifications
You must be signed in to change notification settings - Fork 659
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
Use emscripten directory to store embedded config #367
base: master
Are you sure you want to change the base?
Conversation
@@ -1347,12 +1329,29 @@ def generate_dot_emscripten(active_tools): | |||
name, value = specific_cfg.split('=') | |||
activated_config[name] = value | |||
|
|||
active_config = os.path.join(emsdk_path(), '.active_config') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the "active config"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Its a file that contains that path to the current active .emscripten
.
This way the .emscritpen
and .emscripten_cache
for a given emscripten version don't need to change when switching sdks. All the acrtivate
needs to do is update the current active config but writing the filename to this file.
Peviously we would clobber the .emscritipen
at the top level.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm still confused - what's the scenario where we have multiple emscripten installs? Or is this for a single install with multiple .emscripten
config files?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Each SDK that emsdk installs is its emscripten install, right? You can install many SDKs and activate just one at a time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Previously activating one SDK would clobber the config and cache of the previously active one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we've gone back and forth on this...
Early emsdk did allow this, and even tried to avoid downloading more than once. We decided with @juj that it was too complicated and removed that part. Without the download part, it's not clear how to keep multiple versions around (it used the download file to see what's there, iirc). With the new releases infrastructure, I just ended up doing one dir for upstream and one for fastcomp. That also avoided problems we used to have with keeping around old versions and taking up lots of HD space for users.
Overall I think that's reasonable for the waterfall builds, since people generally upgrade and want the old version to be overwritten. I don't see much of a need for people to swap back and forthe between older and newer versions?
(However, it would be nice if we didn't re-download node.js and the other helper binaries, but we seem to be doing that now; that's a separate issue, though. Another separate issue is moving between upstream and fastcomp, which could reuse one dir, which would also be nice to fix.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we really only support 2 installs right now, in the
upstream/
andfastcomp/
directories. Any installed version just overwrites the previous one in that directory. But I think in the past each release got a directory?
Early emsdk did allow this, and even tried to avoid downloading more than once. We decided with @juj that it was too complicated and removed that part.
Wait what? It has been always the intent that emscripten supports any number of simultaneously installed Tools and SDKs. The intent is that emsdk activate x y z
changes which set of tools are active (if specified with --embedded
, the activation is for the current emsdk directory tree, if you had two emsdk directory trees, they can have separate toolsets active; and if specified without --embedded
, the activation is for the current user).
However I am not sure I follow this PR or the plan at emscripten-core/emscripten#9543, not quite sure what we win there.
I think instead of making --embedded
rewrite the file $EMSDK/.emscripten at each activation, we could make it hash the contents of what would be written to .emscripten, and write a .emscripten_ file, along with .emscripten_cache_ (or just hash emscripten+clang+binaryen combo that does affect the cache, to avoid e.g. java and other tools having an effect). That way we could have separate open cmdlines with separate tools active at the same time, each with their own cache.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now looking at what emsdk install latest
and emsdk install latest-fastcomp
do, they seem to be diverging from what the original intent of emsdk is, and the way that all other SDKs work.
The intent was that SDKs are just a list of Tools, but nothing more. I.e. a SDK just lists a group of Tools to install, but do not have anything in themselves to install. (installing an SDK was a "for t in tools_that_this_sdk_uses: t.install()")
With releases-upstream and releases-fastcomp SDKs, these SDKs all circularly depend on themselves. Instead of these SDKs depending on Tools, these SDKs are marked to depend on themselves (that are SDKs).
Also each Tool has had a unique installation directory, which enabled a custom combination of tools being possible to install. It looks like this releases-upstream
SDK-that-is-also-a-Tool has a hardcoded directory upstream/
that it always installs to, and the releases-fastcomp
SDK-that-is-also-a-Tool has a hardcoded directory fastcomp/
that it installs to. Now I understand the conversation lines above
I thought emsdk supported many version installed in parallel?
Early emsdk did allow this, and even tried to avoid downloading more than once. We decided with @juj that it was too complicated and removed that part.
However emsdk/I did never remove this support, but your releases-upstream
and releases-fastcomp
SDK-Tools adopted an approach that does not have this support. (The avoid downloading more than once was removed to work around a cleanup bug, but a separate topic)
I don't know which way you want to go on this, though just marking my notes, it generates a bit of diverging logic for this in cd161f1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree its confusing and I don't like the new "bundled SDK" thing either. Hopefully we can move back to place where repeat download doesn't happend and where simulatious installation is possible again. I think these two things are important.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we really only support 2 installs right now, in the
upstream/
andfastcomp/
directories. Any installed version just overwrites the previous one in that directory. But I think in the past each release got a directory?Early emsdk did allow this, and even tried to avoid downloading more than once. We decided with @juj that it was too complicated and removed that part.
Wait what? It has been always the intent that emscripten supports any number of simultaneously installed Tools and SDKs. The intent is that
emsdk activate x y z
changes which set of tools are active (if specified with--embedded
, the activation is for the current emsdk directory tree, if you had two emsdk directory trees, they can have separate toolsets active; and if specified without--embedded
, the activation is for the current user).However I am not sure I follow this PR or the plan at emscripten-core/emscripten#9543, not quite sure what we win there.
I think instead of making
--embedded
rewrite the file $EMSDK/.emscripten at each activation, we could make it hash the contents of what would be written to .emscripten, and write a .emscripten_ file, along with .emscripten_cache_ (or just hash emscripten+clang+binaryen combo that does affect the cache, to avoid e.g. java and other tools having an effect). That way we could have separate open cmdlines with separate tools active at the same time, each with their own cache.
The win would basically be that each copy of emscripten you have on your system could have its own config file, and that setting just the PATH would be enough to activate it. Maybe we can discuss more on emscripten-core/emscripten#9543? I'm certainly up for adjusting the plan.
I am a big fan of the deeply embedded idea because for most users it completely removes the use of the home directory and makes each emscripten directory as self sufficient and stand alone thing.
It would mean that you could use multiple SDKs at the same time simply by changing your PATH, and no caches or configs would need to change. It seems a lot simpler than trying to multiplex the files in the $HOME directory.
What does "version" mean in this sentence? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This means that each version of emscripten can have its own cache and config rather than emsdk holding single global one.
What does "version" mean in this sentence?
Each using copy of emscripten on the filesystem. I know the current trend for emsdk that there is only one... but I'd like to see that reversed to allow side-by-side installations. I know I have more than one copy of emscripten on my system the fact that they share common config/cache locations is a pain.
@juj are you on the WebAssembly discord channels yet? https://discord.gg/ETxb2f. Would be good to be able to chat about this kind of stuff in real time maybe? |
Thanks, joined now. Not sure how/if I'll grow into discord, but will try it out. |
17a08e7
to
6c27e97
Compare
This means that each version of emscripten can have its own cache and config rather than emsdk holding single global one. One of the main reasons for doing it this way is so that we can introduce the concept of an embedded config upstream in emscripten. Eventually I'd like to always use embedded config both with emsdk and emscripten. See #9543
1550497
to
95678a8
Compare
…630.1 (emscripten-core#367) Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com>
This means that each version of emscripten can have its own cache
and config rather than emsdk holding single global one.
One of the main reasons for doing it this way is so that we can
introduce the concept of an embedded config upstream in emscripten.
This also means that switching between sdk versions no longer clears
the cache.
Eventually I'd like to always use embedded config both with emsdk
and emscripten. See emscripten-core/emscripten#9543