-
Notifications
You must be signed in to change notification settings - Fork 26
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
add support for a "build-all" task #357
Conversation
This was previously used to force variants to rebuild every time the build was invoked. Now that the dependency tracking is more robust, this can be safely dropped so that images will only be rebuilt when one of their inputs has changed. Signed-off-by: Ben Cressey <[email protected]>
When building all variants, `cargo` will start many builds at once, which won't match up with the value in the environment variable. Instead, use CARGO_MANIFEST_DIR to determine which variant is being built. Parse that into its subcomponents at runtime to avoid needing any of the other variant-related environment variables. Signed-off-by: Ben Cressey <[email protected]>
Rather than failing the build for an unsupported architecture, emit a warning. Otherwise, building all variants would always fail if any variant definition did not support the requested architecture. Signed-off-by: Ben Cressey <[email protected]>
`buildsys` now parses the variant components on its own, and ignores these environment variables. Signed-off-by: Ben Cressey <[email protected]>
Previously, this functionality was in a separate binary because the environment variables needed to be set prior to `buildsys` invocation so that `cargo` could track them for change detection purposes. Now that packages can't be conditionally compiled based on the variant, change detection isn't required, so the binary can be removed. Signed-off-by: Ben Cressey <[email protected]>
twoliter/embedded/Makefile.toml
Outdated
|
||
# Save built artifacts for each architecture. We don't set this everywhere | ||
# because we build host tools with cargo as well, like buildsys and pubsys. | ||
export CARGO_TARGET_DIR=${BUILDSYS_ROOT_DIR}/target/${BUILDSYS_ARCH} |
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.
nit. might want quotes
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.
Cool!
@@ -894,6 +894,29 @@ ln -snf "${BUILDSYS_VERSION_FULL}" "${OUTPUT_LOGS_DIR}/latest" | |||
''' | |||
] | |||
|
|||
[tasks.build-all] | |||
dependencies = ["fetch", "build-sbkeys", "publish-setup", "cargo-metadata"] |
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.
Either this PR or #361 will need to rebase here. I have no problem doing so, and I think you have your approvals first!
twoliter/embedded/Makefile.toml
Outdated
# Save built artifacts for each architecture. We don't set this everywhere | ||
# because we build host tools with cargo as well, like buildsys and pubsys. |
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 comment doesn't seem correct anymore, since we don't build host tools in Makefile.toml. Maybe we can just set CARGO_TARGET_DIR
everywhere?
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.
Maybe we can just set CARGO_TARGET_DIR everywhere?
Maybe, though there could be unexpected interactions with other uses of cargo, like cargo test
in the unit tests task.
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.
LGTM
`buildsys` no longer depends on BUILDSYS_VARIANT, so it's OK to have a task that tells cargo to build everything in the workspace. Since various scripts might depend on the "latest" symlink created by the "build-variant" task, create the same links in the "build-all" task. Signed-off-by: Ben Cressey <[email protected]>
31cd9b1
to
350b5fb
Compare
⬆️ force push to update |
Issue number:
N/A
Description of changes:
This is a follow-up to #282 that removes the last of
buildsys
's dependencies onBUILDSYS_VARIANT
and friends. That in turn makes it possible to add a "build-all" target toMakefile.toml
, which will help speed up CI runs.(Note that there's still no "publish-all" target;
pubsys
continues to rely onBUILDSYS_VARIANT
to know what to publish.)Testing done:
For x86_64:
And for aarch64:
Terms of contribution:
By submitting this pull request, I agree that this contribution is dual-licensed under the terms of both the Apache License, version 2.0, and the MIT license.