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

Use bashbrew to gather the canonical list of supported suites #164

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

tianon
Copy link
Member

@tianon tianon commented Sep 3, 2024

This is an attempt to help codify #162, and make sure that it's correct / mostly automated. I'd love to figure out a clean way to help @docker-library-bot be able to make these kinds of updates automatically, but that's a much larger/more complicated problem.

I'm not 100% sold on it, especially how fiddly and frankly unapproachable my filtering method is.

@tianon tianon marked this pull request as draft September 3, 2024 22:21
@tianon
Copy link
Member Author

tianon commented Sep 9, 2024

Heh, you think it's OK as-is, @yosifkit? I'm not opposed to getting it in, especially since the risk is very low (we'll only ever use it locally when making PRs like #162 as-is, which makes it pretty safe overall).

Copy link

@whalelines whalelines left a comment

Choose a reason for hiding this comment

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

A couple questions

|
latest | .*stable | devel | rolling | testing # stable/development/rolling aliases
|
.* - .* # anything with a hyphen

Choose a reason for hiding this comment

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

I know the "x" option causes the regular expression engine to ignore whitespace, but is there a reason to not just have .*-.* or even just - here?

Copy link
Member Author

Choose a reason for hiding this comment

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

I thought .*-.* looked a little compressed and this helped it be more obvious, personally, but I don't feel strongly about it (especially with the comment out to the side which makes it pretty obvious 😂) -- we can't do just - because of the :+$ anchoring outside the parenthesis here (we're matching the full "tag" portion of the image references explicitly with this).

'
)"
# TODO expand this and do our supported architectures detection here too, while we've already done the lookups? Ubuntu version number lookups via this method too? (unfortunately we can't map Debian codenames to aliases like "stable" this way)
eval "distsSuites=( $dists )"

Choose a reason for hiding this comment

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

Is this safer than mapfile/readarray?

Copy link
Member Author

Choose a reason for hiding this comment

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

It's more reliable in my experience (and certainly no less safe).

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.

3 participants