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

ci: use temporary directory for s2n_head build #4771

Merged
merged 3 commits into from
Sep 25, 2024
Merged

Conversation

lrstewart
Copy link
Contributor

@lrstewart lrstewart commented Sep 14, 2024

Description of changes:

The build command in the nix develop shell ends up creating an "s2n_head" clone in the main repo. That's annoying because 1) the new folder shows up in "git diff" (although tbf that could be fixed by an update to .gitignore) 2) duplicate results show up in any search not specifically configured to ignore the clone. This is particularly annoying because the folder is created even when running the unit tests, not the integration tests (s2n_head is used by the cross_compatibility integ test).

Test scripts shouldn't be writing build artifacts to our source directory, or even keeping them around at all. Persisting the build artifacts is unnecessary-- all we need are the s2nc_head and s2nd_head executables. $BUILD_DIR is a temporary folder, so we can instead put all the intermediate build artifacts there. Alternatively, if we don't want the build artifacts in a temporary folder, I could put them in the build folder.

We also don't need to build all the tests, just s2nc and s2nd. So I switched to targeted build commands. It's much faster.

Testing:

I kicked off a manual build of the standard integ tests: here
I also kicked off a manual build of the nix integ tests: here. However, I had to manually add the "cross_compatibility" test to the spec, because it looks like the nix integ tests don't include it. I'm not sure if that's intentional or an oversight, but the tests do pass with it added.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@github-actions github-actions bot added the s2n-core team label Sep 14, 2024
@lrstewart lrstewart marked this pull request as ready for review September 14, 2024 22:35
@lrstewart lrstewart requested a review from dougch as a code owner September 14, 2024 22:35
@lrstewart lrstewart enabled auto-merge (squash) September 24, 2024 20:38
@lrstewart lrstewart merged commit cd9eaa5 into aws:main Sep 25, 2024
37 checks passed
@lrstewart lrstewart deleted the s2n_head branch September 25, 2024 08:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants