Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
ci: mirror ubuntu:22.04 to ghcr #135530
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
ci: mirror ubuntu:22.04 to ghcr #135530
Changes from all commits
04295f6
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Normally I'm a fan of rewriting complex shell logic into Python or Rust, but in this case, it's literally
curl github > crane && crane ...
, right? I would just include this in the YAML file directly, this file seems unnecessarily complex for the logic that it does. Since we just have a single image now, we don't even need to write a bash for loop :)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'll close this PR and open a new one so that we don't loose the python code in case we need it another time.
Anyway my opinion is that every bash script started with the thought "oh, it's simple, I'll just write it in bash", and than slowly the complexity increased, so I thought to start in python directly and try to do better error reporting, comments and using a temporary directory for the download so that the current directory isn't full of other useless files.
Plus I thought that the code could become complex in the case where:
This looks complex, but for example we could extract the functions to common modules that could be shared among other CI scripts and so in the end the code will read like plain english (similar to the main function)
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.
Right, I definitely agree that once it becomes complex, it might be a good idea to rewrite. But if it's very straightforward in Bash, I would keep it there, until the complexity balloons. Tbh, I would probably do just
if it works :) But maybe crane manages some extra things on top.
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.
yeah I haven't checked the crane source code 👍
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 tried to docker pull and push. The difference is that crane syncs all available architectures.
For example, compare
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 see, thanks for trying! Well, for now we only need x64, so I'd be fine with just staying with the simplest option and just doing pull/push, which is easy to understand (vs using an external tool). We can switch in the future if we also need to mirror ARM images.
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 think it's still useful to mirror all architectures: https://github.com/rust-lang/rust/pull/135574/files#r1918185315