-
Notifications
You must be signed in to change notification settings - Fork 11
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
Add tox env for noble #174
Conversation
tox.ini
Outdated
# 22.04 is the default so we don't need to restore anything after | ||
22.04: sh -c "ln -srf {toxinidir}/charmcraft-22.04.yaml {toxinidir}/charmcraft.yaml && charmcraft pack --project-dir={toxinidir}" | ||
24.04: sh -c "ln -srf {toxinidir}/charmcraft-24.04.yaml {toxinidir}/charmcraft.yaml && charmcraft pack --project-dir={toxinidir}; ln -srf {toxinidir}/charmcraft-22.04.yaml {toxinidir}/charmcraft.yaml" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about making these do:
- backup current charmcraft.yaml
- make their own charmcraft.yaml pointing to the right thing
- pack
- restore charmcraft.yaml
?
It feels odd that 22.04
and 24.04
have different properties around their statefulness. Agreed that it'll work atm, but this is an easy thing for someone to misuse without realizing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure what's the benefit of this approach.
- If we symlink then if someone interrupts the 24.04 and
git add .
then we'd see a small diff (the symlink changed from 22.04 to 24.04). - If we use a third copy of the yaml + untracked backup, then if someone
git add .
then we'd see a slightly larger diff (the charmcraft.yaml will have an entire section replaced).
A symlink seems cleaner. What am I missing?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not against a symlink, just the asymmetry between the two versions:
- 22.04 changes the charmcraft.yaml to the current case, packs
- 24.04 changes the charmcraft.yaml to the current case, packs, and then back to a different case
They don't feel equivalent enough to bundle together. They'll work atm, but someone in future is going to miss that subtlety. What I'm proposing is:
- X: backs up charmcraft.yaml, changes charmcraft.yaml to X, packs, restores backed up charmcraft.yaml
That way all versions of the environment work the same.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
An alternative would be have only a pack-24.04
environment (no 22.04). That feels more suitable imo, since the idea is packing for 24.04 is currently a special, manual case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hah, I see @lucabello asked about this too. I like his original suggestion (or, doing the same thing but with backing up symlinks).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Huh, yeah, might as well drop the 22.04 option. Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm, but I'd rework the tox env to either work the same for all bases or to just implement a single pack-24.04
env
Issue
We need to release a noble (24.04) revision, which only charmcraft v3 can do: with charmcraft 2.x, we'd get for noble:
With charmcraft 3, if we try to add it to the existing list of build-on, charmcraft errors out with:
because charmcraft v3 won't pack noble when charmcraft.yaml has the old schema.
Solution
charmcraft-22.04.yaml
, which is identical to before;charmcraft-24.04.yaml
, with the newbase
schema.charmcraft.yaml
so existing github CI still works as before.Fixes #155.
Drive-by fixes:
Context
Testing Instructions
Upgrade Notes