-
Notifications
You must be signed in to change notification settings - Fork 724
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(nix): Setup a head build for the cross_compatibility integ test #4567
Conversation
Strange |
codebuild/bin/install_s2n_head.sh
Outdated
echo "s2nc_head already exists; not rebuilding s2n_head" | ||
fi |
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.
How will this work locally / with nix? Like, I'm imagining that I'm developing a feature that requires me to run the cross_compatibility test. s2n_head could be quite old if I haven't run clean lately, and my understanding of this behavior is that I wouldn't really have any visibility into that beyond this one stdout message in the big block of build stdout messages.
Maybe we don't have to build every time, but should we have some sort of freshness check? Checking the version against github might be a bit too much-- Maybe as simple as the creation time for s2nc_head? At the bare minimum I'd try to add some visibility to this message, maybe noting which commit is in s2nc_head, how to force s2nc_head to rebuild, maybe some color.
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 had started down the path of checking s2n_head's commit, but thought, at least initially, it's a waste of time since the CI will always be running on a clean instance. Checking the age of the last commit in s2n_head might be lighter weight, let me try a few things.
Co-authored-by: Lindsay Stewart <[email protected]>
Merge commits are not allowed on this repository
Resolved issues:
Resolves #3841
Description of changes:
Modify the script checking out s2n-tls head and building s2nc_head/s2nd_head for use in the cross_compatibility integration test, in a nix devshell.
An ad-hoc CodeBuild test job with only
cross_compatibility
Call-outs:
There might be a more idiomatic nix way to do this.
For the kTLS tests, building twice puts us over our goal of 1 hour CI runs, so an env. flag was added to disable these builds:
S2N_NO_HEADBUILD
.Note: because the devShell contains the
CMAKE_*
environment variables, we don't need-DCMAKE_PREFIX_PATH
and the s2nc/d built from head will use the same libcrypto as the regular build.Testing:
How is this change tested (unit tests, fuzz tests, etc.)? locally/CI
Is this a refactor change? If so, how have you proved that the intended behavior hasn't changed?
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.