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

install: Drop support for old skopeo #265

Merged
merged 2 commits into from
Jan 17, 2024

Conversation

cgwalters
Copy link
Collaborator

Let's just hard require a skopeo that can fetch from containers-storage. Motivated by #263 which was moving this code around.

@github-actions github-actions bot added the area/install Issues related to `bootc install` label Jan 17, 2024
Copy link
Member

@vrothberg vrothberg left a comment

Choose a reason for hiding this comment

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

The test claims that skopeo is too old.

@@ -927,6 +890,9 @@ async fn prepare_install(

let skopeo_supports_containers_storage = skopeo_supports_containers_storage()
.context("Failed to run skopeo (it currently must be installed in the host root)")?;
if !skopeo_supports_containers_storage {
anyhow::bail!("skopeo is too old");
Copy link
Member

@vrothberg vrothberg Jan 17, 2024

Choose a reason for hiding this comment

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

I think the skopeo_supports_containers_storage fn should error out with a message indicating the current / expected version of Skeop. "too old" will put the user on a journey figuring out the requirement.

@cgwalters
Copy link
Collaborator Author

The test claims that skopeo is too old.

Ah yes, ubuntu. I will look at excising this test.

@cgwalters
Copy link
Collaborator Author

OK, I did some tweaking of the GH/ubuntu CI to build skopeo from git, which will also conveniently give us some integration testing as things change there.

@rhatdan
Copy link
Member

rhatdan commented Jan 17, 2024

LGTM

@rhatdan
Copy link
Member

rhatdan commented Jan 17, 2024

@vrothberg PTAL

So that we can drop support for the hacky "copy to temporary OCI"
code.

Signed-off-by: Colin Walters <walters@verbum.org>
Let's just hard require a skopeo that can fetch from `containers-storage`.
Motivated by containers#263 which
was moving this code around.

Signed-off-by: Colin Walters <walters@verbum.org>
@cgwalters
Copy link
Collaborator Author

I think the comments here are basically addressed.

@cgwalters cgwalters merged commit 769fc7e into containers:main Jan 17, 2024
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/install Issues related to `bootc install`
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants