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: Pin external references on a release #1208

Merged
merged 2 commits into from
Oct 31, 2024

Conversation

sudo-bmitch
Copy link
Contributor

This pins references to external specs (distribution and runtime) to their latest release on a release of this project.

See the discussion on #1207 for more context.

This pins references to external specs (distribution and runtime) to their latest release on a release of this project.

Signed-off-by: Brandon Mitchell <[email protected]>
@tianon
Copy link
Member

tianon commented Oct 15, 2024

How do we make sure the latest releases include whatever our links are referring to and that we're not implicitly relying on unreleased content?

(Also, if you want to avoid the GitHub API and thus avoid their API limits as well, you might like git ls-remote)

@sudo-bmitch
Copy link
Contributor Author

How do we make sure the latest releases include whatever our links are referring to and that we're not implicitly relying on unreleased content?

That's why we have a 2/3rds super majority voting on the release. 😄

(Also, if you want to avoid the GitHub API and thus avoid their API limits as well, you might like git ls-remote)

Wouldn't that require a clone of the other repository first? Granted that the maintainer doing the release probably has those, but everyone would have a potentially different directory structure and remote names.

If we were to make a lot of requests, this could also inject a token in the curl, but I figured we could roll the dice with only 2 requests. If it fails, the maintainer doing the release can sort it out since this isn't an automated workflow.

@tianon
Copy link
Member

tianon commented Oct 17, 2024

That's why we have a 2/3rds super majority voting on the release. 😄

Ah right, because we'll review the commit that updates all the references, which is a natural place to review that they're accurate with the surrounding text. That tracks; thanks for helping me through that. 😄

Wouldn't that require a clone of the other repository first?

Nope, that's the beauty of git ls-remote! I'll use a container to illustrate more dramatically (so it's really clear I can't possibly have a pre-existing repository or even a local .git):

$ docker run --rm buildpack-deps:scm sh -euc '
git ls-remote https://github.com/opencontainers/runtime-spec.git refs/tags/v[0-9]* \
| cut -d/ -f3 \
| cut -d^ -f1 \
| grep -vF -- - \
| sort -uV \
| tail -1
'
v1.2.0

@tianon
Copy link
Member

tianon commented Oct 17, 2024

In the spirit of your existing script (since sort -V is a GNU-ism), here's a version that only relies on git and jq instead:

$ docker run --rm buildpack-deps:scm git ls-remote https://github.com/opencontainers/runtime-spec.git 'refs/tags/v[0-9]*' | jq -rnR '[ inputs | split("/")[2] | split("^")[0] | select(contains("-") | not) ] | unique_by(ltrimstr("v") | split(".") | map(tonumber? // .))[-1]'
v1.2.0

Split with whitespace (and comments) so it's easier to read/follow:

$ docker run --rm buildpack-deps:scm git ls-remote https://github.com/opencontainers/runtime-spec.git 'refs/tags/v[0-9]*' \
	| jq -rnR '
		[
			inputs
			| split("/")[2] # "commit-hash\trefs/tags/xxx^{}" -> "xxx^{}"
			| split("^")[0] # "xxx^{}" -> "xxx"
			| select(contains("-") | not) # ignore pre-releases
		]
		| unique_by(ltrimstr("v") | split(".") | map(tonumber? // .)) # very very rough version sorting (and dedupe)
		| .[-1] # we only care about "latest" (the last entry)
	'
v1.2.0

set -e
cd "$(dirname $0)/.."

if [ ! -x "$(command -v curl )" ] || [ ! -x "$(command -v jq )" ] || [ ! -x "$(command -v find )" ] || [ ! -x "$(command -v sed )" ]; then
Copy link
Member

Choose a reason for hiding this comment

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

You're going to change the list of commands, but in general, the command built-in is pretty well behaved so this can be somewhat simpler/cleaner too:

Suggested change
if [ ! -x "$(command -v curl )" ] || [ ! -x "$(command -v jq )" ] || [ ! -x "$(command -v find )" ] || [ ! -x "$(command -v sed )" ]; then
if ! command -v curl > /dev/null || ! command -v jq > /dev/null || ! command -v find > /dev/null || ! command -v sed > /dev/null; then

or even:

Suggested change
if [ ! -x "$(command -v curl )" ] || [ ! -x "$(command -v jq )" ] || [ ! -x "$(command -v find )" ] || [ ! -x "$(command -v sed )" ]; then
if ! { command -v curl && command -v jq && command -v find && command -v sed; } > /dev/null; then

Changes for the command and avoiding the github api requested by tianon.
Please send any support requests for that jq command to tianon. :)

Signed-off-by: Brandon Mitchell <[email protected]>
@sudo-bmitch
Copy link
Contributor Author

@tianon is now the forever-maintainer of that jq command. I disavow all responsibility. 😂

$ docker run --rm buildpack-deps:scm git ls-remote https://github.com/opencontainers/runtime-spec.git 'refs/tags/v[0-9]*' \
	| jq -rnR '
		[
			inputs
			| split("/")[2] # "commit-hash\trefs/tags/xxx^{}" -> "xxx^{}"
			| split("^")[0] # "xxx^{}" -> "xxx"
			| select(contains("-") | not) # ignore pre-releases
		]
		| unique_by(ltrimstr("v") | split(".") | map(tonumber? // .)) # very very rough version sorting (and dedupe)
		| .[-1] # we only care about "latest" (the last entry)
	'
v1.2.0

Copy link
Member

@tianon tianon left a comment

Choose a reason for hiding this comment

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

🫡

@sajayantony sajayantony merged commit f8af71e into opencontainers:main Oct 31, 2024
4 checks passed
@sudo-bmitch sudo-bmitch deleted the pr-pin-urls-on-release branch October 31, 2024 21:21
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