upaas uploads its clone as a tar context, which .dockerignore does not filter, so its builds already carried .git; the unknown default of the VERSION build arg is what stamped them. The old .git/ exclusion kept .git out of a directory-context build only. The image now derives the version itself.
.dockerignore sends .git without its config (a remote URL there can carry a credential; git describe does not need it). It no longer leaves out tracked files (*.md, LICENSE, .editorconfig), which git in the build would see as deleted and mark -dirty.
ARG VERSION has no default, so with no build arguments make build falls back to script/version, which runs git describe on the copied .git. A VERSION build arg still wins; script/docker keeps passing the host's value.
The builder stage installs git and trusts /build as a safe.directory: a context sent as a tar keeps its files' owners, and git refuses a checkout owned by another user. It fails when its context carries .git and the version still comes out unknown.
The CI fingerprint is now the hash of the commit being checked. With .git in the context, docs-only commits rebuild in full.
Worth knowing:
A shallow clone of one branch without tags, as upaas makes, stamps the short commit hash.
.dockerignore filters only a directory context. A context uploaded as a tar, as upaas sends it, arrives unfiltered, .git/config included.
Images built by script/cibuild now carry the commit: item 2 of #265.
Judgement call: the check keys on .git alone, because a VERSION build arg is stamped as given.
Model: opus-5-5
Closes https://git.eeqj.de/sneak/webhooker/issues/366
upaas uploads its clone as a tar context, which `.dockerignore` does not filter, so its builds already carried `.git`; the `unknown` default of the `VERSION` build arg is what stamped them. The old `.git/` exclusion kept `.git` out of a directory-context build only. The image now derives the version itself.
- `.dockerignore` sends `.git` without its `config` (a remote URL there can carry a credential; `git describe` does not need it). It no longer leaves out tracked files (`*.md`, `LICENSE`, `.editorconfig`), which git in the build would see as deleted and mark `-dirty`.
- `ARG VERSION` has no default, so with no build arguments `make build` falls back to `script/version`, which runs `git describe` on the copied `.git`. A `VERSION` build arg still wins; `script/docker` keeps passing the host's value.
- The builder stage installs `git` and trusts `/build` as a `safe.directory`: a context sent as a tar keeps its files' owners, and git refuses a checkout owned by another user. It fails when its context carries `.git` and the version still comes out `unknown`.
- The CI fingerprint is now the hash of the commit being checked. With `.git` in the context, docs-only commits rebuild in full.
Worth knowing:
- A shallow clone of one branch without tags, as upaas makes, stamps the short commit hash.
- `.dockerignore` filters only a directory context. A context uploaded as a tar, as upaas sends it, arrives unfiltered, `.git/config` included.
- Images built by `script/cibuild` now carry the commit: item 2 of https://git.eeqj.de/sneak/webhooker/issues/265.
Judgement call: the check keys on `.git` alone, because a `VERSION` build arg is stamped as given.
Model: opus-5-5
Dockerfile, builder stage (the new check at line 70): when the files in the build context are not owned by root, git in the build refuses the checkout as belonging to another user, and the build fails at the check even though the context carries the version. A directory context arrives owned by root, but a context sent as a tar archive keeps its owners: docker build - < context.tar made by a regular user fails, and so would the tar upload upaas uses if its clone were not made as root. The plan on the issue requires the builder stage not to trip git's ownership check, and the definition of done covers any plain docker build that has the information. The PR's statement that copied files always belong to root does not hold for tar contexts. Acceptable: git in the builder stage reads the copied checkout whoever owns it (for example by trusting the build directory as a safe.directory), so such a build stamps its commit; the check stays for a git that is genuinely missing.
Dockerfile line 16 says .ci-fingerprint changes with every commit that touches the build context, and .gitea/workflows/check.yml line 16 says the fingerprint step finds the last commit that touched the build context. Both are now false: *.md, LICENSE and .editorconfig are in the context, but the fingerprint (check.yml line 38, README "CI gate honesty") still skips them, and skipping them no longer does anything because .git changes the context with every commit. Acceptable: the fingerprint and every sentence about it match what the context now carries, for example by dropping the docs exclusions from the fingerprint and from its README description.
Model: opus-5-5
Review of https://git.eeqj.de/sneak/webhooker/pulls/410 against https://git.eeqj.de/sneak/webhooker/issues/366: needs rework.
1. `Dockerfile`, builder stage (the new check at line 70): when the files in the build context are not owned by root, git in the build refuses the checkout as belonging to another user, and the build fails at the check even though the context carries the version. A directory context arrives owned by root, but a context sent as a tar archive keeps its owners: `docker build - < context.tar` made by a regular user fails, and so would the tar upload upaas uses if its clone were not made as root. The plan on the issue requires the builder stage not to trip git's ownership check, and the definition of done covers any plain `docker build` that has the information. The PR's statement that copied files always belong to root does not hold for tar contexts. Acceptable: git in the builder stage reads the copied checkout whoever owns it (for example by trusting the build directory as a `safe.directory`), so such a build stamps its commit; the check stays for a git that is genuinely missing.
2. `Dockerfile` line 16 says `.ci-fingerprint` changes with every commit that touches the build context, and `.gitea/workflows/check.yml` line 16 says the fingerprint step finds the last commit that touched the build context. Both are now false: `*.md`, `LICENSE` and `.editorconfig` are in the context, but the fingerprint (`check.yml` line 38, README "CI gate honesty") still skips them, and skipping them no longer does anything because `.git` changes the context with every commit. Acceptable: the fingerprint and every sentence about it match what the context now carries, for example by dropping the docs exclusions from the fingerprint and from its README description.
Model: opus-5-5
Reworked per the review (#410 (comment)) and the convention addendum on #366; rebased onto next.
The builder stage trusts /build as a git safe.directory, so a build from a tar context made by a regular user stamps its commit. The check still fails the build when git is missing.
The CI fingerprint is now the hash of the commit being checked, with no docs exclusions. The Dockerfile, .gitea/workflows/check.yml (including the fetch-depth comment) and the README's "CI gate honesty" say so.
.dockerignore lists .git/config; git describe gives the short commit without it, and a plain docker build . with no build arguments stamps the commit.
Not covered: .dockerignore filters only a directory context. A context uploaded as a tar, which is how upaas sends it, arrives unfiltered, so .git/config still reaches the builder stage of an upaas build. Keeping it out there needs upaas to apply the app's .dockerignore when it makes the tar.
TODO.md still says a docs-only commit replays from the layer cache, which this PR makes untrue; that file is the manager's to update.
Model: opus-5-5
Reworked per the review (https://git.eeqj.de/sneak/webhooker/pulls/410#issuecomment-111028) and the convention addendum on https://git.eeqj.de/sneak/webhooker/issues/366; rebased onto `next`.
1. The builder stage trusts `/build` as a git `safe.directory`, so a build from a tar context made by a regular user stamps its commit. The check still fails the build when git is missing.
2. The CI fingerprint is now the hash of the commit being checked, with no docs exclusions. The `Dockerfile`, `.gitea/workflows/check.yml` (including the `fetch-depth` comment) and the README's "CI gate honesty" say so.
3. `.dockerignore` lists `.git/config`; `git describe` gives the short commit without it, and a plain `docker build .` with no build arguments stamps the commit.
- Not covered: `.dockerignore` filters only a directory context. A context uploaded as a tar, which is how upaas sends it, arrives unfiltered, so `.git/config` still reaches the builder stage of an upaas build. Keeping it out there needs upaas to apply the app's `.dockerignore` when it makes the tar.
- `TODO.md` still says a docs-only commit replays from the layer cache, which this PR makes untrue; that file is the manager's to update.
Model: opus-5-5
Review of #410 against #366: needs rework. Both findings of the earlier review are fixed.
Commit message and PR body, opening paragraph: they say the upaas build had no .git in its context. It had one: upaas uploads its clone as a tar, a tar context is not filtered by .dockerignore, so the old .git/ exclusion never applied to it, and the binary said unknown only because ARG VERSION defaulted to unknown. The PR body's own "Worth knowing" list says the tar arrives unfiltered, which contradicts its opening. Acceptable: both say upaas's context already carried .git and the unknown default of the VERSION build arg is what stamped it; the .git/ exclusion kept .git out of a directory-context build only.
.dockerignore lines 1-3 and the README's "Version stamping" paragraph: ".git is sent, without its config" and "It does leave out .git/config" hold only for a directory context. The same README paragraph covers contexts sent as a tar, and for those .git/config reaches the builder stage. Acceptable: the comment and the README say the exclusion applies to a directory context, and that a context sent as a tar carries .git/config unless its sender leaves it out.
On this host's Docker, a context sent as a tar does not honour .dockerignore: docker build - with the tar on standard input (with or without BuildKit) and a build through the Docker API with BuildKit, which is how upaas builds, all receive every file, .git/config included. Keeping .git/config out of upaas builds therefore needs a upaas change; no upaas issue for it is linked yet. upaas builds already carried .git and its config before this PR.
Judgement call: TODO.md still says a docs-only commit replays from the layer cache, which this PR makes untrue; that file is the manager's, so it is not counted against this PR.
Model: opus-5-5
Review of https://git.eeqj.de/sneak/webhooker/pulls/410 against https://git.eeqj.de/sneak/webhooker/issues/366: needs rework. Both findings of the earlier review are fixed.
1. Commit message and PR body, opening paragraph: they say the upaas build had no `.git` in its context. It had one: upaas uploads its clone as a tar, a tar context is not filtered by `.dockerignore`, so the old `.git/` exclusion never applied to it, and the binary said `unknown` only because `ARG VERSION` defaulted to `unknown`. The PR body's own "Worth knowing" list says the tar arrives unfiltered, which contradicts its opening. Acceptable: both say upaas's context already carried `.git` and the `unknown` default of the `VERSION` build arg is what stamped it; the `.git/` exclusion kept `.git` out of a directory-context build only.
2. `.dockerignore` lines 1-3 and the README's "Version stamping" paragraph: "`.git` is sent, without its config" and "It does leave out `.git/config`" hold only for a directory context. The same README paragraph covers contexts sent as a tar, and for those `.git/config` reaches the builder stage. Acceptable: the comment and the README say the exclusion applies to a directory context, and that a context sent as a tar carries `.git/config` unless its sender leaves it out.
On this host's Docker, a context sent as a tar does not honour `.dockerignore`: `docker build -` with the tar on standard input (with or without BuildKit) and a build through the Docker API with BuildKit, which is how upaas builds, all receive every file, `.git/config` included. Keeping `.git/config` out of upaas builds therefore needs a upaas change; no upaas issue for it is linked yet. upaas builds already carried `.git` and its config before this PR.
- Judgement call: `TODO.md` still says a docs-only commit replays from the layer cache, which this PR makes untrue; that file is the manager's, so it is not counted against this PR.
Model: opus-5-5
upaas uploads its clone as a tar context, which .dockerignore does not
filter, so its builds already carried .git; the binary said "unknown"
because the VERSION build arg defaulted to "unknown". The old .git/
exclusion kept .git out of a directory-context build only. .dockerignore
now lets .git through without its config, which can carry a credential,
and leaves out no tracked file (an excluded one would read as deleted and
mark the version -dirty). The VERSION build arg loses its "unknown"
default, so script/version derives the version inside the build; a given
VERSION still takes precedence.
The builder stage installs git, trusts the copied checkout whoever owns
its files, and fails when its context carries .git and the version still
comes out "unknown". The CI fingerprint is now the commit being checked.
Model: opus-5-5
The commit message and the PR body's opening now say upaas sends its clone as a tar, so its builds already carried .git, and that the unknown default of the VERSION build arg is what stamped them; the old .git/ exclusion kept .git out of a directory-context build only.
The .dockerignore comment and the README's "Version stamping" paragraph now say .git/config is left out of a directory context, and that a context sent as a tar carries it unless its sender leaves it out. The README links sneak/upaas#274 for upaas.
Model: opus-5-5
Reworked per https://git.eeqj.de/sneak/webhooker/pulls/410#issuecomment-111976; rebased onto `next`.
1. The commit message and the PR body's opening now say upaas sends its clone as a tar, so its builds already carried `.git`, and that the `unknown` default of the `VERSION` build arg is what stamped them; the old `.git/` exclusion kept `.git` out of a directory-context build only.
2. The `.dockerignore` comment and the README's "Version stamping" paragraph now say `.git/config` is left out of a directory context, and that a context sent as a tar carries it unless its sender leaves it out. The README links https://git.eeqj.de/sneak/upaas/issues/274 for upaas.
Model: opus-5-5
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Closes #366
upaas uploads its clone as a tar context, which
.dockerignoredoes not filter, so its builds already carried.git; theunknowndefault of theVERSIONbuild arg is what stamped them. The old.git/exclusion kept.gitout of a directory-context build only. The image now derives the version itself..dockerignoresends.gitwithout itsconfig(a remote URL there can carry a credential;git describedoes not need it). It no longer leaves out tracked files (*.md,LICENSE,.editorconfig), which git in the build would see as deleted and mark-dirty.ARG VERSIONhas no default, so with no build argumentsmake buildfalls back toscript/version, which runsgit describeon the copied.git. AVERSIONbuild arg still wins;script/dockerkeeps passing the host's value.gitand trusts/buildas asafe.directory: a context sent as a tar keeps its files' owners, and git refuses a checkout owned by another user. It fails when its context carries.gitand the version still comes outunknown..gitin the context, docs-only commits rebuild in full.Worth knowing:
.dockerignorefilters only a directory context. A context uploaded as a tar, as upaas sends it, arrives unfiltered,.git/configincluded.script/cibuildnow carry the commit: item 2 of #265.Judgement call: the check keys on
.gitalone, because aVERSIONbuild arg is stamped as given.Model: opus-5-5
Review of #410 against #366: needs rework.
Dockerfile, builder stage (the new check at line 70): when the files in the build context are not owned by root, git in the build refuses the checkout as belonging to another user, and the build fails at the check even though the context carries the version. A directory context arrives owned by root, but a context sent as a tar archive keeps its owners:docker build - < context.tarmade by a regular user fails, and so would the tar upload upaas uses if its clone were not made as root. The plan on the issue requires the builder stage not to trip git's ownership check, and the definition of done covers any plaindocker buildthat has the information. The PR's statement that copied files always belong to root does not hold for tar contexts. Acceptable: git in the builder stage reads the copied checkout whoever owns it (for example by trusting the build directory as asafe.directory), so such a build stamps its commit; the check stays for a git that is genuinely missing.Dockerfileline 16 says.ci-fingerprintchanges with every commit that touches the build context, and.gitea/workflows/check.ymlline 16 says the fingerprint step finds the last commit that touched the build context. Both are now false:*.md,LICENSEand.editorconfigare in the context, but the fingerprint (check.ymlline 38, README "CI gate honesty") still skips them, and skipping them no longer does anything because.gitchanges the context with every commit. Acceptable: the fingerprint and every sentence about it match what the context now carries, for example by dropping the docs exclusions from the fingerprint and from its README description.Model: opus-5-5
3502fac119todefccd38f6Reworked per the review (#410 (comment)) and the convention addendum on #366; rebased onto
next./buildas a gitsafe.directory, so a build from a tar context made by a regular user stamps its commit. The check still fails the build when git is missing.Dockerfile,.gitea/workflows/check.yml(including thefetch-depthcomment) and the README's "CI gate honesty" say so..dockerignorelists.git/config;git describegives the short commit without it, and a plaindocker build .with no build arguments stamps the commit..dockerignorefilters only a directory context. A context uploaded as a tar, which is how upaas sends it, arrives unfiltered, so.git/configstill reaches the builder stage of an upaas build. Keeping it out there needs upaas to apply the app's.dockerignorewhen it makes the tar.TODO.mdstill says a docs-only commit replays from the layer cache, which this PR makes untrue; that file is the manager's to update.Model: opus-5-5
Review of #410 against #366: needs rework. Both findings of the earlier review are fixed.
Commit message and PR body, opening paragraph: they say the upaas build had no
.gitin its context. It had one: upaas uploads its clone as a tar, a tar context is not filtered by.dockerignore, so the old.git/exclusion never applied to it, and the binary saidunknownonly becauseARG VERSIONdefaulted tounknown. The PR body's own "Worth knowing" list says the tar arrives unfiltered, which contradicts its opening. Acceptable: both say upaas's context already carried.gitand theunknowndefault of theVERSIONbuild arg is what stamped it; the.git/exclusion kept.gitout of a directory-context build only..dockerignorelines 1-3 and the README's "Version stamping" paragraph: ".gitis sent, without its config" and "It does leave out.git/config" hold only for a directory context. The same README paragraph covers contexts sent as a tar, and for those.git/configreaches the builder stage. Acceptable: the comment and the README say the exclusion applies to a directory context, and that a context sent as a tar carries.git/configunless its sender leaves it out.On this host's Docker, a context sent as a tar does not honour
.dockerignore:docker build -with the tar on standard input (with or without BuildKit) and a build through the Docker API with BuildKit, which is how upaas builds, all receive every file,.git/configincluded. Keeping.git/configout of upaas builds therefore needs a upaas change; no upaas issue for it is linked yet. upaas builds already carried.gitand its config before this PR.TODO.mdstill says a docs-only commit replays from the layer cache, which this PR makes untrue; that file is the manager's, so it is not counted against this PR.Model: opus-5-5
defccd38f6to2aca0c981dReworked per #410 (comment); rebased onto
next..git, and that theunknowndefault of theVERSIONbuild arg is what stamped them; the old.git/exclusion kept.gitout of a directory-context build only..dockerignorecomment and the README's "Version stamping" paragraph now say.git/configis left out of a directory context, and that a context sent as a tar carries it unless its sender leaves it out. The README links sneak/upaas#274 for upaas.Model: opus-5-5
Review passed.
Model: opus-5-5