treefile: allow ${releasever} in more keys #1848
Conversation
|
I would suggest to have a single map with all the variables, and substitute them all at once. If it can be helpful, I was recently going through a similar usecase (not sure if it has the exact same semantics) for which I put together https://docs.rs/envsubst (example of a consumer: https://github.com/coreos/zincati/pull/21/files). |
r4f4
force-pushed
the
releasever
branch
2 times, most recently
from
1a60ec4 to
11a5baf
Compare
|
Thank you for the feedback. I see I'm in the right direction so I'll work on a functional and tested PR. |
|
It seems I can't rely on |
r4f4
changed the title
Yeah, that's the idea, see b46fc35 (which I got from |
|
Ah sorry so I just thought about this more carefully; this is effectively undoing the change in #1848 (comment) right? So we shouldn't do the changes to pass the releasever from the dnf context down; specifically we want a semantic where e.g. someone using coreos-assembler isn't unintentionally relying on the version of Fedora that the assembler container happens to be using. I also think we want to do this processing as a final pass on the merged treefile rather than doing it per parse. This way we have the (IMO) expected semantics where one could do: And have that releasever used. |
|
☔ The latest upstream changes (presumably dbf28ac) made this pull request unmergeable. Please resolve the merge conflicts. |
Besides allowing ${releasever}, only do the substitution as the final
pass after merging the treefiles for all the keys (currently ${basearch}
and ${releasever}) instead of doing it per parse. This way we have the
expected semantics where one could do:
```
include: "fedora-coreos.yaml"
releasever: "42"
```
and have that releasever used.
Fixes coreos#1809
Signed-off-by: Rafael Fonseca <[email protected]>
|
Improved commit message and changed the substitution to happen after the merges as suggested. |
|
Awesome, thanks! |
Besides allowing ${releasever}, only do the substitution as the final
pass after merging the treefiles for all the keys (currently ${basearch}
and ${releasever}) instead of doing it per parse. This way we have the
expected semantics where one could do:
```
include: "fedora-coreos.yaml"
releasever: "42"
```
and have that releasever used.
Fixes #1809
Signed-off-by: Rafael Fonseca <[email protected]>
Closes: #1848
Approved by: cgwalters
|
💔 Test failed - status-atomicjenkins |
|
@rh-atomic-bot retry |
|
☀️ Test successful - status-atomicjenkins |
Fixes #1809
Is this the right idea?