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: drop variant arg for variant subcommands #369

Merged
merged 1 commit into from
Sep 12, 2024

Conversation

bcressey
Copy link
Contributor

Issue number:
N/A

Description of changes:
Fixes the repack-variant task by using the common cargo_manifest_dir arg instead of the redundant variant arg.

Testing done:
Confirmed that build-variant, repack-variant, and build-all tasks worked.

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.

The switch from relying on BUILDSYS_VARIANT to CARGO_MANIFEST_DIR did
not take into account the case where `buildsys` is not executed by
`cargo`, and hence CARGO_MANIFEST_DIR is not set in the environment.

This broke the repack-variant subcommand, which receives the value
from a command-line argument when invoked through Makefile.toml.

In any case, having both `variant` and `cargo-manifest-dir` args that
are controlled by the same environment variable is redundant. Fix the
bug and remove the confusion by using `cargo-manifest-dir` alone to
identify the artifact that `buildsys` is meant to act on.

Fixes: 4526d6e ("buildsys: stop relying on BUILDSYS_VARIANT")

Signed-off-by: Ben Cressey <[email protected]>
@bcressey
Copy link
Contributor Author

Previously, repack-variant would fail with this error:

[cargo-make][1] INFO - Running Task: repack-variant
error: the following required arguments were not provided:
  --variant <VARIANT>

Usage: buildsys repack-variant --name <NAME> --variant <VARIANT> --version-build <VERSION_BUILD> --version-image <VERSION_IMAGE> --image-dir <IMAGE_DIR> --arch <ARCH> --cargo-metadata-path <CARGO_METADATA_PATH> --root-dir <ROOT_DIR> --state-dir <STATE_DIR> --version-full <VERSION_FULL> --cargo-manifest-dir <CARGO_MANIFEST_DIR> --sdk-image <SDK_IMAGE> --tools-dir <TOOLS_DIR>

That's because the task invokes buildsys this way:

buildsys repack-variant --cargo-manifest-dir "variants/${BUILDSYS_VARIANT}"

But it doesn't also set CARGO_MANIFEST_DIR in the environment, so the variant arg for the subcommand wasn't getting set.

I opted to simplify the handling in buildsys rather than adjusting the behavior in Makefile.toml.

@bcressey bcressey merged commit 5c85464 into bottlerocket-os:develop Sep 12, 2024
1 check passed
@bcressey bcressey deleted the fix-repack-variant branch September 12, 2024 20:07
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.

3 participants