compatible: Adapt collect-bases step to parse platforms keyword
#250
Conversation
lucasgameiroborges
changed the title
platforms keywordplatforms keyword
lucasgameiroborges
requested review from
carlcsaposs-canonical and
taurus-forever
| @@ -36,7 +36,7 @@ def get_bases(*, craft_: craft.Craft, yaml_data): | |||
|
|
|||
| For snaps & rocks, the Ubuntu version is the same for all architectures. | |||
| """ | |||
| if craft_ is craft.Craft.ROCK: | |||
| if craft_ is craft.Craft.ROCK or "platforms" in yaml_data: | |||
There was a problem hiding this comment.
suggestion (non-blocking): I got a little worried of the fact that we may be looking for this string in the whole document. Turns out that yaml_data is a dict as this point. Would you mind adding a type hint in the signature?
There was a problem hiding this comment.
I am not confident to LGTM this (lack of experise here).
The code is OK, but changes can bite. @dragomirp do you see problems here?
The best case is to review with Carl.
P.S. I am worried about artifact name changes. Some workflow may expect -amd64.charm to fetch... and will start failing.
|
Will give a more thorough usage of this during the rest of the week, it can wait for @carlcsaposs-canonical to return! |
There was a problem hiding this comment.
IMHO it's best to wait for @carlcsaposs-canonical. Changes look good, but we should most probably also update charmcraftcache-hub in lockstep.
There was a problem hiding this comment.
I would suggest waiting as @dragomirp said. platforms keyword is the new syntax across the board for base 24.04 on charms, and there will be changes required to accommodate base in addition or instead of bases.
24.04 support for charms looks like this, no support for bases yet:
base: [email protected]
platforms:
amd64:
|
Tried the fix, and as @taurus-forever, it's failing to find the packed charm: ci error |
Hey @zmraul , I think that's because you should also update the |
Seems like same issue is still popping up |
Yep, thanks for testing, I will investigate! |
There was a problem hiding this comment.
Suggest using the approach in #253 instead to enable temporary testing of 24.04-based charms
Long term, currently thinking that next steps will be:
- stabilize charmcraft 3 (probably by limiting to poetry plugin)
- switch our charms, dpw, ccc, and ccc-hub to charmcraft 3 while only supporting the ST124 shorthand syntax (see https://github.com/canonical/charmcraftst124)
- when st124 is added to the crafts, deprecate https://github.com/canonical/charmcraftst124 and switch dpw snap/rock support to ST124 shorthand syntax only
platformskeyword for snaps and charms (like what's already in place for rocks).build_charmhelper to the new charmcraft 3.x naming:<charm>_<arch>.charmTest workflows: