Decision 0015: operator facing commands carry the Keel name #3
No reviewers
Labels
No labels
accessibility
bug
documentation
duplicate
enhancement
good first issue
help wanted
invalid
question
wontfix
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
Keel-Linux/handbook!3
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "docs/decision-0015-keel-named-commands"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Cross-reference only, nothing is closed from here: Keel-Linux/tracker#12 is the org-wide issue, and the implementation lives in its own repository issue, Keel-Linux/keel-wordpress#7, with Keel-Linux/keel-wordpress#6 as its pull request. The handbook carries the policy and nothing else.
Merge this before Keel-Linux/keel-wordpress#6, which cites decision 0015 in its
changelog,COVERAGE.md,tests/wrappers.batsand both script headers.docs/decisions/0015-operator-command-names.mdrecords the rule the maintainer decided on 2026-09-28: the Keel name is the real command, theturnkey-*name is kept as a symlink to it, and both keep working. It is a general policy, not a per-appliance choice, and the reason for keeping the old name is the same one that keeps/etc/turnkey_versionin 0014 — compatibility with TurnKey appliances is a stated property of this project (0008).Responding to the review
HIGH — the inventory contradicted the note's own scope rule. Correct, and it did so in the direction that makes a contributor under-apply the policy. The rule covers "a script installed by a package this project builds"; the table listed only the two keel-wordpress overlay files, the two in
common, and the foreign packages going to tracker#13. So the note handled somebody else's packages and silently dropped the ones we build.I verified the five myself against the dpkg file lists and against the forks' own trees, and added them:
/usr/sbin/turnkey-initinithooks 2.3.6+keel4Keel-Linux/inithooks,turnkey-init/usr/sbin/turnkey-sudoadmininithooks 2.3.6+keel4Keel-Linux/inithooks,turnkey-sudoadmin/usr/sbin/turnkey-install-security-updatesinithooks 2.3.6+keel4Keel-Linux/inithooks,turnkey-install-security-updates/usr/lib/inithooks/bin/turnkey-init-fenceinithooks 2.3.6+keel4Keel-Linux/inithooks,bin/turnkey-init-fence/usr/bin/turnkey-lexiconconfconsole 2.2.3+keel2Keel-Linux/confconsole,turnkey-lexiconAlso added, from LOW 1:
turnkey-artisanandturnkey-composer, tracked100755files incommonreached throughlaravel.mkandcomposer.mk; and the byte-identical second copy ofturnkey-mysql-install-perf-info-schemasinunit-mariadb(same blobc8f185b2), which the old table attributed tocommonalone, so anybody following it would have renamed one of two copies.turnkey-make-ssl-certfromturnkey-ssl 3.1.1is now listed too, explicitly out of scope, so the next reader does not have to derive that again.The section is restructured: fifteen distinct names grouped by what puts the file on the machine — appliance overlay,
commonoverlay, a package we build, a package somebody else builds — because that is what decides when each one can move. Eleven are ours.turnkey-init-fenceis marked as not onPATHand not a command an operator types, with the observation that the same name is also a conffile, a systemd unit and two firstboot hooks, which is a packaging question the note does not settle. And the section now closes by saying that when the table and the rule disagree, the rule wins — a table is a measurement of one day and this one has been corrected three times.I took the reviewer's framing on the
+keelsuffix: the rule reads on "a package this project builds", andturnkey-sysinfo1.1.0,turnkey-version1.1.3 andturnkey-ssl3.1.1 carry no such suffix whileinithooksandconfconsoledo. The "what a package owns does instead" section now opens by saying it is about packages we do not build, since getting that the wrong way round is precisely the mistake the first version made.MEDIUM — the
-dclaim. Correct, and worse in a note than in code, because a note is applied without asking. Measured with coreutils 9.7 and written into the note as a table:The property is the absence of
-L, not the presence of-d. keel-wordpress#6 now has a test asserting all three rows, so the claim is checked rather than written down.LOW 2 — the Decision section now states that the link is relative and why an absolute one breaks the build-time caller, and that the Keel name is the
turnkey-prefix replaced bykeel-and nothing else. LOW 3 — "must be covered", and it names who is owed those tests. LOW 4 — the reverse-dependency provenance says the container throughout. LOW 5 — merge order is stated at the top of both pull request descriptions.Test plan
Date,Status,## Decision, the three-part justification of brief section 10,## Consequences.dpkg -L/dpkg -Sfile lists,git ls-filesacross the organization's repositories, and the.mkfiles that pull each overlay.+keelversion.This repository ships no package and has no
package / changeloggate, so there is no changelog entry to add, and no checks are reported on the branch.Review
I checked the note's factual claims against fab's source, this organization's repositories and the booted
wordpress-democontainer rather than taking them as read. The reverse-dependency argument, theturnkey-detect-virtcorrection and thecommon-ownership correction all hold. One finding is HIGH: the note's inventory contradicts the note's own scope rule, and it does so in the direction that makes a future contributor under-apply the policy.Verified
apt-cache rdepends turnkey-sysinfois empty both installed-only and archive-wide, andturnkey-version's only reverse dependency isturnkey-sysinfo(turnkey-sysinfo: Depends: python3:any, turnkey-netinfo, turnkey-version). Versions are 1.1.0 and 1.1.3 as stated.dpkg -L turnkey-sysinfolists/usr/bin/turnkey-sysinfo,/usr/bin/turnkey-detect-virt,libsysinfo/{__init__,disk,memstats,virt}.pyand/usr/share/turnkey-sysinfo/contrib/motd. The library and the motd fragment are real and tracker#13 does need both names.commoncorrection: correct.overlays/php/usr/local/bin/turnkey-phpandoverlays/mysql/usr/local/bin/turnkey-mysql-install-perf-info-schemasare ordinary tracked100755files inKeel-Linux/common, reaching the image throughCOMMON_OVERLAYS. On the appliance both areNOT owned by any package, which settles "inherited" against tracker#12. The single caller insidecommonis real:overlays/mysql/usr/lib/confconsole/plugins.d/System_Settings/Mysql_perf_info.py:19-20calls it by bare name.HIGH — the table omits five commands that the note's own scope rule pulls in, and they ship on every appliance
Lines 15-20 against the table at 66-72.
The rule is: the policy applies to "every operator facing command a repository of this organization writes and names itself: a file in an appliance overlay, a file in the
commonoverlays, a script installed by a package this project builds". The carve-out is for "a command that only exists because some other project's package put it there".Measured on the booted
wordpress-democontainer, the third clause covers five commands the table never mentions:/usr/sbin/turnkey-initinithooks2.3.6+keel4/usr/sbin/turnkey-sudoadmininithooks2.3.6+keel4/usr/sbin/turnkey-install-security-updatesinithooks2.3.6+keel4/usr/lib/inithooks/bin/turnkey-init-fenceinithooks2.3.6+keel4/usr/bin/turnkey-lexiconconfconsole2.2.3+keel2Both packages carry a
+keelsuffix — they are built fromKeel-Linux/inithooksandKeel-Linux/confconsoleandkeel-wordpress's ownMakefile:40-41names them among "the project's own packages".turnkey-initis the most operator-facing command on the appliance; it is what an operator types to re-run first-boot configuration.Meanwhile the row the table does devote to packages sends
turnkey-sysinfo,turnkey-versionandturnkey-detect-virtto tracker#13 — and those come fromturnkey-sysinfo1.1.0 andturnkey-version1.1.3, neither carrying a+keelsuffix, i.e. the packages that are genuinely somebody else's. So the note handles the foreign packages and silently drops the ones this project already builds, which is the opposite of what the rule at 15-20 says. (A sixth,/usr/bin/turnkey-make-ssl-certfromturnkey-ssl3.1.1, is also unmentioned; that one is correctly out of scope under the carve-out, but the note never says so either.)The scenario this bites in is the one the note is written to prevent. A contributor picks up tracker#12, reads 0015 as the settled scope, sees
turnkey-initis not in the table and concludes it is inherited surface. Nothing corrects them: there is no gate for a name, the note says at 153-156 that onlygrep -rnfinds these, and the table has already been through two rounds of correction, which makes it read as authoritative.The fix is small — either add the rows, or add one sentence saying the table covers the commands reachable from tracker#12's inventory and that packaged commands from
inithooksandconfconsoleare unenumerated and still in scope.MEDIUM — the
-dclaim is the wrong half of the flag, in the document a contributor will copy fromLines 95-97: "
-dis--no-dereference --preserve=links, so a symlink in an overlay arrives in the image as a symlink."The conclusion is right and I verified it end to end. The attribution is not. In GNU cp,
-P/--no-dereferenceis already the default under-R; only-Ldereferences. Measured:What
-dcontributes tofab-apply-overlayis--preserve=links, i.e. hard links. Symlink survival hinges on nobody passing-L.This matters more here than in the code, because a decision note is applied without asking. As written it teaches that any copy step lacking
-dwill flatten links, which will get-dadded where it does nothing, or a perfectly sound mechanism rejected for not having it. And the verification claimed at 102-109 cannot catch the difference: I dropped-dfromtests/wrappers.bats's owncpand no test failed.Suggested: "
-Rcopies a symlink as a symlink unless-Lis given, and-dadditionally preserves hard links, so a symlink in an overlay arrives in the image as a symlink."LOW 1 — two more
commonoverlay commands, and a duplicate, unlistedcommon/overlays/artisan/usr/local/bin/turnkey-artisanandcommon/overlays/composer/usr/local/bin/turnkey-composerare tracked100755files incommon, reached throughlaravel.mk:3andcomposer.mk:1.turnkey-artisanis the samerunuser-based shape asturnkey-wp. Neither word appears anywhere in the note's 165 lines. They ship on no Keel appliance today, because nothing includes those mk files — which is exactly why they should be named rather than left to be rediscovered when something does.Separately,
unit-mariadb/overlay/usr/local/bin/turnkey-mysql-install-perf-info-schemasis a byte-identical second copy (same blob) of thecommonone. The table attributes that command tocommonalone, so a contributor following the note renames one of two copies and leaves the appliances built from the unit on the old name.LOW 2 — the policy does not say the link must be relative, and the natural choice is wrong
The three bullets at 11-13 are otherwise applicable without asking. They do not say whether
turnkey-xpoints atkeel-xor at/usr/local/bin/keel-x. keel-wordpress chose relative and asserts it (readlink=keel-wp), which is the right answer: an absolute link does not resolve insidefab-chroot, so a build-time caller likeconf.d/mainbreaks. I confirmed the failure mode — making the link absolute breaks three of that repository's tests. Absolute is the more natural thing to type, so one clause in the Decision section would save the next person the build.Related and smaller: the note never states how the Keel name is derived. The table implies a mechanical
turnkey-→keel-substitution; saying so removes the only other open question.LOW 3 — a requirement written in the present tense reads as a description
Lines 157-159: "The compatibility names are covered by tests that run them, in every repository that has a pair." Today that is true of keel-wordpress and of nothing else —
commonhas four such commands and no such test. As a Consequences bullet it is a rule; "must be covered" makes it one.LOW 4 — where a measurement came from
Line 47 says the reverse-dependency figures were "measured on the
wordpress-democontainer", line 54 says the dpkg file list is "on the build host". Both numbers are right; I reproduced them on the container. The note is careful about provenance everywhere else, so the mismatch is worth a word.LOW 5 — ordering against keel-wordpress#6
Keel-Linux/keel-wordpress#6cites "decision 0015 of the handbook" inchangelog,COVERAGE.md,tests/wrappers.batsand in both scripts' header comments. 0015 is not onmainhere yet. If #6 merges first the appliance ships comments pointing at a note that does not exist. Not a defect in either branch, just a merge order worth stating.What I verified versus what I inferred
Verified: every dpkg fact above, by
dpkg -L,dpkg -S,dpkg-query -Wandapt-cache rdependson the bootedwordpress-demo; thecommonandunit-mariadbfile inventory bygit ls-filesacross all 21 repositories; theMysql_perf_info.pycaller; cp's behaviour under-R,-dRand-LR; and that dropping-dfrom the keel-wordpress test changes no test result.Inferred: that the five
inithooks/confconsolecommands are in scope. That is my reading of lines 16-18 applied to packages carrying a+keelversion, and it is the reading that makes the rule and the carve-out consistent. If the maintainer intends "a package this project builds" to mean only packages whose name is Keel's, the note should say that, because on the present wording those five are in.Warning. The HIGH finding is not a reason to hold the decision — the policy itself is sound, well argued, and the three-part justification does the work section 10 asks of it. But the table will be read as the scope, and as it stands it excludes commands the note's own rule includes. I would add the rows or the disclaiming sentence before merge, and take the
-dcorrection at the same time since it is one clause.Re-review
I re-measured the inventory rather than reading the new tables, and checked the two claims that carry weight.
The HIGH is resolved
The five commands from packages this project builds are in, as their own group, with the
+keelversions that make them ours rather than inherited. The section at 148-156 states the reasoning that was missing, and 40-46 fixes the thing that actually caused the error: the previous heading said "owned by a Debian package", which conflated "packaged" with "somebody else's", and it now says "owned by a package this project does not build" with the+keeltest spelled out. That is the correction at the level of the mistake, not at the level of the symptom.Re-measured independently and the arithmetic holds. Fifteen distinct names, eleven ours:
turnkey-wp,turnkey-wordpress-update,turnkey-mysql-install-perf-info-schemas,turnkey-php,turnkey-artisan,turnkey-composer,turnkey-init,turnkey-sudoadmin,turnkey-install-security-updates,turnkey-init-fence,turnkey-lexicon; four not ours:turnkey-sysinfo,turnkey-detect-virt,turnkey-version,turnkey-make-ssl-cert. Justification bullet 1's "thirteen of the fifteen present on awordpress-democontainer, nine of those ours" also matches what the container reports —turnkey-artisanandturnkey-composerare the two absent, correctly, since nothing includeslaravel.mkorcomposer.mk.The blob claim is exact.
git rev-parsegivesc8f185b2ab5cdf273d1b9c11b0cf0e2ab073a792for bothcommon/overlays/mysql/usr/local/bin/turnkey-mysql-install-perf-info-schemasandunit-mariadb/overlay/usr/local/bin/turnkey-mysql-install-perf-info-schemas, and the working files share a sha256. The consequence the note draws at 99-105 is the right one and is the reason that row earns a place in a table it otherwise duplicates.The
turnkey-init-fenceparagraph at 158-164 is precise too.git ls-filesininithooksgives exactly what it names:bin/turnkey-init-fence,debian/inithooks.turnkey-init-fence.service,default/turnkey-init-fence,firstboot.d/30turnkey-init-fence,firstboot.d/97turnkey-init-fence-disableandturnkey-init-fence/htdocs/. Every item checks out, and drawing the line at "the compatibility rule covers the command, the unit and conffile names are a packaging question" is the right place to stop a decision note.turnkey-make-ssl-certas explicitly out of scope, withturnkey-ssl3.1.1 carrying no+keel, is correct and worth having written down — an unlisted name and a name listed as out of scope read very differently to the next person."When the table and the rule disagree, the rule wins" — genuine, on this evidence
I was ready to read that as a way to keep shipping a wrong table, and on its own it would be. It is not on its own. It arrives with a table that went from five names to fifteen, a stated grouping principle (by what puts the file on the machine, because that is what decides when each can move), and an instruction rather than an excuse: "the right fix is to add the row". A precedence clause that licenses inaction looks different — it appears instead of a measurement, not after a better one.
The clause is also just true. A table is a measurement of one day and this one will go stale the next time a repository gains a command; a policy whose scope is defined by an enumeration is a policy that silently shrinks. Putting the rule above the table is what stops that.
What would make it self-correcting rather than merely honest is the one thing still missing, and it is a LOW suggestion, not a condition. Nothing re-measures. The Consequences bullet already says finding these is
grep -rnand not a green build, which is exactly the observation that a five-line check over the organization's trees — everyturnkey-*executable, diffed against the names in this note — would turn into something that fails when the table drifts. The note argues for that check without proposing it.The
-dcorrectionTaken, and stated correctly at 191-210:
-Pis already the default under-R,-dadds--preserve=linksfor hard links, and the property the policy depends on is the absence of-L. The measured table with coreutils 9.7 matches what I measured. The paragraph at 198-210 explains why the attribution matters, which is the part that keeps the next person from adding-dwhere it does nothing.More to the point, it is no longer only prose. keel-wordpress now has a test that pins it, and I confirmed that test fails when the claim is violated: with a
cpshim that dereferences under-R, three tests go red including the one that names this property. The sentence at 219-222 is therefore accurate.The other items from the first pass
All taken. The relative-link rule and the name-derivation rule are now in the Decision section at 15-22, where a contributor applying the policy will meet them, with the
fab-chrootreason attached. "The compatibility names must be covered" replaces the present-tense description, and 279-281 names who owes them. The provenance mismatch is gone — line 70 now sayswordpress-democontainer rather than build host, which matches where the measurement came from. The closing correction aboutturnkey-detect-virtno longer frames itself as "five are really six", which was the sentence that made the old table sound settled.Verified versus inferred
Verified: the blob identity by
git rev-parsein both repositories and by sha256; the fifteen/eleven and thirteen/nine counts by re-listing from the repositories and from the container; theturnkey-init-fencefile set bygit ls-files; the+keelsuffixes bydpkg-query -Won the container; the cp semantics by measurement; and that the keel-wordpress test backing 219-222 fails when the property is false.Inferred: that the precedence clause is a resolution rather than an escape. That is a judgement about intent, and the evidence for it is that the measurement tripled in the same commit.
Approve. Clean above LOW. The single LOW — nothing re-measures the inventory — is a suggestion for a follow-up, not a condition on this note.