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

buildsys: add CI/CD hack, fix mtime for external files #345

Merged
merged 2 commits into from
Aug 30, 2024

Conversation

bcressey
Copy link
Contributor

Issue number:
N/A

Description of changes:
Two buildsys changes, one potentially controversial, one hopefully not.

The first is the BUILDSYS_CICD_HACK variable - naming suggestions wanted! - to allow cargo to recreate crates without actually running expensive package builds. It's a hack because it assumes the build directory is fully populated; if it's not, then bad things will happen the next time a package is rebuilt.

The second fixes an edge case related to external files or bundled Go modules; these can end up being tracked by cargo for changes, so we need to ensure that their modification time is no later than the start of the build. Cargo.toml is already tracked for changes and is a useful sentinel for this purpose.

Testing done:
Tested in bottlerocket-os/bottlerocket-core-kit#67. With both changes, I was able to reuse cached package builds, touch the files modified by a pull request, and observe that only the affected packages were rebuilt.

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.

Copy link
Contributor

@cbgbt cbgbt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀

tools/buildsys/src/args.rs Outdated Show resolved Hide resolved
For local builds, the detection logic around when to rebuild crates
works fairly well outside of edge cases like upgrading Rust.

However, the same is not true for reusing cached builds with CI/CD.
`cargo` relies on file modification times, and Git does not preserve
these across checkouts, making it difficult to get unmodified files
back to the state that `cargo` expects in order to avoid a rebuild.
In addition, the fingerprint that `cargo` assigns for a given crate
is not consistent across checkouts or changes to the Rust toolchain,
which again leads to unnecessary rebuilds.

To sidestep these challenges, add a `BUILDSYS_CICD_HACK` environment
variable that causes `buildsys` to exit before launching any Docker
builds, and after downloading external files and emitting the change
detection commands. This assumes that the build output directory is
complete and current, which is not possible to prove without letting
the build run. If that assumption holds, then the result is that the
crates produced by `cargo` will be regenerated and will be primed to
track subsequent changes.

Signed-off-by: Ben Cressey <bcressey@amazon.com>
External files may end up listed in a package's spec in some cases,
such as patches or the generated Go module bundles.

These files are created at arbitrary times relative to the start of
the build, since they may take a long time to download or prepare.
This can cause spurious rebuilds since `cargo` assumes that any file
tracked for changes will not have been modified after the file that
stores output for the build.

To avoid these rebuilds, set the modification time for external files
and generated bundles to match the `Cargo.toml` manifest.

Signed-off-by: Ben Cressey <bcressey@amazon.com>
@bcressey
Copy link
Contributor Author

First force push handles the rebase.

Second force push adds some documentation, and adjusts the argument parsing to take a bool instead of an string value that's ignored.

Usage: buildsys build-package ...
...
      --cicd-hack
          cicd_hack is used to suppress builds from running after all the cargo-related metadata is emitted. This allows cargo to create a fresh crate, and assumes that the corresponding build artifacts are already present. It is intended for use in a CI/CD scenario where some other process populates the build directory from a cache. Other uses may lead to unexpected build failures that are difficult to troubleshoot [env: BUILDSYS_CICD_HACK=]

Testing:

❯ cargo run --bin buildsys -- build-package ...
[tools/buildsys/src/main.rs:131:5] args = BuildPackageArgs {
...
    common: Common {
        ...,
        cicd_hack: false,
    },
}

❯ cargo run --bin buildsys -- build-package --cicd-hack ...
[tools/buildsys/src/main.rs:131:5] args = BuildPackageArgs {
...
    common: Common {
        ...,
        cicd_hack: true,
    },
}

❯ BUILDSYS_CICD_HACK=true cargo run --bin buildsys -- build-package ...
[tools/buildsys/src/main.rs:131:5] args = BuildPackageArgs {
...
    common: Common {
        ...,
        cicd_hack: true,
    },
}

❯ BUILDSYS_CICD_HACK=false cargo run --bin buildsys -- build-package ...
[tools/buildsys/src/main.rs:131:5] args = BuildPackageArgs {
...
    common: Common {
        ...,
        cicd_hack: false,
    },
}

@bcressey bcressey merged commit 82c19cd into bottlerocket-os:develop Aug 30, 2024
1 check passed
@bcressey bcressey deleted the cicd-hack branch August 30, 2024 22:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants