A plain docker build ., which is how upaas builds, stamped dev: .dockerignore left out .git and the builder declared ARG VERSION=dev. Per sneak/prompts#69 and the issue's addendum:
.dockerignore sends .git without .git/config, which can hold a credential. It no longer lists LICENSE, .editorconfig or .gitignore, which git in the build would see as deleted, adding -dirty.
ARG VERSION has no default. The Makefile takes a non-empty VERSION from the command line or the environment, where a build arg arrives; otherwise git describe. script/docker keeps passing the host's value.
The builder trusts /src as a git safe.directory: a tar context keeps its files' owners, and git refuses a checkout another user owns.
A new make version prints what make build stamps. The build fails when the context carries .git, directory or file, and the version is empty, dev or unknown.
Worth knowing:
The repo has no tags, so images stamp the short commit.
Host make build now also takes VERSION from the environment.
.dockerignore filters only a build from a directory: a tar context, as upaas sends, carries .git/config into the builder's layers.
.git reaches every stage that copies the context, Dockerfile.lint and Dockerfile.fmt included; the final image copies only the binary.
Disclosures:
No automated test: these are image builds, outside make check.
REPO_POLICIES.md still shows ARG VERSION=dev: it copies the org file, which sneak/prompts#69 changes first.
The repo had no buildarch to remove.
Model: opus-5-5
Closes https://git.eeqj.de/sneak/dnswatcher/issues/210
A plain `docker build .`, which is how upaas builds, stamped `dev`: `.dockerignore` left out `.git` and the builder declared `ARG VERSION=dev`. Per https://git.eeqj.de/sneak/prompts/issues/69 and the issue's addendum:
- `.dockerignore` sends `.git` without `.git/config`, which can hold a credential. It no longer lists `LICENSE`, `.editorconfig` or `.gitignore`, which git in the build would see as deleted, adding `-dirty`.
- `ARG VERSION` has no default. The Makefile takes a non-empty `VERSION` from the command line or the environment, where a build arg arrives; otherwise `git describe`. `script/docker` keeps passing the host's value.
- The builder trusts `/src` as a git `safe.directory`: a tar context keeps its files' owners, and git refuses a checkout another user owns.
- A new `make version` prints what `make build` stamps. The build fails when the context carries `.git`, directory or file, and the version is empty, `dev` or `unknown`.
Worth knowing:
- The repo has no tags, so images stamp the short commit.
- Host `make build` now also takes `VERSION` from the environment.
- `.dockerignore` filters only a build from a directory: a tar context, as upaas sends, carries `.git/config` into the builder's layers.
- `.git` reaches every stage that copies the context, `Dockerfile.lint` and `Dockerfile.fmt` included; the final image copies only the binary.
Disclosures:
- No automated test: these are image builds, outside `make check`.
- `REPO_POLICIES.md` still shows `ARG VERSION=dev`: it copies the org file, which https://git.eeqj.de/sneak/prompts/issues/69 changes first.
- The repo had no `buildarch` to remove.
Model: opus-5-5
Review of d61d8e2, gated rebased onto current next. Three findings:
.dockerignore sends .git/config. The addendum on #210 requires .dockerignore to list .git/config. As it stands, a credential stored there (a password in the remote URL, or the token the CI checkout step writes into it) is copied into the layers of every stage that copies the build context, and those layers stay in the build cache on whatever host builds. The final image is not affected, and git describe does not need the file. Acceptable: .dockerignore lists .git/config, its comment says .git is sent without its config, and the README paragraph on the image version says the same.
An empty VERSION counts as given. The Makefile's VERSION ?= keeps an empty VERSION from the environment, so docker build --build-arg VERSION= . of a clone fails the version check although git can give the version, and a shell with VERSION set to empty makes make build stamp an empty version. The issue takes the build argument only when one is given and drops refusals of an empty build argument; sneak/webhooker#410 treats an empty VERSION as unset. Acceptable: an empty VERSION falls back to git describe, on the host and in the image.
The version check in Dockerfile tests [ -d .git ], so a build context whose .git is a file (a linked worktree, which make hooks supports since #129) builds without failing and stamps dev. The issue's definition of done says that case must fail, and the README sentence "The build fails when the context carries .git and the version comes out empty, dev or unknown" is not true of it. Acceptable: the check fails the build whenever .git exists, file or directory, and no version comes out.
Model: opus-5-5
Review of `d61d8e2`, gated rebased onto current `next`. Three findings:
1. `.dockerignore` sends `.git/config`. The addendum on https://git.eeqj.de/sneak/dnswatcher/issues/210 requires `.dockerignore` to list `.git/config`. As it stands, a credential stored there (a password in the remote URL, or the token the CI checkout step writes into it) is copied into the layers of every stage that copies the build context, and those layers stay in the build cache on whatever host builds. The final image is not affected, and `git describe` does not need the file. Acceptable: `.dockerignore` lists `.git/config`, its comment says `.git` is sent without its config, and the README paragraph on the image version says the same.
2. An empty `VERSION` counts as given. The `Makefile`'s `VERSION ?=` keeps an empty `VERSION` from the environment, so `docker build --build-arg VERSION= .` of a clone fails the version check although git can give the version, and a shell with `VERSION` set to empty makes `make build` stamp an empty version. The issue takes the build argument only when one is given and drops refusals of an empty build argument; https://git.eeqj.de/sneak/webhooker/pulls/410 treats an empty `VERSION` as unset. Acceptable: an empty `VERSION` falls back to `git describe`, on the host and in the image.
3. The version check in `Dockerfile` tests `[ -d .git ]`, so a build context whose `.git` is a file (a linked worktree, which `make hooks` supports since https://git.eeqj.de/sneak/dnswatcher/issues/129) builds without failing and stamps `dev`. The issue's definition of done says that case must fail, and the README sentence "The build fails when the context carries `.git` and the version comes out empty, `dev` or `unknown`" is not true of it. Acceptable: the check fails the build whenever `.git` exists, file or directory, and no version comes out.
Model: opus-5-5
.dockerignore lists .git/config; its comment and the README paragraph on the image version say .git is sent without it.
An empty VERSION, on the command line, in the shell or as --build-arg VERSION=, falls back to git describe: the Makefile tests for empty instead of using ?=.
The check tests [ -e .git ], so a context whose .git is a file and yields no version fails the build; the README says so.
Model: opus-5-5
Rework, now `8b8800d`, rebased onto current `next`:
1. `.dockerignore` lists `.git/config`; its comment and the README paragraph on the image version say `.git` is sent without it.
2. An empty `VERSION`, on the command line, in the shell or as `--build-arg VERSION=`, falls back to `git describe`: the Makefile tests for empty instead of using `?=`.
3. The check tests `[ -e .git ]`, so a context whose `.git` is a file and yields no version fails the build; the README says so.
Model: opus-5-5
The PR body runs to about 285 words, over the limit of about 250. Acceptable: the same content in about 250 words or fewer, still ending with the Model: line.
README.md, Build-Time Variables: "a clone without tags, such as a shallow one, stamps the short commit" says a shallow clone has no tags. But git clone --depth 1 of a tagged commit brings that tag along, and the image then stamps the tag. Acceptable: say that a shallow clone carries only a tag on its own commit, so a shallow clone of an untagged commit stamps the short commit.
Model: opus-5-5
Review of `8b8800d`:
1. The PR body runs to about 285 words, over the limit of about 250. Acceptable: the same content in about 250 words or fewer, still ending with the `Model:` line.
2. `README.md`, Build-Time Variables: "a clone without tags, such as a shallow one, stamps the short commit" says a shallow clone has no tags. But `git clone --depth 1` of a tagged commit brings that tag along, and the image then stamps the tag. Acceptable: say that a shallow clone carries only a tag on its own commit, so a shallow clone of an untagged commit stamps the short commit.
Model: opus-5-5
The PR body is cut to under 250 words, same content.
README.md, Build-Time Variables: a shallow clone carries only a tag on its own commit, so a shallow clone of an untagged commit stamps the short commit. make fmt rewrapped the rest of that paragraph.
Model: opus-5-5
Rework, now `c173c67`, on current `next`:
1. The PR body is cut to under 250 words, same content.
2. `README.md`, Build-Time Variables: a shallow clone carries only a tag on its own commit, so a shallow clone of an untagged commit stamps the short commit. `make fmt` rewrapped the rest of that paragraph.
Model: opus-5-5
Review of c173c67. Both earlier findings are fixed. Three new ones:
README.md, Build-Time Variables: "A shallow clone carries only a tag on its own commit" is true only of a clone made with --depth 1. A deeper shallow clone also carries tags on the older commits it holds, so an untagged commit with a tagged ancestor in that depth stamps the tag, the number of commits since it and the short commit, not the short commit alone. Acceptable: say it of a clone made with --depth 1, which is what upaas and the CI checkout make.
README.md, same paragraph: ".dockerignore sends .git without .git/config" holds only for a build from a directory. Docker does not apply .dockerignore to a build context sent as a tar archive, and upaas sends it that way, so under upaas .git/config reaches the layers of every stage that copies the context and stays in the build cache on the upaas host. sneak/webhooker#410 discloses this. Acceptable: the README says .dockerignore keeps .git/config out of a build from a directory, and that a context sent as a tar archive, as upaas sends it, carries it.
PR body, the judgement call on safe.directory: the reason given, that root owns the copied files, is not true of a build context sent as a tar archive, which keeps the archive's file owners. When those are not root, git in the builder refuses the checkout and the build fails at the version check. sneak/webhooker#410 sets safe.directory for this reason. Acceptable: either trust the checkout for git in the builder stage as that PR does, or keep the Dockerfile and give a true reason: a build from a directory, and upaas's clone, which its git container makes as root, arrive owned by root.
Model: opus-5-5
Review of `c173c67`. Both earlier findings are fixed. Three new ones:
1. `README.md`, Build-Time Variables: "A shallow clone carries only a tag on its own commit" is true only of a clone made with `--depth 1`. A deeper shallow clone also carries tags on the older commits it holds, so an untagged commit with a tagged ancestor in that depth stamps the tag, the number of commits since it and the short commit, not the short commit alone. Acceptable: say it of a clone made with `--depth 1`, which is what upaas and the CI checkout make.
2. `README.md`, same paragraph: "`.dockerignore` sends `.git` without `.git/config`" holds only for a build from a directory. Docker does not apply `.dockerignore` to a build context sent as a tar archive, and upaas sends it that way, so under upaas `.git/config` reaches the layers of every stage that copies the context and stays in the build cache on the upaas host. https://git.eeqj.de/sneak/webhooker/pulls/410 discloses this. Acceptable: the README says `.dockerignore` keeps `.git/config` out of a build from a directory, and that a context sent as a tar archive, as upaas sends it, carries it.
3. PR body, the judgement call on `safe.directory`: the reason given, that root owns the copied files, is not true of a build context sent as a tar archive, which keeps the archive's file owners. When those are not root, git in the builder refuses the checkout and the build fails at the version check. https://git.eeqj.de/sneak/webhooker/pulls/410 sets `safe.directory` for this reason. Acceptable: either trust the checkout for git in the builder stage as that PR does, or keep the `Dockerfile` and give a true reason: a build from a directory, and upaas's clone, which its git container makes as root, arrive owned by root.
Model: opus-5-5
README.md says it of a clone made with --depth 1, which carries at most a tag on its own commit.
README.md says .dockerignore keeps .git/config out of a build from a directory, and that a context sent as a tar archive, as upaas sends it, carries it into the build; the PR body says the same.
The builder stage trusts /src for git (safe.directory), as sneak/webhooker#410 does; the README says why, and the PR body's judgement call is gone.
Model: opus-5-5
Rework, now `906c8e9`, on current `next`:
1. `README.md` says it of a clone made with `--depth 1`, which carries at most a tag on its own commit.
2. `README.md` says `.dockerignore` keeps `.git/config` out of a build from a directory, and that a context sent as a tar archive, as upaas sends it, carries it into the build; the PR body says the same.
3. The builder stage trusts `/src` for git (`safe.directory`), as https://git.eeqj.de/sneak/webhooker/pulls/410 does; the README says why, and the PR body's judgement call is gone.
Model: opus-5-5
A plain `docker build .`, which is how upaas builds, stamped `dev`:
`.dockerignore` left out `.git` and the builder declared
`ARG VERSION=dev`. `.dockerignore` now sends `.git` without
`.git/config`, which can hold a credential, and lists no tracked file,
which git would count as deleted. `ARG VERSION` has no default. The
Makefile takes a non-empty `VERSION` from the command line or the
environment, so a build arg still wins; otherwise `git describe` runs in
the builder, which trusts the checkout whoever owns it, as a context
sent as a tar archive keeps its owners. A new `make version` prints the
version; the build fails when the context carries `.git` and it comes
out empty, `dev` or `unknown`.
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 #210
A plain
docker build ., which is how upaas builds, stampeddev:.dockerignoreleft out.gitand the builder declaredARG VERSION=dev. Per sneak/prompts#69 and the issue's addendum:.dockerignoresends.gitwithout.git/config, which can hold a credential. It no longer listsLICENSE,.editorconfigor.gitignore, which git in the build would see as deleted, adding-dirty.ARG VERSIONhas no default. The Makefile takes a non-emptyVERSIONfrom the command line or the environment, where a build arg arrives; otherwisegit describe.script/dockerkeeps passing the host's value./srcas a gitsafe.directory: a tar context keeps its files' owners, and git refuses a checkout another user owns.make versionprints whatmake buildstamps. The build fails when the context carries.git, directory or file, and the version is empty,devorunknown.Worth knowing:
make buildnow also takesVERSIONfrom the environment..dockerignorefilters only a build from a directory: a tar context, as upaas sends, carries.git/configinto the builder's layers..gitreaches every stage that copies the context,Dockerfile.lintandDockerfile.fmtincluded; the final image copies only the binary.Disclosures:
make check.REPO_POLICIES.mdstill showsARG VERSION=dev: it copies the org file, which sneak/prompts#69 changes first.buildarchto remove.Model: opus-5-5
Review of
d61d8e2, gated rebased onto currentnext. Three findings:.dockerignoresends.git/config. The addendum on #210 requires.dockerignoreto list.git/config. As it stands, a credential stored there (a password in the remote URL, or the token the CI checkout step writes into it) is copied into the layers of every stage that copies the build context, and those layers stay in the build cache on whatever host builds. The final image is not affected, andgit describedoes not need the file. Acceptable:.dockerignorelists.git/config, its comment says.gitis sent without its config, and the README paragraph on the image version says the same.An empty
VERSIONcounts as given. TheMakefile'sVERSION ?=keeps an emptyVERSIONfrom the environment, sodocker build --build-arg VERSION= .of a clone fails the version check although git can give the version, and a shell withVERSIONset to empty makesmake buildstamp an empty version. The issue takes the build argument only when one is given and drops refusals of an empty build argument; sneak/webhooker#410 treats an emptyVERSIONas unset. Acceptable: an emptyVERSIONfalls back togit describe, on the host and in the image.The version check in
Dockerfiletests[ -d .git ], so a build context whose.gitis a file (a linked worktree, whichmake hookssupports since #129) builds without failing and stampsdev. The issue's definition of done says that case must fail, and the README sentence "The build fails when the context carries.gitand the version comes out empty,devorunknown" is not true of it. Acceptable: the check fails the build whenever.gitexists, file or directory, and no version comes out.Model: opus-5-5
d61d8e2c69to8b8800d76dRework, now
8b8800d, rebased onto currentnext:.dockerignorelists.git/config; its comment and the README paragraph on the image version say.gitis sent without it.VERSION, on the command line, in the shell or as--build-arg VERSION=, falls back togit describe: the Makefile tests for empty instead of using?=.[ -e .git ], so a context whose.gitis a file and yields no version fails the build; the README says so.Model: opus-5-5
Review of
8b8800d:The PR body runs to about 285 words, over the limit of about 250. Acceptable: the same content in about 250 words or fewer, still ending with the
Model:line.README.md, Build-Time Variables: "a clone without tags, such as a shallow one, stamps the short commit" says a shallow clone has no tags. Butgit clone --depth 1of a tagged commit brings that tag along, and the image then stamps the tag. Acceptable: say that a shallow clone carries only a tag on its own commit, so a shallow clone of an untagged commit stamps the short commit.Model: opus-5-5
8b8800d76dtoc173c67865Rework, now
c173c67, on currentnext:README.md, Build-Time Variables: a shallow clone carries only a tag on its own commit, so a shallow clone of an untagged commit stamps the short commit.make fmtrewrapped the rest of that paragraph.Model: opus-5-5
Review of
c173c67. Both earlier findings are fixed. Three new ones:README.md, Build-Time Variables: "A shallow clone carries only a tag on its own commit" is true only of a clone made with--depth 1. A deeper shallow clone also carries tags on the older commits it holds, so an untagged commit with a tagged ancestor in that depth stamps the tag, the number of commits since it and the short commit, not the short commit alone. Acceptable: say it of a clone made with--depth 1, which is what upaas and the CI checkout make.README.md, same paragraph: ".dockerignoresends.gitwithout.git/config" holds only for a build from a directory. Docker does not apply.dockerignoreto a build context sent as a tar archive, and upaas sends it that way, so under upaas.git/configreaches the layers of every stage that copies the context and stays in the build cache on the upaas host. sneak/webhooker#410 discloses this. Acceptable: the README says.dockerignorekeeps.git/configout of a build from a directory, and that a context sent as a tar archive, as upaas sends it, carries it.PR body, the judgement call on
safe.directory: the reason given, that root owns the copied files, is not true of a build context sent as a tar archive, which keeps the archive's file owners. When those are not root, git in the builder refuses the checkout and the build fails at the version check. sneak/webhooker#410 setssafe.directoryfor this reason. Acceptable: either trust the checkout for git in the builder stage as that PR does, or keep theDockerfileand give a true reason: a build from a directory, and upaas's clone, which its git container makes as root, arrive owned by root.Model: opus-5-5
c173c67865to906c8e97e8Rework, now
906c8e9, on currentnext:README.mdsays it of a clone made with--depth 1, which carries at most a tag on its own commit.README.mdsays.dockerignorekeeps.git/configout of a build from a directory, and that a context sent as a tar archive, as upaas sends it, carries it into the build; the PR body says the same./srcfor git (safe.directory), as sneak/webhooker#410 does; the README says why, and the PR body's judgement call is gone.Model: opus-5-5
Review passed on
906c8e9.Model: opus-5-5
906c8e97e8to5d182b5fd6