better variant support in buildsys #2339
Conversation
bcressey
force-pushed
the
buildsys-bconds
branch
2 times, most recently
from
aaf664f
to
6baa2f8
Compare
|
Rebase with no changes to confirm that the |
There was a problem hiding this comment.
The failing tests are due to a clippy warning for testsys. Should be gone with a rebase including 5865114
I was thinking about possible ways to shave off unnecessary rebuilds by adding even finer grained control, like define sets of platforms (e.g. [metal], [aws, vmware]) and only rebuilding at transitions between these sets. But that is probably adding a lot of unnecessary complexity for rather small gain in build times, while also reducing flexibility. Any thoughts on that?
I like that idea. I think it might be worth trying to see how much time that could save. |
This would be pretty trivial to do with serde, I think, just a matter of deserializing to a list of platforms (etc) rather than a string. One my of back pocket changes is disabling i8042 altogether for the aws platform, since it keeps showing up as a potential 500+ ms stall at boot, and it isn't used for anything. It might be needed for the VMware console though. Lustre is in the opposite category, a driver that's aws-only that could usefully be disabled for other platforms. Xen might be another candidate. So I see specializing the kernel config for these use cases as somewhat desirable or inevitable, which means the build time savings would disappear. But again, the added complexity isn't very much. |
| && echo "%bcond_without $(V=${VARIANT_PLATFORM,,}; echo ${V//-/_})_platform" > .bconds \ | ||
| && echo "%bcond_without $(V=${VARIANT_RUNTIME,,}; echo ${V//-/_})_runtime" >> .bconds \ | ||
| && echo "%bcond_without $(V=${VARIANT_FAMILY,,}; echo ${V//-/_})_family" >> .bconds \ | ||
| && echo "%bcond_without $(V=${VARIANT_FLAVOR:-no}; V=${V,,}; echo ${V//-/_})_flavor" >> .bconds \ |
There was a problem hiding this comment.
I saw that this will end up as with_nvidia_flavor for NVIDIA variants, and one of its uses is to include driverdog. I wonder if we should be proactive and instead of doing with_nvidia_flavor we could have something more generic for future hardware integrations (AMD, Inferentia, etc).
There was a problem hiding this comment.
I'd prefer to wait until we have some other flavors to figure out what this should look like. GPU support is a likely source of "flavor" but we could have others, like a FIPS flavor, that don't have a hardware component.
Parse the variant tuple into its components with the standard module for that purpose, and pass each component into the build environment, to avoid the need for ad-hoc parsing in spec files. Signed-off-by: Ben Cressey <bcressey@amazon.com>
bcressey
force-pushed
the
buildsys-bconds
branch
from
6baa2f8
to
8520115
Compare
The previous definition of variant sensitivity treated any change to the variant as requiring a rebuild. However, packages might depend on only one component of the variant tuple. For example, the kernel package might conditionally enable some modules for the "aws" platform, and a different set for the "metal" platform. To prevent unnecessary rebuilds, packages are now allowed to specify which component, or group of components, they depend upon. Signed-off-by: Ben Cressey <bcressey@amazon.com>
Kernel support for various hardware depends on the target platform, and not other attributes of the variant like the orchestrator agent. Signed-off-by: Ben Cressey <bcressey@amazon.com>
This allows package specs to reference particular components of the variant, without duplicating the parsing logic. Signed-off-by: Ben Cressey <bcressey@amazon.com>
Leverage the newly-added variant platform macro to always include platform-specific kernel configuration files, and ensure these files exist even if empty. Signed-off-by: Ben Cressey <bcressey@amazon.com>
This allows RPM spec files to conditionalize parts of the build in an idiomatic way, using `with` tests rather than string comparisons. Signed-off-by: Ben Cressey <bcressey@amazon.com>
Leverage the new build conditionals to clean up the various string comparisons and to make it easier to add new conditional sections. Signed-off-by: Ben Cressey <bcressey@amazon.com>
bcressey
force-pushed
the
buildsys-bconds
branch
from
8520115
to
5a24633
Compare
|
above force push takes away the redundant "take" |
| @@ -9,6 +9,7 @@ publish = false | |||
| exclude = ["README.md"] | |||
|
|
|||
| [dependencies] | |||
| bottlerocket-variant = { version = "0.1", path = "../../sources/bottlerocket-variant" } | |||
There was a problem hiding this comment.
yay. but also weird. do we have tools crates depending on sources crates and sources crates depending on tools crates? should we consider having a common directory someday?
Issue number:
Closes #2211
Description of changes:
This builds on the variant standardization work done in #2134 in a few ways.
Packages can now specify different levels of variant sensitivity; for example, they may only need to be rebuilt when the platform changes, rather than when any component changes.
Packages can make use of a new set of variant-related macros, such as
%{_cross_variant_platform}, along with the existing%{_cross_variant}macro.Packages can also take advantage of automatically generated build conditionals to handle conditional logic in the idiomatic way for RPM spec files. This is similar in spirit to
cfgattributes emitted for Rust builds.Testing done:
Built
aws,vmware, andmetalvariants.As expected, the kernel package emitted this
cargodirective:Confirmed that the correct files were included in
os:static-podsfor*-k8s-*cfsignalandshibakenforaws-*plutoforaws-k8s-*ecs-settings-applierfor*-ecs-*driverdogfor*-nvidiaTerms 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.