Skip to content
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

feat: use debian arch in platforms #380

Merged
merged 3 commits into from
Oct 20, 2023

Conversation

tigarmo
Copy link
Collaborator

@tigarmo tigarmo commented Oct 18, 2023

This commit puts Rockcraft in line with other craft tools: the
architectures listed in the platforms are in Debian format, and we
convert when necessary (such as when fetching images from a Docker
registry).

As a side-effect, we also drop the "build variant" notion from the
platforms - currently this variant is only used when fetching/creating
images, so keep the variant logic localized to rockcraft.oci.

Fixes #370, #358

@tigarmo tigarmo force-pushed the CRAFT-2106-platform-deb-arch branch 2 times, most recently from 0b765a5 to 9617a1f Compare October 18, 2023 18:41
This commit puts Rockcraft in line with other craft tools: the
architectures listed in the platforms are in Debian format, and we
convert when necessary (such as when fetching images from a Docker
registry).

As a side-effect, we also drop the "build variant" notion from the
platforms - currently this variant is only used when fetching/creating
images, so keep the variant logic localized to rockcraft.oci.
@tigarmo tigarmo force-pushed the CRAFT-2106-platform-deb-arch branch from 9617a1f to 23b64dc Compare October 18, 2023 18:51
@tigarmo tigarmo changed the title wip refactor archs and variants feat: use debian arch in platforms Oct 18, 2023
rockcraft/oci.py Outdated Show resolved Hide resolved
@tigarmo tigarmo marked this pull request as ready for review October 18, 2023 20:18
Copy link
Collaborator

@cjdcordeiro cjdcordeiro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice to see the craft_application doing some of this work :) Nice work.

Just a few notes.

rockcraft/models/project.py Outdated Show resolved Hide resolved
rockcraft/oci.py Outdated Show resolved Hide resolved
rockcraft/oci.py Outdated Show resolved Hide resolved
rockcraft/oci.py Outdated Show resolved Hide resolved
rockcraft/oci.py Outdated Show resolved Hide resolved
rockcraft/oci.py Outdated Show resolved Hide resolved
rockcraft/oci.py Outdated Show resolved Hide resolved
This way both ``project`` and ``oci`` can make use of it
Copy link
Collaborator

@cjdcordeiro cjdcordeiro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

noice!! very clean.

I left a few non-blocking nitpicks.

Despite the approval, I'd still recommend double checking https://github.com/canonical/rockcraft/pull/380/files#r1364486923 just to be sure Umoci is working well for non-amd64 archs

rockcraft/architectures.py Show resolved Hide resolved
rockcraft/oci.py Outdated Show resolved Hide resolved
rockcraft/oci.py Outdated Show resolved Hide resolved
rockcraft/oci.py Outdated Show resolved Hide resolved
@tigarmo
Copy link
Collaborator Author

tigarmo commented Oct 20, 2023

@cjdcordeiro regarding the umoci arch: as we discussed, I updated the code to use GOARCH as that is most likely the expected format (based on umoci's code and the OCI spec) and next week I'll confirm that assumption with a manual test on a non-standard arch

@tigarmo tigarmo merged commit f312d11 into feature/craft-application Oct 20, 2023
14 checks passed
@tigarmo tigarmo deleted the CRAFT-2106-platform-deb-arch branch October 20, 2023 15:01
tigarmo added a commit that referenced this pull request Oct 25, 2023
This commit puts Rockcraft in line with other craft tools: the
architectures listed in the platforms are in Debian format, and we
convert to GOARCH when necessary (such as when fetching images
from a Docker registry or creating empty ones with umoci).

As a side-effect, we also drop the "build variant" notion from the
platforms - currently this variant is only used when fetching/creating
images, so keep the variant logic localized to rockcraft.oci.
tigarmo added a commit that referenced this pull request Oct 26, 2023
This commit puts Rockcraft in line with other craft tools: the
architectures listed in the platforms are in Debian format, and we
convert to GOARCH when necessary (such as when fetching images
from a Docker registry or creating empty ones with umoci).

As a side-effect, we also drop the "build variant" notion from the
platforms - currently this variant is only used when fetching/creating
images, so keep the variant logic localized to rockcraft.oci.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants