Config file input #8
Merged
Conversation
This file contains 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
|
Here's the tools PR: bazeltools/rules_minidock_tools#278 |
johnynek
reviewed
Oct 13, 2023
minidock/container_compose.bzl
Outdated
| @@ -53,6 +54,10 @@ container_compose = rule( | |||
| "layers": attr.label_list( | |||
| providers = [ContainerInfo], | |||
| ), | |||
| "external_config": attr.label( | |||
There was a problem hiding this comment.
do we not want a label list here since we can already merge?
There was a problem hiding this comment.
I honestly hadnt considered having more than one file, but we certainly can.
wrobstory
commented
Oct 18, 2023
| expected_labels = { | ||
| "label1": "foo", | ||
| "label2": "bar", | ||
| "external-config-label-1": "extlabel1", |
There was a problem hiding this comment.
This verifies that our external configs are getting merged properly
|
@johnynek alrighty: this is ready to go, it can take the genrule files and merge them properly. |
johnynek
approved these changes
Oct 18, 2023
minidock/container_assemble.bzl
Outdated
| inputs = [merger_config_file] | ||
| for target in composed_external_config.config: | ||
| external_config_files = target.files.to_list() | ||
| inputs = inputs + external_config_files |
There was a problem hiding this comment.
you don't need to linearize here. add_all accepts depsets: https://bazel.build/rules/lib/builtins/Args#add_all
so instead:
transdepsets = [composed_transitive_deps]
for target in composed_external_config.config:
external_config_files = target.files
transdepsets.append(external_config_files)
merger_args.add("--external-config-path").add_all(external_config_files)
merger_input = depset([merger_config_file], transitive = transdepsets)There was a problem hiding this comment.
oh intersting, i didnt know depset had an append, updating
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.
We'd like to do be able to capture things from the environment/at runtime and have them merged in to the metadata of our docker container. One way we can do this is what you see in the BUILD.bazel file change below: we could have our
external_config_filegrab the git commit, git branch, etc and then have that merged in (I will probably update this PR to do just that, but for now its a static file).We will then pass that to the merger app, which will merge it alongside the various config that we layer into the composition.
I was quite torn on exactly where to specify this, but container_compose felt like the right place for now.
This will be WIP until I can merge the accompanying PR in rules_minidock_tools, which I will link in a moment.