Keel-Linux/fab
1
0
Fork 0
Code Issues 3 Pull requests 1 Projects Releases Packages Wiki Activity Actions

Build as keel-fab, with a version this project chose #9

Open
marcos-mendez wants to merge 9 commits from pkg/keel-fab into master
pull from: pkg/keel-fab
merge into: Keel-Linux:master
Conversation 2 Commits 9 Files changed 19 +1079 -64
marcos-mendez commented 2026-09-28 05:10:13 -03:00 (Migrated from github.com)
Copy link

Closes #8. It also closes #7, in the same pull request rather than a separate one: #7 asks for a changelog entry that makes the running version exist in git, for the require-changelog wiring, and for provenance checkable from a manifest and a git tag. The first is already true and was true before #7 was filed (below); the other two are here. There is nothing left in #7 that this does not do, and the constraint is one pull request per repository.

What 1.1.1+keel2 actually is, measured before anything was written

The installed package on the build host is byte for byte /root/src/fab/../fab_1.1.1+keel2_all.deb, sha256 1b8ee5bcc2a9e91596f521abe561f4c69a4c1b92e1ee014d952eabd8b6186adf, built there on 2026-09-27 at 17:14:56 UTC. /var/lib/dpkg/info/fab.md5sums is identical to the .deb's own md5sums, and dpkg -V fab is clean.

It was built from /root/src/fab at commit e610377 with a clean worktree. Every file it ships is in git. Comparing the extracted .deb against that checkout, file by file: 22 of 25 byte identical, and the three that differ (/usr/bin/fab, share/make-release-deb.py, share/turnkey-version.py) differ only in dh_python3's shebang normalisation, #!/usr/bin/python3 to #! /usr/bin/python3. product.mk is a06bfe03, as #7 records. There is nothing in the running fab that is not in this repository.

And the changelog entry exists. e610377 is debian: changelog 1.1.1+keel2, the unit slots the build host needs, and it merged to master as #6 at 2026-09-27T19:58:53Z, ten hours before #7 was filed at 2026-09-28T06:06:20Z. git log --oneline -- debian/changelog on master today returns two commits, not one. The measurement in #7 read a stale ref: in the build host clone HEAD is e610377, but remotes/origin/master is 16730a5, the #5 merge, because the clone had not fetched since before #6.

So the defect is not the one #7 describes, and it is worse in one respect and better in another:

#7 says Measured
1.1.1+keel2 appears nowhere in the history It is on master, merged in #6 before #7 was filed
The installed files match HEAD They do, everywhere, modulo a deterministic shebang rewrite
The version was bumped at packaging time True, and it is the real defect: the package was built and installed at 17:14 and no branch carried the version until 19:58
Two behaviour changing pull requests merged with no entry True: #4 and #5 both changed share/product.mk, a path this package ships
Provenance must be checkable from a manifest and a tag Neither packaged version was tagged at all. git tag -l carried only upstream's v0.5..v1.1.1

The last row is what was actually unfixed, and it is why core.manifest's fab_version 1.1.1+keel1 named no commit. Two annotated tags now exist, pushed ahead of this branch so the new check passes: fab/1.1.1+keel1 on b07a733 and fab/1.1.1+keel2 on e610377, each recording what was built from it and that it was tagged after the fact.

One unrelated finding, for whoever owns docs/build-host.md: it records the +keel1 .deb as sha256 f4bf09be9535…, and the file on the host and its own .changes both say 1dc54a20acc2….

The shape, and why

The package becomes keel-fab at 2.0.0. The repository stays fab. Decision 0006 rules on repository names, and infrastructure keeps its upstream name; this renames the Debian package, which is a different axis, and the fork, its history and its upstream compatibility are untouched. No +keelN.

2.0.0 and not 0.1.0, and the changelog gate is what settled it. 0.1.0 was proposed first, matching the scheme every package this project owns already starts at: keel 0.1.0, keel-transition 0.1.0, keel-archive-keyring 0.1.0. The gate wired up in this same branch refused it on its first run, which is the first thing it has ever caught here. dpkg --compare-versions 0.1.0 gt 1.1.1+keel2 is false; require-changelog compares the proposed top version against the base's exactly that way, reprepro and dpkg-genchanges read a changelog as one monotonic series too, and dpkg-genchanges was already warning on the 0.1.0 build. A rename does not give the changelog a fresh start. The reset was the wrong signal as well: the other Keel packages start at 0.1.0 because they had no predecessor, while this code has been building every layer in production since before it was ours. 2.0.0 sorts above every upstream 1.x and both +keelN builds, carries no suffix, and says what happened. tests/packaging.sh now asserts the ordering with that measurement in the comment.

The apt tooling agrees without being changed, which is the strongest argument for the name. apt/lib/build.sh classifies a package as native when the Source starts with keel, the clone has no upstream remote, and neither debian/watch nor debian/upstream/metadata exists. This clone has no upstream remote and neither file, so all three now hold and bin/build-package will accept 0.1.0 on its own. The +keelN guard correctly stops applying instead of having to be overridden with --native.

Provides: fab, Conflicts: fab, Replaces: fab, all unversioned. apt-cache rdepends fab on the build host is empty and no debian/control in the organization depends on fab, so Provides satisfies nothing here. It does not protect an install by name: apt prefers a real fab in an enabled archive (TurnKey's, at 999 on the build host) and removes keel-fab for it, and a fab plan cannot resolve a virtual name at all (tkldev#4). What protects the host is the negative pin Package: fab / Pin: release o=turnkeylinux / Pin-Priority: -1 (not Pin: origin "", which only matches a local repository), which makes apt#15 a prerequisite of the conversion. Conflicts and Replaces are the load-bearing pair: the two packages ship the same paths and must never be co-installed. A versioned Provides: fab (= 1.1.1+keel2) was considered and rejected: it would satisfy a versioned dependency on fab, of which there are none anywhere, at the cost of writing upstream's version number back into a package whose point is not to carry it.

The call sites decide the command names, and they say keep them

Counted across the organization (about 700 references, measured in review over 40 repositories; the first count was 10 to 20 percent lower), and not one of them reads the Debian package name: they are $PATH command lookups, make variables and filesystem paths, and Debian imposes no relation between a package's name and the paths it ships.

Symbol References What it is
fab-chroot ~100 command on PATH
fab-apply-overlay 34 command on PATH
fab-plan-resolve 25 command on PATH
fab-apply-removelist 12 command on PATH
fab-apply-patch, fab-install, fab-cpp, fab-query, fab-plan-annotate 21 commands on PATH
fab-investigate, fab-rewind 17 commands on PATH
FAB_PATH ~256 make variable, /turnkey/fab-keel
FAB_ARCH ~92 make variable
$(FAB_PATH)/common 65 a git checkout, not a packaged path
/usr/share/fab 9 packaged path, via FAB_SHARE_PATH
import fab 0 the module is fablib, and always was

So every command name, every path and the fablib module stay exactly as they are. Renaming the binaries is a much larger change and is not in here. The nine /usr/bin/fab-* aliases are each asserted separately in tests/packaging.sh, because the one thing a rename really does break is debian/fab.install, debian/fab.links and debian/fab.docs: debhelper keys them on the binary package name, and a rename that leaves them behind builds a package with no /usr/bin/fab* and no /usr/share/fab while saying nothing, with the failure surfacing at the first fab-chroot of the next build.

Nine sites do read the package name. Eight are listed under "not in this pull request" below. The ninth was inside this repository and is fixed here: fab --version ran apt-cache policy fab, which asks about whatever package is called fab on the machine rather than about itself. After this rename it would answer (none), measured on the build host against a real virtual package. bt-layer:214 writes that answer into every layer manifest as fab_version, so it would have written (none) into the provenance record of every image built afterwards. debian/rules now records the changelog version at /usr/share/fab/version and fablib/version.py reads it back, which is also the right answer on a host that installed a .deb from a file and has no archive entry either way.

Which fab built this layer, in both directions

A manifest records the builder as a version string and nothing else, so the string is provenance only if it resolves to a commit.

  • Manifest to commit. fab_version 1.1.1+keel1 becomes git rev-parse fab/1.1.1+keel1. bin/check-release-tags makes that total: every changelog entry whose distribution is not UNRELEASED is a release, and every release below the newest must have a tag <source>/<version> whose tree carries that version at the top of its changelog. The newest entry is exempt, being the release a pull request is proposing. The source name comes from the entry, so the two versions released before the rename are checked as fab/… and not keel-fab/….
  • Commit to manifest. git describe --match '*/*' on a commit names its release, and that tag's changelog gives the version a manifest would record. No new machinery.

Recording a commit in the manifest as well is worth doing, and is not needed now. keel already keeps any manifest key it does not know and writes it back unchanged (keel/docs/layers.md, "Optional fields"), so a fab_commit field costs one line at bt-layer:214 and nothing at all in keel. What it buys over the tag is the case the tag cannot cover: a .deb built from an untagged or dirty tree, which is exactly what happened on 2026-09-27. The tag invariant closes that procedurally and the CI job keeps it closed, so fab_commit is belt and braces rather than a prerequisite. It is a buildtasks change, so it is a second pull request in that repository, and it is filed as Keel-Linux/buildtasks#12 rather than bundled here.

Converting the build host, and the way back

Nothing was built, installed or changed on the build host. Access there was read only throughout, no lock was taken, and no layer was published or rebuilt. The package was built in a throwaway debian:trixie container to prove debian/rules works and to compare the result against what the host runs.

The conversion is one command and is reversible, because fab_1.1.1+keel2_all.deb stays in /root/src and nothing deletes it:

# forward
apt-get install ./keel-fab_2.0.0_all.deb          # removes fab, Conflicts
# and /etc/apt/preferences.d/keel-fab becomes  Package: keel-fab
#                                              Pin: version 2.*

# back
apt-get install /root/src/fab_1.1.1+keel2_all.deb  # removes keel-fab
# and restore the pin stanza

Verify after either direction with fab --version, dpkg -L keel-fab | grep /usr/bin, and one fab-chroot in a scratch tree.

Two things to know before scheduling it. /etc/apt/preferences.d/keel-fab pins Package: fab / Pin: version 1.1.1+keel1*, and the installed version is 1.1.1+keel2, so that pin matches nothing today and only Debian version ordering is keeping the local build in place over upstream's 1.1.1 at priority 999; it needs fixing whether or not this lands. And docs/build-host.md sections 2 and 3 describe the package and the pin and will need the new name.

Measured

Suite Checks
tests/source-date-epoch.sh 19 of 19
tests/units.sh 56 of 56
tests/packaging.sh 36 of 36
tests/release-tags.sh 15 of 15

126 of 126, 100 percent. The gate stays at 100. The TAP helpers moved to tests/tap.sh when the third suite wanted them; the handbook records copying a test library instead of sharing it as something this project did wrong and would do again unless written down.

What no assertion about debian/ can prove is that the built package is the same package, so that was measured, over the full file set. keel-fab 2.0.0 against the fab 1.1.1+keel2 installed on the build host: 35 entries old, 37 new; 27 at the same path, 26 byte identical (all nine /usr/bin/fab-* symlinks and share/product.mk among them; /usr/bin/fab differs by the get_version change alone); 8 renamed by debhelper keying on the package name (four dist-info files, three under usr/share/doc, runtime.d/fab.rtupdate); 2 new, fablib/version.py and /usr/share/fab/version. Nothing dropped, and dpkg-genchanges no longer warns.

That comparison is also what caught the one real regression in this change, which no reading of debian/ would have. dh_python3 finds a private python directory by the binary package name, so while the package was called fab it picked up /usr/share/fab on its own; renaming it silently dropped the byte-compilation registration and the shebang rewrite. Naming the directory fixes that, but a single call with an argument replaces the default pass rather than adding to it, and the default pass is what moves fablib off /usr/lib/python3.13/dist-packages onto the version independent /usr/lib/python3/dist-packages, so one call trades one silent divergence for a worse one. All three variants were built and compared; both calls are needed, and debian/rules says why.

Not in this pull request, deliberately

Each is a different repository, so each is its own issue and its own pull request.

Where What
apt (#15) Prerequisite of the conversion. The pin matches nothing today; the conversion needs the negative pin on the real fab above. 4 package-name reads in tests/build-package.bats (:56, :192-194), docs/layout.md pool path, and the native classification depending on there being no upstream remote.
buildtasks (#12) fab_commit in the manifest at bt-layer:214. Also the dead bt-img:113-118 gate, which assumes a fab_<ver>_all.deb filename.
tkldev (#4) tkldev-setup:372 installs fab by name, which Provides does not protect (see above); plan/main:3 names it in a fab plan, which cannot resolve a virtual package and drops it silently. Before the archive ever drops fab.
handbook docs/infra-recovery.md:96 installs fab by name; docs/build-host.md sections 2 to 4. At conversion time.
bootstrap README.rst:32 lists fab as a dependency; Makefile:41 checks for the command, which is unaffected.
organization (tracker#18) Seven more repositories ship something installable and do not call require-changelog, keel-core among them. Audited in Keel-Linux/tracker#18, since it is common to the whole organization.
handbook (#18) Decision note 0017, its own issue and pull request. docs/build-host.md sections 2 and 3 at conversion time.

Test plan

  • Four suites green, 141 of 141, tests/coverage.sh 100.
  • shellcheck -x clean on every file this branch adds or touches. The remaining findings in tests/regtest.sh and tests/override.sh are upstream's and untouched.
  • The package builds in a debian:trixie container and its contents were compared against the .deb installed on the build host, path set and byte content.
  • bin/check-release-tags passes on this branch, with fab/1.1.1+keel1 and fab/1.1.1+keel2 pushed.
  • Nothing built, installed, rebuilt or changed on the build host; read only access, no lock taken, no layer published.
  • package / changelog, release-tags and tests / coverage all green. The first two ran here for the first time, and package / changelog earned its keep immediately by refusing 0.1.0.
  • Maintainer confirms keel-fab and 2.0.0, and schedules the conversion. Handbook decision note 0017 (Keel-Linux/handbook#18) carries the reasoning.
Closes #8. **It also closes #7**, in the same pull request rather than a separate one: #7 asks for a changelog entry that makes the running version exist in git, for the `require-changelog` wiring, and for provenance checkable from a manifest and a git tag. The first is already true and was true before #7 was filed (below); the other two are here. There is nothing left in #7 that this does not do, and the constraint is one pull request per repository. ## What `1.1.1+keel2` actually is, measured before anything was written The installed package on the build host is byte for byte `/root/src/fab/../fab_1.1.1+keel2_all.deb`, sha256 `1b8ee5bcc2a9e91596f521abe561f4c69a4c1b92e1ee014d952eabd8b6186adf`, built there on 2026-09-27 at 17:14:56 UTC. `/var/lib/dpkg/info/fab.md5sums` is identical to the `.deb`'s own `md5sums`, and `dpkg -V fab` is clean. It was built from `/root/src/fab` at commit `e610377` with a clean worktree. **Every file it ships is in git.** Comparing the extracted `.deb` against that checkout, file by file: 22 of 25 byte identical, and the three that differ (`/usr/bin/fab`, `share/make-release-deb.py`, `share/turnkey-version.py`) differ only in `dh_python3`'s shebang normalisation, `#!/usr/bin/python3` to `#! /usr/bin/python3`. `product.mk` is `a06bfe03`, as #7 records. There is nothing in the running fab that is not in this repository. **And the changelog entry exists.** `e610377` is *debian: changelog 1.1.1+keel2, the unit slots the build host needs*, and it merged to `master` as #6 at 2026-09-27T19:58:53Z, ten hours before #7 was filed at 2026-09-28T06:06:20Z. `git log --oneline -- debian/changelog` on `master` today returns two commits, not one. The measurement in #7 read a stale ref: in the build host clone `HEAD` is `e610377`, but `remotes/origin/master` is `16730a5`, the #5 merge, because the clone had not fetched since before #6. So the defect is not the one #7 describes, and it is worse in one respect and better in another: | #7 says | Measured | |---|---| | `1.1.1+keel2` appears nowhere in the history | It is on `master`, merged in #6 before #7 was filed | | The installed files match HEAD | They do, everywhere, modulo a deterministic shebang rewrite | | The version was bumped at packaging time | True, and it is the real defect: the package was built and installed at 17:14 and no branch carried the version until 19:58 | | Two behaviour changing pull requests merged with no entry | True: #4 and #5 both changed `share/product.mk`, a path this package ships | | Provenance must be checkable from a manifest and a tag | **Neither packaged version was tagged at all.** `git tag -l` carried only upstream's `v0.5`..`v1.1.1` | The last row is what was actually unfixed, and it is why `core.manifest`'s `fab_version 1.1.1+keel1` named no commit. Two annotated tags now exist, pushed ahead of this branch so the new check passes: `fab/1.1.1+keel1` on `b07a733` and `fab/1.1.1+keel2` on `e610377`, each recording what was built from it and that it was tagged after the fact. One unrelated finding, for whoever owns `docs/build-host.md`: it records the `+keel1` `.deb` as sha256 `f4bf09be9535…`, and the file on the host and its own `.changes` both say `1dc54a20acc2…`. ## The shape, and why **The package becomes `keel-fab` at `2.0.0`. The repository stays `fab`.** Decision 0006 rules on repository names, and infrastructure keeps its upstream name; this renames the Debian package, which is a different axis, and the fork, its history and its upstream compatibility are untouched. No `+keelN`. **`2.0.0` and not `0.1.0`, and the changelog gate is what settled it.** `0.1.0` was proposed first, matching the scheme every package this project owns already starts at: `keel` 0.1.0, `keel-transition` 0.1.0, `keel-archive-keyring` 0.1.0. The gate wired up in this same branch refused it on its first run, which is the first thing it has ever caught here. `dpkg --compare-versions 0.1.0 gt 1.1.1+keel2` is false; `require-changelog` compares the proposed top version against the base's exactly that way, `reprepro` and `dpkg-genchanges` read a changelog as one monotonic series too, and `dpkg-genchanges` was already warning on the 0.1.0 build. **A rename does not give the changelog a fresh start.** The reset was the wrong signal as well: the other Keel packages start at 0.1.0 because they had no predecessor, while this code has been building every layer in production since before it was ours. `2.0.0` sorts above every upstream 1.x and both `+keelN` builds, carries no suffix, and says what happened. `tests/packaging.sh` now asserts the ordering with that measurement in the comment. The apt tooling agrees without being changed, which is the strongest argument for the name. `apt/lib/build.sh` classifies a package as native when the `Source` starts with `keel`, the clone has no `upstream` remote, and neither `debian/watch` nor `debian/upstream/metadata` exists. This clone has no `upstream` remote and neither file, so all three now hold and `bin/build-package` will accept `0.1.0` on its own. The `+keelN` guard correctly stops applying instead of having to be overridden with `--native`. **`Provides: fab`, `Conflicts: fab`, `Replaces: fab`, all unversioned.** `apt-cache rdepends fab` on the build host is empty and no `debian/control` in the organization depends on `fab`, so `Provides` satisfies nothing here. **It does not protect an install by name**: apt prefers a real `fab` in an enabled archive (TurnKey's, at 999 on the build host) and removes `keel-fab` for it, and a fab plan cannot resolve a virtual name at all (tkldev#4). What protects the host is the negative pin `Package: fab` / `Pin: release o=turnkeylinux` / `Pin-Priority: -1` (not `Pin: origin ""`, which only matches a local repository), which makes apt#15 a prerequisite of the conversion. `Conflicts` and `Replaces` are the load-bearing pair: the two packages ship the same paths and must never be co-installed. A versioned `Provides: fab (= 1.1.1+keel2)` was considered and rejected: it would satisfy a versioned dependency on `fab`, of which there are none anywhere, at the cost of writing upstream's version number back into a package whose point is not to carry it. ## The call sites decide the command names, and they say keep them Counted across the organization (about 700 references, measured in review over 40 repositories; the first count was 10 to 20 percent lower), and **not one of them reads the Debian package name**: they are `$PATH` command lookups, make variables and filesystem paths, and Debian imposes no relation between a package's name and the paths it ships. | Symbol | References | What it is | |---|---|---| | `fab-chroot` | ~100 | command on `PATH` | | `fab-apply-overlay` | 34 | command on `PATH` | | `fab-plan-resolve` | 25 | command on `PATH` | | `fab-apply-removelist` | 12 | command on `PATH` | | `fab-apply-patch`, `fab-install`, `fab-cpp`, `fab-query`, `fab-plan-annotate` | 21 | commands on `PATH` | | `fab-investigate`, `fab-rewind` | 17 | commands on `PATH` | | `FAB_PATH` | ~256 | make variable, `/turnkey/fab-keel` | | `FAB_ARCH` | ~92 | make variable | | `$(FAB_PATH)/common` | 65 | a git checkout, not a packaged path | | `/usr/share/fab` | 9 | packaged path, via `FAB_SHARE_PATH` | | `import fab` | 0 | the module is `fablib`, and always was | So every command name, every path and the `fablib` module stay exactly as they are. Renaming the binaries is a much larger change and is not in here. The nine `/usr/bin/fab-*` aliases are each asserted separately in `tests/packaging.sh`, because the one thing a rename really does break is `debian/fab.install`, `debian/fab.links` and `debian/fab.docs`: debhelper keys them on the binary package name, and a rename that leaves them behind builds a package with no `/usr/bin/fab*` and no `/usr/share/fab` while saying nothing, with the failure surfacing at the first `fab-chroot` of the next build. Nine sites do read the package name. Eight are listed under "not in this pull request" below. The ninth was inside this repository and is fixed here: `fab --version` ran `apt-cache policy fab`, which asks about whatever package is called `fab` on the machine rather than about itself. After this rename it would answer `(none)`, measured on the build host against a real virtual package. `bt-layer:214` writes that answer into every layer manifest as `fab_version`, so it would have written `(none)` into the provenance record of every image built afterwards. `debian/rules` now records the changelog version at `/usr/share/fab/version` and `fablib/version.py` reads it back, which is also the right answer on a host that installed a `.deb` from a file and has no archive entry either way. ## Which fab built this layer, in both directions A manifest records the builder as a version string and nothing else, so the string is provenance only if it resolves to a commit. - **Manifest to commit.** `fab_version 1.1.1+keel1` becomes `git rev-parse fab/1.1.1+keel1`. `bin/check-release-tags` makes that total: every changelog entry whose distribution is not `UNRELEASED` is a release, and every release below the newest must have a tag `<source>/<version>` whose tree carries that version at the top of its changelog. The newest entry is exempt, being the release a pull request is proposing. The source name comes from the entry, so the two versions released before the rename are checked as `fab/…` and not `keel-fab/…`. - **Commit to manifest.** `git describe --match '*/*'` on a commit names its release, and that tag's changelog gives the version a manifest would record. No new machinery. **Recording a commit in the manifest as well is worth doing, and is not needed now.** `keel` already keeps any manifest key it does not know and writes it back unchanged (`keel/docs/layers.md`, "Optional fields"), so a `fab_commit` field costs one line at `bt-layer:214` and nothing at all in `keel`. What it buys over the tag is the case the tag cannot cover: a `.deb` built from an untagged or dirty tree, which is exactly what happened on 2026-09-27. The tag invariant closes that procedurally and the CI job keeps it closed, so `fab_commit` is belt and braces rather than a prerequisite. It is a `buildtasks` change, so it is a second pull request in that repository, and it is filed as Keel-Linux/buildtasks#12 rather than bundled here. ## Converting the build host, and the way back **Nothing was built, installed or changed on the build host.** Access there was read only throughout, no lock was taken, and no layer was published or rebuilt. The package was built in a throwaway `debian:trixie` container to prove `debian/rules` works and to compare the result against what the host runs. The conversion is one command and is reversible, because `fab_1.1.1+keel2_all.deb` stays in `/root/src` and nothing deletes it: ``` # forward apt-get install ./keel-fab_2.0.0_all.deb # removes fab, Conflicts # and /etc/apt/preferences.d/keel-fab becomes Package: keel-fab # Pin: version 2.* # back apt-get install /root/src/fab_1.1.1+keel2_all.deb # removes keel-fab # and restore the pin stanza ``` Verify after either direction with `fab --version`, `dpkg -L keel-fab | grep /usr/bin`, and one `fab-chroot` in a scratch tree. Two things to know before scheduling it. `/etc/apt/preferences.d/keel-fab` pins `Package: fab` / `Pin: version 1.1.1+keel1*`, and the installed version is `1.1.1+keel2`, so **that pin matches nothing today** and only Debian version ordering is keeping the local build in place over upstream's `1.1.1` at priority 999; it needs fixing whether or not this lands. And `docs/build-host.md` sections 2 and 3 describe the package and the pin and will need the new name. ## Measured | Suite | Checks | |---|---| | tests/source-date-epoch.sh | 19 of 19 | | tests/units.sh | 56 of 56 | | tests/packaging.sh | 36 of 36 | | tests/release-tags.sh | 15 of 15 | **126 of 126, 100 percent.** The gate stays at 100. The TAP helpers moved to `tests/tap.sh` when the third suite wanted them; the handbook records copying a test library instead of sharing it as something this project did wrong and would do again unless written down. What no assertion about `debian/` can prove is that the built package is the same package, so that was measured, over the full file set. `keel-fab 2.0.0` against the `fab 1.1.1+keel2` installed on the build host: **35 entries old, 37 new; 27 at the same path, 26 byte identical** (all nine `/usr/bin/fab-*` symlinks and `share/product.mk` among them; `/usr/bin/fab` differs by the `get_version` change alone); **8 renamed by debhelper** keying on the package name (four `dist-info` files, three under `usr/share/doc`, `runtime.d/fab.rtupdate`); **2 new**, `fablib/version.py` and `/usr/share/fab/version`. Nothing dropped, and `dpkg-genchanges` no longer warns. That comparison is also what caught the one real regression in this change, which no reading of `debian/` would have. `dh_python3` finds a private python directory *by the binary package name*, so while the package was called `fab` it picked up `/usr/share/fab` on its own; renaming it silently dropped the byte-compilation registration and the shebang rewrite. Naming the directory fixes that, but a single call with an argument replaces the default pass rather than adding to it, and the default pass is what moves `fablib` off `/usr/lib/python3.13/dist-packages` onto the version independent `/usr/lib/python3/dist-packages`, so one call trades one silent divergence for a worse one. All three variants were built and compared; both calls are needed, and `debian/rules` says why. ## Not in this pull request, deliberately Each is a different repository, so each is its own issue and its own pull request. | Where | What | |---|---| | `apt` (#15) | **Prerequisite of the conversion.** The pin matches nothing today; the conversion needs the negative pin on the real `fab` above. 4 package-name reads in `tests/build-package.bats` (`:56`, `:192-194`), `docs/layout.md` pool path, and the native classification depending on there being no `upstream` remote. | | `buildtasks` (#12) | `fab_commit` in the manifest at `bt-layer:214`. Also the dead `bt-img:113-118` gate, which assumes a `fab_<ver>_all.deb` filename. | | `tkldev` (#4) | `tkldev-setup:372` installs `fab` by name, which `Provides` does not protect (see above); `plan/main:3` names it in a fab plan, which cannot resolve a virtual package and drops it silently. Before the archive ever drops `fab`. | | `handbook` | `docs/infra-recovery.md:96` installs `fab` by name; `docs/build-host.md` sections 2 to 4. At conversion time. | | `bootstrap` | `README.rst:32` lists `fab` as a dependency; `Makefile:41` checks for the *command*, which is unaffected. | | organization (tracker#18) | Seven more repositories ship something installable and do not call `require-changelog`, `keel-core` among them. Audited in Keel-Linux/tracker#18, since it is common to the whole organization. | | `handbook` (#18) | Decision note 0017, its own issue and pull request. `docs/build-host.md` sections 2 and 3 at conversion time. | ## Test plan - [x] Four suites green, 141 of 141, `tests/coverage.sh 100`. - [x] `shellcheck -x` clean on every file this branch adds or touches. The remaining findings in `tests/regtest.sh` and `tests/override.sh` are upstream's and untouched. - [x] The package builds in a `debian:trixie` container and its contents were compared against the `.deb` installed on the build host, path set and byte content. - [x] `bin/check-release-tags` passes on this branch, with `fab/1.1.1+keel1` and `fab/1.1.1+keel2` pushed. - [x] Nothing built, installed, rebuilt or changed on the build host; read only access, no lock taken, no layer published. - [x] `package / changelog`, `release-tags` and `tests / coverage` all green. The first two ran here for the first time, and `package / changelog` earned its keep immediately by refusing `0.1.0`. - [ ] Maintainer confirms `keel-fab` and `2.0.0`, and schedules the conversion. Handbook decision note 0017 (Keel-Linux/handbook#18) carries the reasoning.
marcos-mendez commented 2026-09-28 06:43:49 -03:00 (Migrated from github.com)
Copy link

Review of #9

Read-only throughout: nothing was built, installed or changed on the build host, no lock was taken. The package was built independently in a throwaway debian:trixie container, and the apt behaviour below was measured in a second container rather than on the host.

First, the thing #7 got wrong

Confirmed, and the correction in this pull request is the right outcome.

e610377 is "debian: changelog 1.1.1+keel2", it is an ancestor of master, and it merged as #6 at 2026-09-27T19:58:53Z. #7 was filed at 2026-09-28T06:06:20Z. git log --oneline origin/master -- debian/changelog returns two commits.

The cause is one step more specific than "the build host's own master". On the host, /root/src/fab is on local branch pkg/keel2 at e610377 with a clean worktree, and git log -- debian/changelog there, at HEAD, already returns both commits. What is stale is the remote-tracking ref: remotes/origin/master on that host is 16730a5, the #5 merge, so the host has not fetched since before #6 landed. master is 858d193, behind 4 of a stale origin/master and behind 6 of the real one. A measurement taken against origin/master in that clone produces exactly the line #7 reports.

The 1.1.1+keel2 installed tree claim also reproduces, exactly, and by a stronger method than file counting. Of the 18 paths the installed package takes from git, 15 are byte identical to e610377, and for the three that are not, re-applying only the shebang normalisation reproduces the installed md5sums:

usr/bin/fab                      f0c0d062b69a7734605a9fa7ea7fb31a  MATCH
usr/share/fab/make-release-deb.py 38ae5d92c156dc6b46e25707dbe25743 MATCH
usr/share/fab/turnkey-version.py  c658c643277f1a8f1dbc3e4eaa9a40aa MATCH

sha256 1b8ee5bc… on /root/src/fab_1.1.1+keel2_all.deb matches, dpkg -V fab is clean, and both annotated tags point where the body says. Tagging after the fact is the honest thing here and the annotations say so in the tag message, which is the right place for it. Nothing running on the build host is outside this repository.

Everything below is about what the change does next, ordered by severity.

HIGH 1. Provides: fab does not protect the build host. Measured.

debian/control:28, and the body's row "tkldev | Both are satisfied by Provides: fab, so neither is urgent".

apt prefers a real package over a virtual provider. The build host has a real fab in an enabled archive:

$ apt-cache policy fab                    # build host, read only
 *** 1.1.1+keel2 100   /var/lib/dpkg/status
     1.1.1       999   http://archive.turnkeylinux.org/debian trixie/main

Reproduced in a container with keel-fab 2.0.0 installed from a file at 100 and a real fab in an archive at 999:

$ apt-get install -s fab          # exactly what tkldev-setup:372 runs
The following packages will be REMOVED:
  keel-fab
The following NEW packages will be installed:
  fab
Remv keel-fab [2.0.0]
Inst fab (1.1.1 …)

Same result when upstream's real fab is 3.0.0, above keel-fab 2.0.0. So after the conversion, one tkldev-setup run, or one run of the disaster-recovery procedure in handbook/docs/infra-recovery.md:96, silently replaces the builder with upstream's fab, losing the SOURCE_DATE_EPOCH handling and the unit phases, and the next build is wrong rather than broken. apt-get upgrade and dist-upgrade are safe: nothing depends on fab, so nothing pulls it in unattended.

What closes it, also measured:

# /etc/apt/preferences.d/keel-fab
Package: fab
Pin: origin ""          # or the TurnKey origin
Pin-Priority: -1

With that in place apt-get install fab correctly answers "keel-fab is already the newest version".

This makes apt#15 a prerequisite of the conversion, not an independent follow-up. Its current text asks for the fab stanza to be widened and then renamed; it does not ask for upstream's real fab to be made unselectable, and that is the part the rename creates. apt#15 also still says the new stanza should pin 0.*, which was written before 2.0.0.

HIGH 2. tkldev/plan/main:3 is not satisfied by Provides: either, and fails silently

Same body row. This one is not an apt question at all. tkldev/Makefile is include $(FAB_PATH)/common/mk/turnkey.mk, so plan/main goes through fab-plan-resolve, and fab's own resolver does not resolve virtual packages:

  • fablib/plan.py:205-206 takes the pool's answer and does package_name = fname.split("_")[0], then self._deps[deps[package_name]]. The .deb filename's first field must be string-equal to the requested name. A keel-fab_2.0.0_all.deb returned for a request of fab raises an uncaught KeyError.
  • _get_provided (plan.py:329) reads Provides only from packages already fetched, and provided is only subtracted from all_missing at plan.py:406. It never causes a provider to be fetched.

And the miss is silent: missing is initialised at plan.py:366 and never mutated, while brokendeps is built by iterating it at plan.py:421-422, so resolve() returns (spec, []) even when all_missing is non-empty. A plan entry the pool cannot supply is dropped from the spec with no error.

That is upstream code and not this pull request's doing, so it is not a blocker for the diff. But it means the deferral rationale is wrong for this site, and it is latent rather than immediate only because apt/pool/main/f/fab/fab_1.1.1+keel1_all.deb is still in the archive: TKLDev images built after the conversion would keep shipping the pre-rename fab while the build host runs keel-fab, and the day fab leaves the pool they would ship no fab at all without saying so. plan/main:3 needs to become keel-fab, and it needs its own issue before the conversion, not after.

HIGH 3. The newest release is exempt forever, which is #7's hole one version deep

bin/check-release-tags:95-105.

The exemption is right for a pull request. The problem is that nothing ever revokes it. release-tags has no if:, so it does run on push: branches: [master], but the script exempts the newest entry there too. So keel-fab 2.0.0 merges with the check green, gets built, gets installed on the host, and bt-layer:214 writes fab_version 2.0.0 into every manifest, while git rev-parse keel-fab/2.0.0 fails, until somebody adds the next changelog entry. That is exactly the state #7 was filed about.

Two ways to close it, either is fine:

  • have the push: master run require the newest release to be tagged, for example bin/check-release-tags --require-newest, and push the tag with the merge as was done for fab/1.1.1+keel1 and fab/1.1.1+keel2; or
  • put the invariant where the defect actually happened, in bin/build-package: refuse to build a .deb whose top changelog version has no tag. The untagged .deb on 2026-09-27 was produced by a build, not by a merge.

There is also a bypass worth knowing about: an UNRELEASED entry on top keeps the newest released entry exempt, because proposed is set from the first non-UNRELEASED entry. Verified:

$ bash bin/check-release-tags .      # changelog: 4.0 UNRELEASED, 3.0 trixie, …
p/3.0 is the release being proposed, not tagged yet

HIGH 4. The gate accepts a lightweight tag on a commit that is not in the branch

bin/check-release-tags:59-61 and :107-120.

has_tag only checks refs/tags/<name> exists, and tag_version only compares the first line of that tree's changelog. Nothing checks the tag is annotated, and nothing checks it is reachable. Demonstrated on a scratch repository: two lightweight tags on an orphan branch whose commits contain entirely different content pass the gate.

p/2.0 names the commit that carries 2.0
p/1.0 names the commit that carries 1.0
rc=0
$ git merge-base --is-ancestor p/2.0 main || echo "NOT reachable from main"
NOT reachable from main

The annotated part is not cosmetic. The note's other direction, git describe --match '*/*', only sees annotated tags:

$ git describe --match '*/*' HEAD~1
fatal: No annotated tags can describe '6ce89b3…'.
However, there were unannotated tags: try --tags.

So the two halves of the "in both directions" invariant are enforced to different standards, and the half that CI checks is the weaker one. Two lines fix it:

[[ "$(git -C "$1" cat-file -t "refs/tags/$2")" == tag ]] || …   # annotated
git -C "$1" merge-base --is-ancestor "refs/tags/$2" HEAD || …   # reachable

The tags actually pushed are annotated and on master, so this is about what the rule permits next time, not about what is there now. tests/release-tags.sh currently locks the looser behaviour in, because every fixture uses git tag "$2".

MEDIUM 5. fab --version can still write a wrong fab_version, through FAB_SHARE_PATH

fablib/version.py:22-24.

share_path() honours FAB_SHARE_PATH. Nothing else in fab or fablib reads that variable any more; its only other life is as a make variable, share/product.mk:60 and common/mk/turnkey.mk:26, both ?= and neither exported, so a normal bt-layer run is fine and I could not make it misfire. But an exported FAB_SHARE_PATH pointing at a checkout, which is a plausible thing for someone debugging fab to do, makes package_version() return unknown, fab --version print it and exit 0, and bt-layer:214 write fab_version unknown into the manifest. bt-layer cannot catch it either: fab_version "$(fab --version)" is a command substitution in an argument list, so even a non-zero exit is discarded under set -e.

Given the stated premise that a wrong provenance record is worse than none, the test override wants to be its own variable, say FAB_VERSION_FILE, so that nothing a build environment sets can redirect the version lookup. Making --version exit non-zero on unknown is worth doing too, but only helps once buildtasks#12 reads the status.

The rest of that path is sound. I built the package and /usr/share/fab/version contains exactly 2.0.0, package_version() returns '2.0.0', and the file can never be empty because read_text().strip() or UNKNOWN covers it. override_dh_install writing into a directory dh_install must have created means a missing directory is a loud build failure, not a silent empty file, which is the right way round.

MEDIUM 6. The "28 paths, 26 byte identical" measurement excludes 7 files without saying so

I rebuilt the package and compared it against the fab 1.1.1+keel2 installed on the host. The substantive claims all hold, and the dh_python3 finding is real and well handled: make-release-deb.py and turnkey-version.py come out byte identical to the installed ones, runtime.d/keel-fab.rtupdate is present, and fablib lands in /usr/lib/python3/dist-packages rather than python3.13, which is the three-way result the two calls were for.

The arithmetic is not reproducible as written. The old package ships 26 regular files plus 9 symlinks. 28 reconstructs only as 19 files plus 9 symlinks, that is, with the 4 dist-info files and the 3 usr/share/doc/fab/ files dropped. All three things the summary does not mention live in that dropped set:

  • usr/share/doc/fab/ becomes usr/share/doc/keel-fab/, so three paths the old package had are not present under their old names. Nothing in the organization reads that path, so the impact is nil, but "all 28 paths the old package had are present" is not what happened.
  • changelog.gz differs, 115f2f5c to 20760196. It is a third differing file, not two.
  • usr/lib/python3/dist-packages/fab-1.1.0.dist-info/ is unchanged, still fab, still 1.1.0.

Also usr/share/python3/runtime.d/fab.rtupdate becomes keel-fab.rtupdate, which the body covers with a glob rather than calling it a rename.

Restating it as "35 entries in the old package, 32 byte identical, 3 renamed by debhelper, 2 new" would be both truer and stronger.

MEDIUM 7. pyproject.toml is untouched, so there are now three version sources

pyproject.toml:7-8 keeps name = "fab", version = "1.1.0". importlib.metadata.version("fab") on the built package returns 1.1.0 while dpkg says keel-fab 2.0.0 and /usr/share/fab/version says 2.0.0. Keeping it frozen is probably deliberate, since it is what keeps the four dist-info files byte identical, but nothing says so, and the next person to "fix" version reporting will reach for importlib.metadata and get 1.1.0. One comment in pyproject.toml or in debian/rules settles it.

MEDIUM 8. The native classification depends on there being no upstream remote, and 0008 invites one

apt/lib/build.sh:83. The argument for the name is that all three native conditions now hold, and one of them is git -C "$dir" remote | grep -qx upstream. Decision 0008 keeps the fork upstream compatible with cherry-picks in both directions, and the ordinary way to cherry-pick from upstream is git remote add upstream. Whoever does that on the build host clone makes bin/build-package classify keel-fab as a rebuild and refuse 2.0.0 for lacking +keelN. Neither this pull request nor apt#15 mentions it. It wants either a note in apt/lib/build.sh or a condition that does not depend on a local remote name.

MEDIUM 9. bin/check-release-tags has no floor

Every non-UNRELEASED entry below the newest is a release, with no lower bound. It passes today only because the changelog has four entries and the oldest, upstream's fab (1.1.1), is UNRELEASED. If a future merge from upstream brings real upstream entries into the file, the job demands fab/1.1.0, fab/1.0.3 and so on forever, and upstream's tags are v1.1.0, v1.0.3. A floor, or a --since, costs a line.

Related, smaller: a version with an epoch can never satisfy the rule, because refs/tags/keel-fab/1:2.0.0 is not a legal refname. Worth a line in the header given the note proposes this rule for the other repositories.

MEDIUM 10. The call-site inventory misses two live sites and understates one scope

The central negative claim is sound and I reproduced it independently across all 40 repositories in the organization: no debian/control anywhere depends on fab, import fab is genuinely zero, no recipe Makefile, no shared makefile in common and nothing in keel, keel-core or buildtasks reads the package name. buildtasks and the 22 recipe Makefiles need no edit. Missing from the table of nine:

  • handbook/docs/infra-recovery.md:96 : apt-get install -y fab deck pool turnkey-chroot …, the documented rebuild-from-a-fresh-Debian procedure. This is the one that combines with HIGH 1: it installs upstream's fab on a recovered host.
  • apt/lib/build.sh:112,118 : build_archive_version passes the repository name to apt-cache policy. That is the same bug this pull request just fixed in fab --version. It is an error-path hint only, so nothing breaks, but it belongs in the row.
  • The handbook row scopes the change to docs/build-host.md "sections 2 and 3". Section 4, "Rebuilding and reinstalling fab", lines 88-111, is the runbook someone will actually execute, and it names dpkg -l fab, apt-cache policy fab and the pin file.

In the other direction, "~30 literal fab assertions in tests/build-package.bats" overcounts: of the 34 occurrences only 4 are package-name reads, at :56 and :192-194. The rest are the repository name, which is not changing. build_changes_path:137 derives filenames from dpkg-parsechangelog -S Source, so the .deb and .changes names follow the rename on their own.

MEDIUM 11. The reference counts are not reproducible, and the table contradicts the prose

Measured across all 40 repositories, excluding .git/:

Symbol Table Measured
fab-chroot ~100 116
fab-apply-overlay 34 39
fab-plan-resolve 25 28
FAB_PATH ~256 298
FAB_ARCH ~92 111
/usr/share/fab 9 31
import fab 0 0

Low by 10 to 20 percent throughout and by 3.4 times for /usr/share/fab. Undercounting is the safe direction for a claim of the form "none of these reads the package name", so the conclusion is unaffected, but the prose says "roughly 380 references" while the table sums to 631. Say which tree and which exclusions produced the numbers, or round them off.

MEDIUM 12. The coverage number is a pass rate

tests/coverage.sh:37-40 computes the share of checks that pass, so it reads 100 whenever the suites are green and can only fall when a test fails. Adding packaging and release-tags to suites= therefore does not measure whether their branches are exercised; the claim rests entirely on the header's hand enumeration. That is pre-existing and honestly described, but docs/traps.md already carries "100 percent line coverage on the same file" as something that misled this project once, and the two new suites are the first here where a branch can go unexercised without the number moving. Worth a sentence in COVERAGE.md.

LOW

  • The count is 126, not 125. 19 + 56 + 36 + 15, and I ran all four: 1..19, 1..56, 1..36, 1..15, zero not ok. The Measured table says 126 and the test plan says 125.
  • "nine hours before #7 was filed" is 10 hours 7 minutes. 19:58:53Z to 06:06:20Z.
  • "the build host's own master, which git branch -vv there still reports as behind 4" is behind 4 of a origin/master that is itself stale at 16730a5; against the real master it is behind 6. The conclusion is right either way, and the sharper statement is that the host has not fetched since before #6.
  • "shellcheck -x clean on every file this branch adds or touches" is true at warning level and above. tests/units.sh and tests/source-date-epoch.sh, both touched here, carry 6 and 9 note-level SC2317. Identical counts on master, so nothing was introduced; the claim just wants "no warnings or errors".
  • debian/rules:15 hardcodes debian/keel-fab/. It fails loudly if the name changes again, which is acceptable, but $(shell dh_listpackages) costs nothing.
  • One operator hazard worth a line in the runbook. apt can never co-install the two: with --no-remove it stops at E: Packages need to be removed but remove is disabled. dpkg -i --force-conflicts --force-overwrite can, and because Replaces is one-directional the result is that keel-fab keeps ownership of /usr/bin/fab and all nine aliases even though fab unpacked second. Removing keel-fab then deletes them, leaving fab 1.1.1+keel2 installed, dpkg -V fab exiting 0, and no /usr/bin/fab and no /usr/bin/fab-chroot on the machine. So: never reach for --force-conflicts here, and verify with dpkg -S /usr/bin/fab rather than dpkg -V.

On the conversion itself

The mechanics are sound and I checked all three directions in a container.

  • Forward, no extra flags: apt-get install ./keel-fab_2.0.0_all.deb removes fab, installs keel-fab, and every path including the nine aliases ends up owned by keel-fab.
  • Back, no extra flags and no --allow-downgrades needed since the names differ: apt-get install /root/src/fab_1.1.1+keel2_all.deb removes keel-fab and restores /usr/bin/fab-chroot.
  • Failing halfway does leave a window with no fab at all, but it is recoverable offline from the kept .deb, which is the property that matters.

So the reversibility claim holds. What is not yet true is that it is safe to schedule: HIGH 1 has to be closed in apt#15 first, with the negative pin and not only the widened stanza, and HIGH 2 needs tkldev/plan/main filed before the archive ever drops fab.

Verdict

No CRITICAL. Four HIGH, none of them in the diff itself: the packaging is right, the dh_python3 work is right and well evidenced, the version choice is right and the test pins it, and #7's premise really was wrong. The HIGH items are in the deferral reasoning and in the reach of the new gate, and three of the four are a few lines each.

Warning.

## Review of #9 Read-only throughout: nothing was built, installed or changed on the build host, no lock was taken. The package was built independently in a throwaway `debian:trixie` container, and the apt behaviour below was measured in a second container rather than on the host. ### First, the thing #7 got wrong Confirmed, and the correction in this pull request is the right outcome. `e610377` *is* "debian: changelog 1.1.1+keel2", it is an ancestor of `master`, and it merged as #6 at `2026-09-27T19:58:53Z`. #7 was filed at `2026-09-28T06:06:20Z`. `git log --oneline origin/master -- debian/changelog` returns two commits. The cause is one step more specific than "the build host's own `master`". On the host, `/root/src/fab` is on local branch `pkg/keel2` at `e610377` with a clean worktree, and `git log -- debian/changelog` *there, at HEAD,* already returns both commits. What is stale is the remote-tracking ref: `remotes/origin/master` on that host is `16730a5`, the #5 merge, so the host has not fetched since before #6 landed. `master` is `858d193`, `behind 4` of a stale `origin/master` and behind 6 of the real one. A measurement taken against `origin/master` in that clone produces exactly the line #7 reports. The `1.1.1+keel2` installed tree claim also reproduces, exactly, and by a stronger method than file counting. Of the 18 paths the installed package takes from git, 15 are byte identical to `e610377`, and for the three that are not, re-applying only the shebang normalisation reproduces the installed md5sums: ``` usr/bin/fab f0c0d062b69a7734605a9fa7ea7fb31a MATCH usr/share/fab/make-release-deb.py 38ae5d92c156dc6b46e25707dbe25743 MATCH usr/share/fab/turnkey-version.py c658c643277f1a8f1dbc3e4eaa9a40aa MATCH ``` `sha256 1b8ee5bc…` on `/root/src/fab_1.1.1+keel2_all.deb` matches, `dpkg -V fab` is clean, and both annotated tags point where the body says. Tagging after the fact is the honest thing here and the annotations say so in the tag message, which is the right place for it. Nothing running on the build host is outside this repository. Everything below is about what the change does next, ordered by severity. --- ### HIGH 1. `Provides: fab` does not protect the build host. Measured. `debian/control:28`, and the body's row "`tkldev` | Both are satisfied by `Provides: fab`, so neither is urgent". apt prefers a **real** package over a virtual provider. The build host has a real `fab` in an enabled archive: ``` $ apt-cache policy fab # build host, read only *** 1.1.1+keel2 100 /var/lib/dpkg/status 1.1.1 999 http://archive.turnkeylinux.org/debian trixie/main ``` Reproduced in a container with `keel-fab 2.0.0` installed from a file at 100 and a real `fab` in an archive at 999: ``` $ apt-get install -s fab # exactly what tkldev-setup:372 runs The following packages will be REMOVED: keel-fab The following NEW packages will be installed: fab Remv keel-fab [2.0.0] Inst fab (1.1.1 …) ``` Same result when upstream's real `fab` is `3.0.0`, above `keel-fab 2.0.0`. So after the conversion, one `tkldev-setup` run, or one run of the disaster-recovery procedure in `handbook/docs/infra-recovery.md:96`, silently replaces the builder with upstream's fab, losing the `SOURCE_DATE_EPOCH` handling and the unit phases, and the next build is wrong rather than broken. `apt-get upgrade` and `dist-upgrade` are safe: nothing depends on `fab`, so nothing pulls it in unattended. What closes it, also measured: ``` # /etc/apt/preferences.d/keel-fab Package: fab Pin: origin "" # or the TurnKey origin Pin-Priority: -1 ``` With that in place `apt-get install fab` correctly answers "keel-fab is already the newest version". This makes apt#15 a **prerequisite of the conversion, not an independent follow-up**. Its current text asks for the `fab` stanza to be widened and then renamed; it does not ask for upstream's real `fab` to be made unselectable, and that is the part the rename creates. apt#15 also still says the new stanza should pin `0.*`, which was written before `2.0.0`. ### HIGH 2. `tkldev/plan/main:3` is not satisfied by `Provides:` either, and fails silently Same body row. This one is not an apt question at all. `tkldev/Makefile` is `include $(FAB_PATH)/common/mk/turnkey.mk`, so `plan/main` goes through `fab-plan-resolve`, and fab's own resolver does not resolve virtual packages: - `fablib/plan.py:205-206` takes the pool's answer and does `package_name = fname.split("_")[0]`, then `self._deps[deps[package_name]]`. The `.deb` filename's first field must be string-equal to the requested name. A `keel-fab_2.0.0_all.deb` returned for a request of `fab` raises an uncaught `KeyError`. - `_get_provided` (`plan.py:329`) reads `Provides` only from packages **already fetched**, and `provided` is only subtracted from `all_missing` at `plan.py:406`. It never causes a provider to be fetched. And the miss is silent: `missing` is initialised at `plan.py:366` and never mutated, while `brokendeps` is built by iterating it at `plan.py:421-422`, so `resolve()` returns `(spec, [])` even when `all_missing` is non-empty. A plan entry the pool cannot supply is dropped from the spec with no error. That is upstream code and not this pull request's doing, so it is not a blocker for the diff. But it means the deferral rationale is wrong for this site, and it is latent rather than immediate only because `apt/pool/main/f/fab/fab_1.1.1+keel1_all.deb` is still in the archive: TKLDev images built after the conversion would keep shipping the pre-rename fab while the build host runs `keel-fab`, and the day `fab` leaves the pool they would ship no fab at all without saying so. `plan/main:3` needs to become `keel-fab`, and it needs its own issue before the conversion, not after. ### HIGH 3. The newest release is exempt forever, which is #7's hole one version deep `bin/check-release-tags:95-105`. The exemption is right for a pull request. The problem is that nothing ever revokes it. `release-tags` has no `if:`, so it does run on `push: branches: [master]`, but the script exempts the newest entry there too. So `keel-fab 2.0.0` merges with the check green, gets built, gets installed on the host, and `bt-layer:214` writes `fab_version 2.0.0` into every manifest, while `git rev-parse keel-fab/2.0.0` fails, until somebody adds the *next* changelog entry. That is exactly the state #7 was filed about. Two ways to close it, either is fine: - have the `push: master` run require the newest release to be tagged, for example `bin/check-release-tags --require-newest`, and push the tag with the merge as was done for `fab/1.1.1+keel1` and `fab/1.1.1+keel2`; or - put the invariant where the defect actually happened, in `bin/build-package`: refuse to build a `.deb` whose top changelog version has no tag. The untagged `.deb` on 2026-09-27 was produced by a build, not by a merge. There is also a bypass worth knowing about: an `UNRELEASED` entry on top keeps the newest *released* entry exempt, because `proposed` is set from the first non-`UNRELEASED` entry. Verified: ``` $ bash bin/check-release-tags . # changelog: 4.0 UNRELEASED, 3.0 trixie, … p/3.0 is the release being proposed, not tagged yet ``` ### HIGH 4. The gate accepts a lightweight tag on a commit that is not in the branch `bin/check-release-tags:59-61` and `:107-120`. `has_tag` only checks `refs/tags/<name>` exists, and `tag_version` only compares the first line of that tree's changelog. Nothing checks the tag is annotated, and nothing checks it is reachable. Demonstrated on a scratch repository: two lightweight tags on an orphan branch whose commits contain entirely different content pass the gate. ``` p/2.0 names the commit that carries 2.0 p/1.0 names the commit that carries 1.0 rc=0 $ git merge-base --is-ancestor p/2.0 main || echo "NOT reachable from main" NOT reachable from main ``` The annotated part is not cosmetic. The note's other direction, `git describe --match '*/*'`, only sees annotated tags: ``` $ git describe --match '*/*' HEAD~1 fatal: No annotated tags can describe '6ce89b3…'. However, there were unannotated tags: try --tags. ``` So the two halves of the "in both directions" invariant are enforced to different standards, and the half that CI checks is the weaker one. Two lines fix it: ```bash [[ "$(git -C "$1" cat-file -t "refs/tags/$2")" == tag ]] || … # annotated git -C "$1" merge-base --is-ancestor "refs/tags/$2" HEAD || … # reachable ``` The tags actually pushed are annotated and on `master`, so this is about what the rule permits next time, not about what is there now. `tests/release-tags.sh` currently locks the looser behaviour in, because every fixture uses `git tag "$2"`. --- ### MEDIUM 5. `fab --version` can still write a wrong `fab_version`, through `FAB_SHARE_PATH` `fablib/version.py:22-24`. `share_path()` honours `FAB_SHARE_PATH`. Nothing else in `fab` or `fablib` reads that variable any more; its only other life is as a make variable, `share/product.mk:60` and `common/mk/turnkey.mk:26`, both `?=` and neither exported, so a normal `bt-layer` run is fine and I could not make it misfire. But an exported `FAB_SHARE_PATH` pointing at a checkout, which is a plausible thing for someone debugging fab to do, makes `package_version()` return `unknown`, `fab --version` print it and exit 0, and `bt-layer:214` write `fab_version unknown` into the manifest. `bt-layer` cannot catch it either: `fab_version "$(fab --version)"` is a command substitution in an argument list, so even a non-zero exit is discarded under `set -e`. Given the stated premise that a wrong provenance record is worse than none, the test override wants to be its own variable, say `FAB_VERSION_FILE`, so that nothing a build environment sets can redirect the version lookup. Making `--version` exit non-zero on `unknown` is worth doing too, but only helps once buildtasks#12 reads the status. The rest of that path is sound. I built the package and `/usr/share/fab/version` contains exactly `2.0.0`, `package_version()` returns `'2.0.0'`, and the file can never be empty because `read_text().strip() or UNKNOWN` covers it. `override_dh_install` writing into a directory `dh_install` must have created means a missing directory is a loud build failure, not a silent empty file, which is the right way round. ### MEDIUM 6. The "28 paths, 26 byte identical" measurement excludes 7 files without saying so I rebuilt the package and compared it against the `fab 1.1.1+keel2` installed on the host. The substantive claims all hold, and the `dh_python3` finding is real and well handled: `make-release-deb.py` and `turnkey-version.py` come out byte identical to the installed ones, `runtime.d/keel-fab.rtupdate` is present, and `fablib` lands in `/usr/lib/python3/dist-packages` rather than `python3.13`, which is the three-way result the two calls were for. The arithmetic is not reproducible as written. The old package ships 26 regular files plus 9 symlinks. 28 reconstructs only as 19 files plus 9 symlinks, that is, with the 4 `dist-info` files and the 3 `usr/share/doc/fab/` files dropped. All three things the summary does not mention live in that dropped set: - `usr/share/doc/fab/` becomes `usr/share/doc/keel-fab/`, so three paths the old package had are **not** present under their old names. Nothing in the organization reads that path, so the impact is nil, but "all 28 paths the old package had are present" is not what happened. - `changelog.gz` differs, `115f2f5c` to `20760196`. It is a third differing file, not two. - `usr/lib/python3/dist-packages/fab-1.1.0.dist-info/` is unchanged, still `fab`, still `1.1.0`. Also `usr/share/python3/runtime.d/fab.rtupdate` becomes `keel-fab.rtupdate`, which the body covers with a glob rather than calling it a rename. Restating it as "35 entries in the old package, 32 byte identical, 3 renamed by debhelper, 2 new" would be both truer and stronger. ### MEDIUM 7. `pyproject.toml` is untouched, so there are now three version sources `pyproject.toml:7-8` keeps `name = "fab"`, `version = "1.1.0"`. `importlib.metadata.version("fab")` on the built package returns `1.1.0` while dpkg says `keel-fab 2.0.0` and `/usr/share/fab/version` says `2.0.0`. Keeping it frozen is probably deliberate, since it is what keeps the four `dist-info` files byte identical, but nothing says so, and the next person to "fix" version reporting will reach for `importlib.metadata` and get `1.1.0`. One comment in `pyproject.toml` or in `debian/rules` settles it. ### MEDIUM 8. The native classification depends on there being no `upstream` remote, and 0008 invites one `apt/lib/build.sh:83`. The argument for the name is that all three native conditions now hold, and one of them is `git -C "$dir" remote | grep -qx upstream`. Decision 0008 keeps the fork upstream compatible with cherry-picks in both directions, and the ordinary way to cherry-pick from upstream is `git remote add upstream`. Whoever does that on the build host clone makes `bin/build-package` classify `keel-fab` as a rebuild and refuse `2.0.0` for lacking `+keelN`. Neither this pull request nor apt#15 mentions it. It wants either a note in `apt/lib/build.sh` or a condition that does not depend on a local remote name. ### MEDIUM 9. `bin/check-release-tags` has no floor Every non-`UNRELEASED` entry below the newest is a release, with no lower bound. It passes today only because the changelog has four entries and the oldest, upstream's `fab (1.1.1)`, is `UNRELEASED`. If a future merge from upstream brings real upstream entries into the file, the job demands `fab/1.1.0`, `fab/1.0.3` and so on forever, and upstream's tags are `v1.1.0`, `v1.0.3`. A floor, or a `--since`, costs a line. Related, smaller: a version with an epoch can never satisfy the rule, because `refs/tags/keel-fab/1:2.0.0` is not a legal refname. Worth a line in the header given the note proposes this rule for the other repositories. ### MEDIUM 10. The call-site inventory misses two live sites and understates one scope The central negative claim is sound and I reproduced it independently across all 40 repositories in the organization: no `debian/control` anywhere depends on `fab`, `import fab` is genuinely zero, no recipe Makefile, no shared makefile in `common` and nothing in `keel`, `keel-core` or `buildtasks` reads the package name. buildtasks and the 22 recipe Makefiles need no edit. Missing from the table of nine: - `handbook/docs/infra-recovery.md:96` : `apt-get install -y fab deck pool turnkey-chroot …`, the documented rebuild-from-a-fresh-Debian procedure. This is the one that combines with HIGH 1: it installs upstream's fab on a recovered host. - `apt/lib/build.sh:112,118` : `build_archive_version` passes the *repository* name to `apt-cache policy`. That is the same bug this pull request just fixed in `fab --version`. It is an error-path hint only, so nothing breaks, but it belongs in the row. - The handbook row scopes the change to `docs/build-host.md` "sections 2 and 3". Section 4, "Rebuilding and reinstalling fab", lines 88-111, is the runbook someone will actually execute, and it names `dpkg -l fab`, `apt-cache policy fab` and the pin file. In the other direction, "~30 literal `fab` assertions in `tests/build-package.bats`" overcounts: of the 34 occurrences only 4 are package-name reads, at `:56` and `:192-194`. The rest are the *repository* name, which is not changing. `build_changes_path:137` derives filenames from `dpkg-parsechangelog -S Source`, so the `.deb` and `.changes` names follow the rename on their own. ### MEDIUM 11. The reference counts are not reproducible, and the table contradicts the prose Measured across all 40 repositories, excluding `.git/`: | Symbol | Table | Measured | |---|---|---| | `fab-chroot` | ~100 | 116 | | `fab-apply-overlay` | 34 | 39 | | `fab-plan-resolve` | 25 | 28 | | `FAB_PATH` | ~256 | 298 | | `FAB_ARCH` | ~92 | 111 | | `/usr/share/fab` | 9 | 31 | | `import fab` | 0 | 0 | Low by 10 to 20 percent throughout and by 3.4 times for `/usr/share/fab`. Undercounting is the safe direction for a claim of the form "none of these reads the package name", so the conclusion is unaffected, but the prose says "roughly 380 references" while the table sums to 631. Say which tree and which exclusions produced the numbers, or round them off. ### MEDIUM 12. The coverage number is a pass rate `tests/coverage.sh:37-40` computes the share of checks that pass, so it reads 100 whenever the suites are green and can only fall when a test fails. Adding `packaging` and `release-tags` to `suites=` therefore does not measure whether their branches are exercised; the claim rests entirely on the header's hand enumeration. That is pre-existing and honestly described, but `docs/traps.md` already carries "100 percent line coverage on the same file" as something that misled this project once, and the two new suites are the first here where a branch can go unexercised without the number moving. Worth a sentence in `COVERAGE.md`. --- ### LOW - The count is 126, not 125. `19 + 56 + 36 + 15`, and I ran all four: `1..19`, `1..56`, `1..36`, `1..15`, zero `not ok`. The Measured table says 126 and the test plan says 125. - "nine hours before #7 was filed" is 10 hours 7 minutes. `19:58:53Z` to `06:06:20Z`. - "the build host's own `master`, which `git branch -vv` there still reports as `behind 4`" is behind 4 of a `origin/master` that is itself stale at `16730a5`; against the real `master` it is behind 6. The conclusion is right either way, and the sharper statement is that the host has not fetched since before #6. - "`shellcheck -x` clean on every file this branch adds or touches" is true at warning level and above. `tests/units.sh` and `tests/source-date-epoch.sh`, both touched here, carry 6 and 9 note-level `SC2317`. Identical counts on `master`, so nothing was introduced; the claim just wants "no warnings or errors". - `debian/rules:15` hardcodes `debian/keel-fab/`. It fails loudly if the name changes again, which is acceptable, but `$(shell dh_listpackages)` costs nothing. - One operator hazard worth a line in the runbook. `apt` can never co-install the two: with `--no-remove` it stops at `E: Packages need to be removed but remove is disabled`. `dpkg -i --force-conflicts --force-overwrite` can, and because `Replaces` is one-directional the result is that `keel-fab` keeps ownership of `/usr/bin/fab` and all nine aliases even though `fab` unpacked second. Removing `keel-fab` then deletes them, leaving `fab 1.1.1+keel2` installed, **`dpkg -V fab` exiting 0**, and no `/usr/bin/fab` and no `/usr/bin/fab-chroot` on the machine. So: never reach for `--force-conflicts` here, and verify with `dpkg -S /usr/bin/fab` rather than `dpkg -V`. ### On the conversion itself The mechanics are sound and I checked all three directions in a container. - Forward, no extra flags: `apt-get install ./keel-fab_2.0.0_all.deb` removes `fab`, installs `keel-fab`, and every path including the nine aliases ends up owned by `keel-fab`. - Back, no extra flags and no `--allow-downgrades` needed since the names differ: `apt-get install /root/src/fab_1.1.1+keel2_all.deb` removes `keel-fab` and restores `/usr/bin/fab-chroot`. - Failing halfway does leave a window with no fab at all, but it is recoverable offline from the kept `.deb`, which is the property that matters. So the reversibility claim holds. What is not yet true is that it is safe to schedule: HIGH 1 has to be closed in apt#15 first, with the negative pin and not only the widened stanza, and HIGH 2 needs `tkldev/plan/main` filed before the archive ever drops `fab`. ### Verdict No CRITICAL. Four HIGH, none of them in the diff itself: the packaging is right, the `dh_python3` work is right and well evidenced, the version choice is right and the test pins it, and #7's premise really was wrong. The HIGH items are in the deferral reasoning and in the reach of the new gate, and three of the four are a few lines each. **Warning.**
marcos-mendez commented 2026-09-29 00:40:24 -03:00 (Migrated from github.com)
Copy link

HIGH 3 and 4 are fixed in 6a1f93c (reproduced: an untagged newest release fails under --require-newest, and an UNRELEASED entry on top no longer exempts the release below it; a lightweight or unreachable tag fails). HIGH 1 and 2 gate the conversion, not this merge, and are apt#15 and tkldev#4; note the negative pin has to be Pin: release o=turnkeylinux, since Pin: origin "" only matches a local repository (reproduced, see apt#15). After merging, push an annotated keel-fab/2.0.0 on the merged head, or the default-branch release-tags job goes red.

HIGH 3 and 4 are fixed in 6a1f93c (reproduced: an untagged newest release fails under `--require-newest`, and an UNRELEASED entry on top no longer exempts the release below it; a lightweight or unreachable tag fails). HIGH 1 and 2 gate the conversion, not this merge, and are apt#15 and tkldev#4; note the negative pin has to be `Pin: release o=turnkeylinux`, since `Pin: origin ""` only matches a local repository (reproduced, see apt#15). After merging, push an annotated `keel-fab/2.0.0` on the merged head, or the default-branch `release-tags` job goes red.
This pull request has changes conflicting with the target branch.
  • debian/changelog
  • debian/control
View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u origin pkg/keel-fab:pkg/keel-fab
git switch pkg/keel-fab
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels   
accessibility

Barrier affecting people with disabilities

  
bug

Something isn't working

  
documentation

Improvements or additions to documentation

  
duplicate

This issue or pull request already exists

  
enhancement

New feature or request

  
good first issue

Good for newcomers

  
help wanted

Extra attention is needed

  
invalid

This doesn't seem right

  
question

Further information is requested

  
wontfix

This will not be worked on

No labels
accessibility
bug
documentation
duplicate
enhancement
good first issue
help wanted
invalid
question
wontfix
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
Keel-Linux/fab!9
No description provided.