Add initial jq-based templating engine #456
Conversation
| {{ if env.version | endswith("-rc") then ( -}} | ||
| case "$GHOST_VERSION" in *-alpha* | *-beta* | *-rc*) installCmd="$installCmd --channel next" ;; esac; \ | ||
| {{ ) else "" end -}} |
There was a problem hiding this comment.
We could probably do this unilaterally for the -rc and not have the case anymore, but avoiding the change for now is fine too 👍
apply-templates.sh
Outdated
| [ -f versions.json ] # run "versions.sh" first | ||
|
|
||
| cd "$(dirname "$(readlink -f "$BASH_SOURCE")")" |
There was a problem hiding this comment.
We do the -f as a rough sanity check so that we don't have to do the cd 👀
(otherwise they're checking different directories entirely 😅)
| [ -f versions.json ] # run "versions.sh" first | |
| cd "$(dirname "$(readlink -f "$BASH_SOURCE")")" | |
| [ -f versions.json ] # run "versions.sh" first |
There was a problem hiding this comment.
I kept the cd as a defense in depth for the rm -rf $version to not accidentally delete something unexpected. But yeah, the order is wrong.
| if [ -d "$version" ]; then | ||
| rm -rf "$version" | ||
| fi |
There was a problem hiding this comment.
| if [ -d "$version" ]; then | |
| rm -rf "$version" | |
| fi | |
| rm -rf "$version" |
-fwill make sure this doesn't fail when it doesn't exist,- and if it exists but isn't a directory, it would sometimes fail later (depending on the next conditional) so it's easier to manage/think about the drift if we just enforce the state at this level
generate-stackbrew-library.sh
Outdated
| | xargs -r bashbrew cat --format '["{{ .RepoName }}:{{ .TagName }}"]="{{ join " " .TagEntry.Architectures }}"' | ||
| )" | ||
| eval "declare -g -A parentRepoToArches=( $parentRepoToArchesStr )" | ||
| : "${BASHBREW_LIBRARY:-https://github.com/docker-library/official-images/raw/HEAD/library}/" |
There was a problem hiding this comment.
Without an export this doesn't apply, and we don't actually support a URL path in BASBHREW_LIBRARY (we assume it points to a directory), which is why it was implemented the way it was in the previous version of this function. 😅
There was a problem hiding this comment.
Ah, I was over aggressive on the removal of the find + awk. After testing with BASHBREW_LIBRARY set and unset, I have made an adjustment (coming soon).
generate-stackbrew-library.sh
Outdated
| getArches 'ghost' | ||
| parentArchesJson="$(getArchesJson 'ghost')" | ||
|
|
||
| commit="$(fileCommit '*/**')" # |
There was a problem hiding this comment.
Was this # for a TODO of some kind or something? 😅
generate-stackbrew-library.sh
Outdated
| jq --raw-output '.[].variants[].from' versions.json \ | ||
| | sort -u \ |
There was a problem hiding this comment.
| jq --raw-output '.[].variants[].from' versions.json \ | |
| | sort -u \ | |
| jq --raw-output '[ .[].variants[].from ] | unique[]' versions.json \ |
generate-stackbrew-library.sh
Outdated
|
|
||
| jq --raw-output '.[].variants[].from' versions.json \ | ||
| | sort -u \ | ||
| | xargs -r bashbrew cat --format '{ "{{ .RepoName }}:{{ .TagName }}": {{ json .TagEntry.Architectures }} }' \ |
There was a problem hiding this comment.
| | xargs -r bashbrew cat --format '{ "{{ .RepoName }}:{{ .TagName }}": {{ json .TagEntry.Architectures }} }' \ | |
| | xargs -r bashbrew cat --format '{ {{ join ":" .RepoName .TagName | json }}: {{ json .TagEntry.Architectures }} }' \ |
generate-stackbrew-library.sh
Outdated
| jq --raw-output '.[].variants[].from' versions.json \ | ||
| | sort -u \ | ||
| | xargs -r bashbrew cat --format '{ "{{ .RepoName }}:{{ .TagName }}": {{ json .TagEntry.Architectures }} }' \ | ||
| | jq --compact-output --null-input '[ inputs ] | add' |
There was a problem hiding this comment.
This is just slurp 😅
| | jq --compact-output --null-input '[ inputs ] | add' | |
| | jq --compact-output --slurp 'add' |
generate-stackbrew-library.sh
Outdated
| EOE | ||
| done | ||
| done | ||
| jq --raw-output --argjson parentArches "$parentArchesJson" ' |
There was a problem hiding this comment.
IMO this is long enough that we should move the body of it into a separate generate-stackbrew-library.jq file so we get nice syntax highlighting (from our editors and GitHub). 😇
exec jq \
--raw-output \
--argjson parentArches "$parentArchesJson" \
--from-file generate-stackbrew-library.jq \
versions.json \
--args -- "$@"
generate-stackbrew-library.sh
Outdated
| # returns a list of tags for this variant | ||
| # usage: variant_tags("version"; [sortedVersionList]; "variant"; [sortedVariantList]) | ||
| # e.g. variant_tags("1.2.3.4"; ["1.2.3.4", "0.1.2.3"]; "trixie"; ["trixie", "alpine3.22"]) -> ["1.2.3.4-trixie", "1.2.3-trixie", ...] | ||
| def variant_tags($version; $versionListSorted; $variant; $variantListSorted): |
There was a problem hiding this comment.
Given that each of these are only used once, what's the benefit to having functions over just embedding the logic below? 😅
There was a problem hiding this comment.
I was actually trying to not embed all the logic inline so that it could be easier to understand and simpler to extend to other images. But it has kind of come back into basically one function. I think that it could be improved to be more generic and useful to any image repo we maintain.
There was a problem hiding this comment.
Personally I find it a little harder to read this way and I'm not convinced it will actually be as reusable as we want it to be in practice, but I don't feel strongly about it and I'm willing to give it a chance. 😄 👍
yosifkit
force-pushed
the
jq-template
branch
2 times, most recently
from
95d3768 to
83ce252
Compare
|
I checked this out locally so I could redo your original diff now that it's had some changes/updates, and `bashbrew cat`:$ diff -u <(bashbrew cat ghost) <(bashbrew cat <(./generate-stackbrew-library.sh))
--- /dev/fd/63 2025-10-22 15:15:17.266193532 -0700
+++ /dev/fd/62 2025-10-22 15:15:17.266193532 -0700
@@ -1,22 +1,19 @@
Maintainers: Tianon Gravi <[email protected]> (@tianon), Joseph Ferguson <[email protected]> (@yosifkit), Austin Burdine <[email protected]> (@acburdine)
GitRepo: https://github.com/docker-library/ghost.git
+GitCommit: 83ce25215e22a36c646c0a6165a531f12d92339f
-Tags: 6.5.0, 6.5, 6, latest
-Architectures: amd64, arm32v7, arm64v8, ppc64le, s390x
-GitCommit: f952f7615274894d6efb0fbc765f5368580fb39a
-Directory: 6/debian
+Tags: 6.5.0-bookworm, 6.5-bookworm, 6-bookworm, bookworm, 6.5.0, 6.5, 6, latest
+Architectures: amd64, arm32v7, arm64v8, s390x
+Directory: 6/bookworm
-Tags: 6.5.0-alpine, 6.5-alpine, 6-alpine, alpine
-Architectures: amd64, arm32v6, arm32v7, arm64v8
-GitCommit: f952f7615274894d6efb0fbc765f5368580fb39a
-Directory: 6/alpine
+Tags: 6.5.0-alpine3.22, 6.5-alpine3.22, 6-alpine3.22, alpine3.22, 6.5.0-alpine, 6.5-alpine, 6-alpine, alpine
+Architectures: amd64, arm64v8
+Directory: 6/alpine3.22
-Tags: 5.130.5, 5.130, 5
-Architectures: amd64, arm32v7, arm64v8, ppc64le, s390x
-GitCommit: fdba3d80f50da610007165f5fe46f9b8af69764b
-Directory: 5/debian
+Tags: 5.130.5-bookworm, 5.130-bookworm, 5-bookworm, 5.130.5, 5.130, 5
+Architectures: amd64, arm32v7, arm64v8, s390x
+Directory: 5/bookworm
-Tags: 5.130.5-alpine, 5.130-alpine, 5-alpine
-Architectures: amd64, arm32v6, arm32v7, arm64v8
-GitCommit: fdba3d80f50da610007165f5fe46f9b8af69764b
-Directory: 5/alpine
+Tags: 5.130.5-alpine3.22, 5.130-alpine3.22, 5-alpine3.22, 5.130.5-alpine, 5.130-alpine, 5-alpine
+Architectures: amd64, arm64v8
+Directory: 5/alpine3.22`bashbrew list`:$ diff -u <(bashbrew list ghost | cut -d: -f2) <(bashbrew list <(./generate-stackbrew-library.sh) | cut -d: -f2)
--- /dev/fd/63 2025-10-22 15:16:27.434905448 -0700
+++ /dev/fd/62 2025-10-22 15:16:27.434905448 -0700
@@ -1,14 +1,28 @@
+6.5.0-bookworm
+6.5-bookworm
+6-bookworm
+bookworm
6.5.0
6.5
6
latest
+6.5.0-alpine3.22
+6.5-alpine3.22
+6-alpine3.22
+alpine3.22
6.5.0-alpine
6.5-alpine
6-alpine
alpine
+5.130.5-bookworm
+5.130-bookworm
+5-bookworm
5.130.5
5.130
5
+5.130.5-alpine3.22
+5.130-alpine3.22
+5-alpine3.22
5.130.5-alpine
5.130-alpine
5-alpine |
There was a problem hiding this comment.
Oh hmm, I didn't notice these renames - this implies that we might support more than one version of a given base distribution -- is that an intentional change? (I certainly wasn't intending to support more than one each here 😅)
There was a problem hiding this comment.
Yes and no. I did add variants that could support multiple base distribution versions, but mostly to add extra tags. I don't think we should keep more than one Debian suite or Alpine version per ghost version.
|
This force push includes a partial rewrite of |
| dist: { | ||
| linux: [ | ||
| # list of debian versions, in descending order | ||
| #"trixie", # TODO | ||
| "bookworm", | ||
| empty # trailing comma | ||
| ], | ||
| linuxmusl: [ | ||
| # list of alpine versions, in descending order | ||
| "3.22", | ||
| empty | ||
| | "alpine" + . | ||
| ] | ||
| }[.[0]], |
There was a problem hiding this comment.
Relevant to the above conversation about supporting multiple variants, I think it's fine to leave the data model here able to support multiple, but we should update the code filling it in to make it more clear we won't accept more than one (unless something really exceptional happens, I guess), and so that we don't accidentally forget in the future; maybe something like this (very open to alternatives, but this is a straw man)?
| dist: { | |
| linux: [ | |
| # list of debian versions, in descending order | |
| #"trixie", # TODO | |
| "bookworm", | |
| empty # trailing comma | |
| ], | |
| linuxmusl: [ | |
| # list of alpine versions, in descending order | |
| "3.22", | |
| empty | |
| | "alpine" + . | |
| ] | |
| }[.[0]], | |
| dist: { | |
| # each of these should be a single distro version unless something *really* exceptional happens | |
| linux: [ "bookworm" ], | |
| linuxmusl: [ "alpine3.22" ], | |
| }[.[0]], |
| | capture("^ *\"@img/sharp-(?<name>linux[a-z0-9-]+)\" \"[0-9.]+\"$") | ||
| | .name | ||
| | split("-") |
There was a problem hiding this comment.
Given the immediate split to take off more data, couldn't this instead massage the regex a little so that .name is the exact value we need?
| | capture("^ *\"@img/sharp-(?<name>linux[a-z0-9-]+)\" \"[0-9.]+\"$") | |
| | .name | |
| | split("-") | |
| | capture("^ *\"@img/sharp-(?<dist>linux[a-z]*)-(?<arch>[a-z0-9]+)\" \"[0-9.]+\"$") |
(and then using .dist and .arch instead of .[0] and .[1] below - could even be .dist |= { ... }[.] | .arch |= { ... }[.])
| }[.[1]], | ||
| } | ||
| ) as $item ({}; .[$item.dist[]].arches += [ $item.arch ]) | ||
| | with_entries(.value.arches |= sort) |
There was a problem hiding this comment.
This could just be .[].arches |= sort:
$ jq -nc '{ foo: { bar: [ "c", "b", "a" ] } } | .[].bar |= sort'
{"foo":{"bar":["a","b","c"]}}
$ # still works if we get even more whack:
$ jq -nc '{ foo: { bar: [ "c", "b", "a" ] } } | .[][] |= sort'
{"foo":{"bar":["a","b","c"]}}However, the order should already be deterministic from yarn.lock, so I'm not convinced it's necessary for us to sort here. 😅
| # move alpine variants to the end of the object | ||
| # debian suites and alpine versions are each relatively sorted already | ||
| | map(.value.variants |= ( to_entries | sort_by(.key | contains("alpine")) | from_entries)) |
There was a problem hiding this comment.
It feels weird to munge variants on these objects so late instead of doing so above when we're generating them 😅
I'll add a suggestion above for a potential way to resolve this without an explicit sort 👀
| reduce ( | ||
| inputs | ||
| | capture("^ *\"@img/sharp-(?<name>linux[a-z0-9-]+)\" \"[0-9.]+\"$") | ||
| | .name | ||
| | split("-") | ||
| | { | ||
| dist: { | ||
| linux: [ | ||
| # list of debian versions, in descending order | ||
| #"trixie", # TODO | ||
| "bookworm", | ||
| empty # trailing comma | ||
| ], | ||
| linuxmusl: [ | ||
| # list of alpine versions, in descending order | ||
| "3.22", | ||
| empty | ||
| | "alpine" + . | ||
| ] | ||
| }[.[0]], | ||
| arch: { | ||
| x64: "amd64", | ||
| arm64: "arm64v8", | ||
| arm: "arm32v7", | ||
| s390x: "s390x", | ||
| }[.[1]], | ||
| } | ||
| ) as $item ({}; .[$item.dist[]].arches += [ $item.arch ]) | ||
| | with_entries(.value.arches |= sort) |
There was a problem hiding this comment.
Here's one possibility for fixing the ordering of Debian first, Alpine second without an explicit sort (incorporating the above suggestions too):
| reduce ( | |
| inputs | |
| | capture("^ *\"@img/sharp-(?<name>linux[a-z0-9-]+)\" \"[0-9.]+\"$") | |
| | .name | |
| | split("-") | |
| | { | |
| dist: { | |
| linux: [ | |
| # list of debian versions, in descending order | |
| #"trixie", # TODO | |
| "bookworm", | |
| empty # trailing comma | |
| ], | |
| linuxmusl: [ | |
| # list of alpine versions, in descending order | |
| "3.22", | |
| empty | |
| | "alpine" + . | |
| ] | |
| }[.[0]], | |
| arch: { | |
| x64: "amd64", | |
| arm64: "arm64v8", | |
| arm: "arm32v7", | |
| s390x: "s390x", | |
| }[.[1]], | |
| } | |
| ) as $item ({}; .[$item.dist[]].arches += [ $item.arch ]) | |
| | with_entries(.value.arches |= sort) | |
| reduce ( | |
| inputs | |
| | capture("^ *\"@img/sharp-(?<dist>linux[a-z]*)-(?<arch>[a-z0-9]+)\" \"[0-9.]+\"$") | |
| ) as $item ({ | |
| # this controls the variant ordering | |
| linux: [], # non-Alpine first | |
| linuxmusl: [], # Alpine second | |
| }; .[$item.dist] += [ $item.arch ]) | |
| | with_entries( | |
| select(.value | length > 0) | |
| | .key |= { | |
| # each of these should be a single distro version unless something *really* exceptional happens | |
| linux: [ "bookworm" ][], | |
| linuxmusl: [ "alpine3.22" ][], | |
| }[.] | |
| | .value |= { arches: [ { | |
| x64: "amd64", | |
| arm64: "arm64v8", | |
| arm: "arm32v7", | |
| s390x: "s390x", | |
| }[.[]] ] } # TODO maybe a "// empty" here so we ignore anything we are not explicitly handling? error() would be super reasonable too but would be worth swapping this to ".[] | {...}[.]" or "arches: map(...)" instead so we can include the offending value in the error | |
| ) |
| to_entries | ||
| # sort by version number, descending | ||
| | sort_by(.value.version | split("[.-]"; "") | map(tonumber? // .) ) |
There was a problem hiding this comment.
There's a few instances of weird parenthesis "cuddling" throughout this PR (where a parenthesis and its pair don't have the same whitespace) 😅
I don't have a strong opinion on individual cases whether they should be func(foo) or func( foo ) or (foo(bar)) or ( foo(bar) ) or ( foo( bar ) ), but I do have a strong opinion that matching pairs should have the same whitespace at each end, and otherwise am fine with whatever's most readable/ergonomic (I personally usually default to "tight" unless things look exceptionally hard to read, but that's often my clue that I need newlines instead 😅)
| | map(.value.variants |= ( to_entries | sort_by(.key | contains("alpine")) | from_entries)) | ||
| | from_entries | ||
| '> versions.json |
There was a problem hiding this comment.
| '> versions.json | |
| ' > versions.json |
| )" | ||
|
|
||
| export fullVersion cliVersion | ||
| json="$(jq <<<"$json" -c --argjson doc "$doc" ' |
There was a problem hiding this comment.
| json="$(jq <<<"$json" -c --argjson doc "$doc" ' | |
| json="$(jq <<<"$json" --compact-output --argjson doc "$doc" ' |
| | ( $allVariants | map(select(contains("alpine"))))[0] as $latestAlpine | ||
| | ( $allVariants | map(select(contains("alpine") | not)))[0] as $latestDebian |
There was a problem hiding this comment.
| | ( $allVariants | map(select(contains("alpine"))))[0] as $latestAlpine | |
| | ( $allVariants | map(select(contains("alpine") | not)))[0] as $latestDebian | |
| | first( $allVariants[] | select(contains("alpine")) ) as $latestAlpine | |
| | first( $allVariants[] | select(contains("alpine") | not) ) as $latestDebian |
I think this is actually slightly more efficient inside jq if we redo the .[].variants | keys_unsorted[] inside the first() because it won't complete the full iteration and will actively short-circuit, but readability definitely suffers if we do that 😅
| @@ -0,0 +1,73 @@ | |||
| # latest version is the first one since it is already in version order | |||
| keys_unsorted[0] as $latestMajor | |||
There was a problem hiding this comment.
Is this still true if we have pre-releases? Don't we need to filter them out? ie:
| keys_unsorted[0] as $latestMajor | |
| first(keys_unsorted[] | select(endswith("-rc") | not)) as $latestMajor |
Oh, and doesn't that also have issues if we have, for example, a 7-rc and 7 is null? So this needs to account for the value too?
| keys_unsorted[0] as $latestMajor | |
| first(to_entries[] | select(.value and (.key | endswith("-rc") | not)).key) as $latestMajor |
|
|
||
|
|
||
| # loop over major versions | ||
| | to_entries[] |
There was a problem hiding this comment.
Oh, if we did to_entries sooner, we'd have less iteration above -- basically if this file started with to_entries right off the bat, we'd then have:
to_entries
| first(.[] | select(.value and (.key | endswith("-rc") | not)).key) as $latestMajor
| [ .[].value.variants | keys_unsorted[] ] as $allVariants
| first( $allVariants[] | select(contains("alpine")) ) as $latestAlpine
| first( $allVariants[] | select(contains("alpine") | not) ) as $latestDebian
| .[]
| .key as $major
| .value
...(I guess it really only helps with $latestMajor 🤷 but that's not nothing!)
| # latest version is the first one since it is already in version order | ||
| keys_unsorted[0] as $latestMajor | ||
| # list of all variants in version.json order (debian suites followed by alpine versions, each in descending order) | ||
| | [ .[].variants | keys_unsorted[] ] as $allVariants |
There was a problem hiding this comment.
I guess arguably, this should actually be .[$latestMajor].variants explicitly, right? Because we don't want to "default" any other aliases (like if a pre-release version had a new Alpine or Debian version that the latest release doesn't have yet). That also makes it less useful/meaningful/DRY to have $allVariants because that's a pretty static lookup. 😅
... and yes, this is in direct contrast to my suggestion that we move up the to_entries because then we don't have trivial lookup here unless we save the entire $latestMajor key/value pair instead of just the key 😂
So maybe that'd look something like this?
to_entries
| first(.[] | select(.value and (.key | endswith("-rc") | not))) as $latestMajor
| first( $latestMajor.value.variants[] | select(contains("alpine")) ) as $latestAlpine
| first( $latestMajor.value.variants[] | select(contains("alpine") | not) ) as $latestDebian
| $latestMajor.key as $latestMajor
| .[]
| .key as $major
| .value
...| # e.g. "1.2.3.4" -> [ "1.2.3.4", "1.2.3", "1.2", "1", "" ] | ||
| | ( | ||
| .version | ||
| # TODO improve rc suffix support (e.g. this won't work for -beta or -rc.1 versioned software) |
There was a problem hiding this comment.
Maybe this is really something like if $major | endswith("-rc") then [ ., $major ] else ... instead?
| if $variant == $latestAlpine then | ||
| ($variant | sub("[0-9.]+$"; "")) # "alpine3.22" -> "alpine" | ||
| else empty end, |
There was a problem hiding this comment.
This is a really verbose way to spell "alpine" 😅
Why the sub? 🤔
| if $variant == $latestAlpine then | |
| ($variant | sub("[0-9.]+$"; "")) # "alpine3.22" -> "alpine" | |
| else empty end, | |
| if $variant == $latestAlpine then "alpine" else empty end, |
2 participants
Based on docker-library/php#1052 and related.
Notably this drops architectures that don't have prebuilt
sharplibraries (a dep of ghost that useslibvipsand requires a newer version than is in Alpine or Debian to build sharp).This also rewrites
generate-stackbrew-library.shto use lots ofjqinstead of bash string replacements. Maybe it is better, maybe it is not. This also brings in Debian suite and Alpine version tags (and then we could have more than one suite/version).