bake: allow override of list variables via environment #3161
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Roughly speaking, if a bake variable is a list with uniform type, it can be overridden via environment variable. The environment variable must be a CSV string whose fields can be coerced to that of the list. Signed-off-by: Roberto Villarreal <rrjjvv@yahoo.com>
|
Closing in favor of #3167 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Roughly speaking, if a bake variable is a list with uniform type, it can be overridden via environment variable. The environment variable must be a CSV string whose fields can be coerced to that of the list.
Resolves #3160
I decided to proactively create a PR (despite my original comments in the issue) since I thought it unlikely to be outright rejected (given it was marked as a TODO), and any feedback or requested changes might be easier and/or more productive alongside an actual proposed implementation with test cases.
The diff looks artificially large due to extracting the conversion logic into its own function. There's no obvious reason the primitives in a list should be processed differently than a primitive on its own, so I thought it'd be preferred to do the refactor rather than copy/pasting that logic into the new tuple/list branches. The actual functionality for the three primitives remains unchanged.
While not the longest in the file, my new test method is among the longest. I thought it more important to match the style of existing tests, using comments where I'd typically use a subtest. And speaking of tests, there was one integration test that did not pass for me on a pristine checkout. After my change, all existing test continue to pass except for that same one that failed prior to my change. It's definitely unrelated to my change and has the feel of something that a core maintainer might not see due to having a unique setup (or maybe using different procedures to build). Specifically,
buildx/tests/version.go
Line 48 in 2eaea64
If this isn't actually expected/anticipated, I'd be happy to dig and/or provide any other details you need.