feat: enable use of preexisting weval binary #156
Conversation
This commit adds the ability to use a pre-existing `weval` binary as an option to `componentize`. This should enable builds without internet but hopefully also cut down on failures due to being unable to fetch the binary dynamically. Signed-off-by: Victor Adossi <[email protected]>
Signed-off-by: Victor Adossi <[email protected]>
Signed-off-by: Victor Adossi <[email protected]>
Signed-off-by: Victor Adossi <[email protected]>
vados-cosmonic
force-pushed
the
feat=enable-use-of-preexisting-weval-binary
branch
24 times, most recently
from
61e63f8
to
729001d
Compare
vados-cosmonic
force-pushed
the
feat=enable-use-of-preexisting-weval-binary
branch
16 times, most recently
from
210d9b0
to
a8f74f5
Compare
This commit heavily refactors the CI build pipeline to separate different build steps to improve build times and caching. It *should* be easier to see the dependency between the builds: - splicer (jit/AOT independent) - StarlingMonkey JIT - StarlingMonkey AOT (Weval) Along with these builds, JIT/AOT tests that should come after can pull the relevant cached artifacts. Signed-off-by: Victor Adossi <[email protected]>
vados-cosmonic
force-pushed
the
feat=enable-use-of-preexisting-weval-binary
branch
from
a8f74f5
to
13203d4
Compare
|
Hey @guybedford requesting another review just because of the dramatic change to the CI stuff 🙇 -- took a while but it's finally 🟢 I'm pretty sure I covered the dependency graph properly, the build should now be much faster. We'll have to pay extra attention to cache keys for the three builds in the future, though IMO the current set should work for a while. Right now we build against |
|
What happened to the Weval-specific update to just use the direct release URLs? If that doesn't require API gating that sounds ideal to me. Is this PR still needed if that lands? |
|
Hey @guybedford sorry a bit late getting to this, but I was hoping that the update to use direct URLs and this could land -- this PR would enable using a pre-existing weval binary (which would remove our need to pull the URLs at all -- though we should still pick the more direct URL) |
|
@vados-cosmonic thanks for the confirmation - I don't mean to be pedantic, but I feel that it's important to get this one fixed upstream first, before landing the alternative paths, as that solves the issue for all Weval users instead of just us. |
|
I've posted bytecodealliance/weval#13 for clarity. |
This commit enables using a pre-existing
wevalbinary -- this is mostly to try to avoid some flaky tests injcodownstream, but this is probably a reasonable thing to have in general.I also shuffled around some instructions/docs while I was in there.