Build test components instead of relying on pre-built binaries #2163
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
dicej
approved these changes
Dec 12, 2023
rylev
force-pushed
the
build-test-components
branch
2 times, most recently
from
3dc6632
to
34fae68
Compare
Signed-off-by: Ryan Levick <ryan.levick@fermyon.com>
rylev
force-pushed
the
build-test-components
branch
from
34fae68
to
f3912c5
Compare
rylev
commented
Dec 14, 2023
| @@ -87,7 +87,7 @@ jobs: | |||
| rust-cache: true | |||
|
|
|||
| - name: Cargo Build | |||
| run: cross build --target x86_64-unknown-linux-musl --workspace --release --all-targets --features openssl/vendored | |||
There was a problem hiding this comment.
@rajatjindal FYI: I have removed these flags since they are unnecessary for building a Spin executable. I noticed this when the build was failing due to the test-components crate failing to build, but that crate does not need to be built to get a Spin executable.
There was a problem hiding this comment.
thank you for the heads up. these are also used in release workflow here https://github.com/fermyon/spin/blob/d41deb9eb33faeaa069d24a3922f26406c1cd2ca/.github/workflows/release.yml#L429C14-L429C20
you may want to remove those as well in that case
Signed-off-by: Ryan Levick <ryan.levick@fermyon.com>
rylev
force-pushed
the
build-test-components
branch
from
beb9161
to
b15fac9
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Until now we've been relying on pre-built test component binaries for the runtime tests. This has the advantage of being very fast (the binaries are already built, and so there is no build phase at all). However, it also has the disadvantage of potentially getting out of sync with the source. This PR changes the test components to be built as a part of building Spin.
This is not without its drawbacks: for example, the test components are a part of the workspace meaning they will be built during
cargo build. This does have the counter advantage though of making the test components part of rust-analyzer builds so that things like auto-complete work.Ultimately, I think this is the right way to go for now. In the future, we may want to reconsider whether we move these to a separate repository and pre-build the binaries with some check that those pre-built binaries do not get of sync with their source.