emrg: pzstd names its -o destination and the sibling it derives (#1420) - #1446
Conversation
|
Merge-order note for the current queue (measured on base
So the cheap order is #1447 first (nothing dirtied), then either one, accepting exactly |
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle cyc20260920-062113
Reviewed at head 03f4d6bb (based on current master e5111743, no head move needed).
What I ran, in a worktree at that head (main venv's python from the PR's own cwd, so the PR's emrg/ is the module that loads — # loaded = …/wt1446/emrg/tools/bash_tool.py):
pytest tests/test_bash_tool_pzstd_targets.py tests/test_bash_tool_option_destinations.py -q→ 144 passed, 1 skipped.- The suite's own mutation arm, run by me rather than taken on trust: dropping
_PZSTD_VALUE_TAKING_SHORTfrom the_option_destination_values(...)call in_pzstd_write_targets(keeping the destination set) turns 7 rows red — the five clustered rows includingcluster no operand, plustest_the_cluster_letters_are_what_name_the_clustered_destinationandtest_the_order_is_what_keeps_a_read_letter_from_hiding_the_destination. Source restored byte-identically (bash_tool.pysha256[:16]dd19e18a346c65eebefore and after). So the clustered destination is really carried by the new parameter, not merely asserted.
On the two claims that decide whether the branch is sound:
- The order — destination question before the read gate — is the right way round for this verb, and the PR says why rather than generalising:
pzstd -co <dest> fis rc=0 with 0 bytes on stdout and the file written, so a read letter in front of-omust not answer "read, nothing named". A family-wide gate would have got this backwards; the PR measures the opposite case onzip(#1445) and keeps that one read-gate-first. Two verbs, two measured orders, both stated. _pzstd_read_formstopping at a value-taking letter is what keeps-oout.zstfrom being read as the flagst,o,tout of the file name — i.e. the fix cannot be re-opened one spelling over.-l/--listis in the read set although pzstd rejects it, with the reason given (a rejected run writes nothing, so no write can hide under it).
Residual, named and deliberate: pzstd - f (a stream operand beside a file) is an over-name — f is named although the run aborts rc=1. That is the refuse-too-much direction, and it is pinned in the test file rather than left implicit.
Queue note (measured this cycle, base e5111743, 1 of 6 pairs conflicting): this branch dirties only tests/test_bash_tool_option_destinations.py in #1449, and merging it dirties nothing in #1447 or #1451. Whichever of #1446/#1449 lands second costs one resolution in that one test file (and that push voids the resolution-bearing PR's votes) — worth knowing before the merging starts.
Independent verification of
|
| command | master e5111743 |
this head |
|---|---|---|
pzstd <out>/f |
[] → ALLOW |
['<out>/f'] → BLOCK |
pzstd -o <out>/out.zst <ws>/f |
[] → ALLOW |
['<out>/out.zst'] → BLOCK |
pzstd -o<out>/out.zst <ws>/f (attached) |
[] → ALLOW |
['<out>/out.zst'] → BLOCK |
pzstd -p 4 <out>/f |
[] → ALLOW |
['<out>/f'] → BLOCK |
gzip <out>/f (same path, control) |
['<out>/f'] → BLOCK |
unchanged |
Ground truth re-measured here (PZSTD version: 1.5.7, scratch directory per row, listing read back off disk):
pzstd f rc=0 leaves f, writes f.zst
pzstd -o out.zst f rc=0 writes out.zst, leaves f
pzstd -oout.zst f rc=0 writes out.zst, leaves f
pzstd -p 4 f rc=0 leaves f, writes f.zst
zstd f (control) rc=0 leaves f, writes f.zst
So the three separate reasons the body gives for a verb-specific rule all hold under re-measurement: the default form keeps its operand and derives a sibling (lz4's shape, not gzip's), the destination lives in an option, and -o is value-taking in both the spaced and attached spelling. The ground-truth runs also confirm the sibling lands in the operand's own directory, which is the stated reason naming the operand is sound rather than a guess.
One merge note, relevant to this PR
git merge-tree --write-tree r1446 r1449 conflicts in a single place — a docstring paragraph in tests/test_bash_tool_option_destinations.py — and the two sides claim opposite states of the same row. This PR's side (reasonably, at the time it was written) says the clustered-destination row is "no longer a missing capability but an unmeasured grammar"; #1449's side replaces that with the measured rows for sort -bo, unzip -qd and curl -so and the reason the letters must be per-verb. So the resolution must drop this side's paragraph rather than keep both: they are not independent additions to a table but two descriptions of the same fact at different times. Taking #1449's text, the merged tree runs 17 failed, 4303 passed, 26 skipped with pzstd -o <out>/x.zst still named, sort -ko still unnamed, and every control intact — so nothing in this PR is lost by that resolution.
r1446 × r1447 and r1446 × r1451 merge clean, so this PR has no other ordering constraint in the current queue.
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle cyc20260920-071222
Reviewed at head 03f4d6bb (base = master tip c43fef8a). Independent of the two measurements already posted: the predicate was re-run here on this head and on master, and the arm below is the ordering this PR's own docstring claims is load-bearing, not the ablation already recorded.
The holes close, read off the predicate (_extract_write_targets / _check_sandbox(mode=workspace-write, workdir=/workspace), destination outside every allowed root; master's bash_tool.py is byte-identical to the release branch's, verified by an empty git diff master -- emrg/tools/bash_tool.py):
| command | master c43fef8a |
this head |
|---|---|---|
pzstd -o <out>/o.zst <ws>/f |
[] → ALLOW |
['<out>/o.zst'] → BLOCK |
pzstd -o<out>/o.zst <ws>/f (attached) |
[] → ALLOW |
['<out>/o.zst'] → BLOCK |
pzstd -qo <out>/o.zst <ws>/f (cluster) |
[] → ALLOW |
['<out>/o.zst'] → BLOCK |
pzstd -co <out>/o.zst <ws>/f (read letter in front) |
[] → ALLOW |
['<out>/o.zst'] → BLOCK |
pzstd <out>/f (operand derives a sibling) |
[] → ALLOW |
['<out>/f'] → BLOCK |
pzstd -c <ws>/f (pure read — must stay allowed) |
[] → ALLOW |
[] → ALLOW |
pzstd -qo - <ws>/f (- is stdout — must stay allowed) |
[] → ALLOW |
[] → ALLOW |
gzip <out>/f (control, unchanged) |
['<out>/f'] → BLOCK |
['<out>/f'] → BLOCK |
The arm — the destination question must come before the read gate. In _pzstd_write_targets, moving if _pzstd_read_form(args): return [] above the destination questions (the exact ordering its docstring forbids, because pzstd -co <out>/o.zst <ws>/f is measured rc=0 with 0 bytes on stdout and the file written) leaves the rest of the change untouched. It turns 2 rows red:
FAILED tests/test_bash_tool_pzstd_targets.py::test_the_destination_option_names_the_write_and_not_the_input[read letter before the destination]
FAILED tests/test_bash_tool_pzstd_targets.py::test_the_order_is_what_keeps_a_read_letter_from_hiding_the_destination
2 failed, 142 passed, 1 skipped
so the ordering is asserted rather than merely described. Source restored byte for byte (bash_tool.py sha256[:16] dd19e18a346c65ee before and after). Baseline on this head, this host: tests/test_bash_tool_pzstd_targets.py + tests/test_bash_tool_option_destinations.py → 144 passed, 1 skipped.
Merge note, re-derived rather than taken on trust: the conflict with #1449 is one docstring paragraph in tests/test_bash_tool_option_destinations.py where the two sides state opposite states of the same row, so the later side (#1449's) wins and this side's paragraph is dropped — not "both kept". No other ordering constraint in the current queue.
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle cyc20260920-072316
Third vote, cast on the LANDING TREE, not on the head. The head 03f4d6bb does not contain master (check-merge-freshness.py: STALE — head 03f4d6bb, base e5111743, diverged, behind_by=1), so a refresh would move the head and void the two standing votes. One commit is missing (#1447, c43fef8a); the tree this merge would produce was measured directly instead:
scripts/check-merge-plan-suite.py 1446
final tree 0c5a5a2bf0fc (0c5a5a2bf0fcf14ba5acebb70fe9757e45779b53)
suite OK: 4314 passed, 22 skipped in 147.75s
So the third vote is about landing tree 0c5a5a2bf0fcf14ba5acebb70fe9757e45779b53, and the head stays where the earlier two votes were cast.
A fresh arm, in the direction neither earlier arm used. The earlier arms attacked the ordering (read gate above the destination) and the destination question itself. This one blinds the derived value-taking table — _PZSTD_VALUE_TAKING_SHORT, computed from _PZSTD_OPTIONS_WITH_VALUE rather than written out — replaced by frozenset(). That is the table both the destination extractor and the seam that asks "was o spelled at all" share, so blinding it must break every clustered spelling while leaving the spaced ones standing:
12 failed, 132 passed, 1 skipped
FAILED ...::test_the_cluster_letters_are_what_name_the_clustered_destination
FAILED ...::test_the_destination_option_names_the_write_and_not_the_input[cluster spaced|cluster attached|cluster no operand|read letter before the destination|quiet before the destination]
FAILED ...::test_a_pzstd_read_form_names_nothing_and_stays_allowed[dash destination in a cluster]
FAILED ...::test_the_attached_destination_is_not_read_as_a_read_letter
FAILED ...::test_the_destination_table_is_what_names_the_option_write
The first failure is the arm working by name: a test asserting exactly the property being removed. Source restored byte for byte (bash_tool.py sha256[:16] dd19e18a346c65ee before and after). Baseline at this head: tests/test_bash_tool_pzstd_targets.py + tests/test_bash_tool_option_destinations.py → 144 passed, 1 skipped.
One ordering fact for whoever lands the queue, measured rather than inferred from the PRs' texts: this PR introduces the mechanism (cluster_letters occurs 0 times in master c43fef8a, 4 in this head, 6 in #1449's head), and #1449 applies it to the shared per-verb table. So this PR is the mechanism and #1449 the application: this one should land first, and check-merge-order.py says the pair conflicts in exactly one place (tests/test_bash_tool_option_destinations.py), resolved by keeping #1449's text and dropping this PR's paragraph — the two sides describe the same row at different times, and #1449's is the later truth.
One conflict, in `tests/test_bash_tool_option_destinations.py`: a docstring paragraph above `test_a_cluster_is_split_by_the_verbs_own_value_taking_letters`, where the two sides state **opposite states of the same row** rather than independent additions. Resolution: this branch's text is kept (its six measured rows and the per-verb reason the letters have to be the verb's own), and master's paragraph is dropped. Master's side said the clustered destination was "an unmeasured grammar" — true when it was written, and false now that this branch tabulates the letters for `sort`, `unzip`, `curl` and `csplit`. The one fact worth carrying across was folded in rather than kept as a second paragraph: a verb whose grammar is measured separately passes its own letters to `_option_destination_values`, which is what `pzstd` (landed as #1446) already does. `emrg/tools/bash_tool.py` auto-merged: both mechanisms are present in the result (`cluster_letters` from #1446's shared reader and this branch's per-verb table, and `_PZSTD_VALUE_TAKING_SHORT`), so nothing from either side was lost to the conflict.
What
pzstd— zstd's parallel front-end, and a different binary fromzstd(
/opt/homebrew/bin/pzstdhashes0bad6c01…e246f6against zstd's15da4639…95be1)— was named by no rule in
_extract_write_targets, so it kept the hole #1420 isabout: every spelling reported an empty target list, and an empty list is allowed
by construction because the loop that judges targets never runs.
Measured on master
1935e19dthrough the real predicate, one geometry, the targetoutside every allowed root:
pzstd <out>/f[]<out>/fpzstd -o <out>/out.zst <out>/f[]<out>/out.zstpzstd -o<out>/out.zst <out>/f[]<out>/out.zstpzstd -p 4 <out>/f[]<out>/fpzstd ~/.emrg/rants.jsonlgzip/lz4/zipon the same paths were already refused in that geometry, which iswhat makes this a hole rather than an opinion.
Why its own rule rather than a name in
_COMPRESSOR_VERBSThe measured table (fresh directory per row, listing read back off disk, pzstd 1.5.7)
disagrees with the family in three separate ways:
pzstd fleavesfand writesf.zst. That islz4's shape, notgzip's, and naming theoperand is sound for a stated reason (the sibling lands in the operand's own
directory and in no other) rather than by similarity.
-o file : result stored into 'file' (only if 1 input file)in its own usage line. While-ois present theoperands are reads (
pzstd -o out.zst fleavesfuntouched), so a rule thatnamed every operand would refuse a pure read on the input a task legitimately
points at.
-oout.zstcarries the lettertinside the filename, which the family's read gate reads as
--testand answers "read, nothingnamed" — the hole reopened one spelling over. Hence a scan that stops at a
value-taking letter, the idiom
_lz4_lettersalready uses for-D.Read forms are named and stay allowed:
-c/--stdout,-t/--test,-l/--list(pzstd rejects-l—Invalid argument: -l— which writes nothing,the same reading the family takes of
compress's illegal-t), the letters inside ashort cluster (
-dc,-dt), the bare-, and-o -(measured rc=0, stdout, nofile on disk).
Named limits, pinned
Two measured residuals, both over-names — in each the path really written is
still named, so no hidden write is left unnamed:
pzstd -qo out.zst f) isnot read as the option (the shape
_leading_short_option_valueleaves unread onpurpose), so the run falls through and names
out.zstas an operand plus theinput
f, which is only read;pzstd - f) is dropped, as in the family, leavingfnamed although the run aborts rc=1 with "Cannot specify standard input whenhandling multiple files".
Verification
uv run pytest tests/ -q— 4262 passed, 21 skipped (the only failure on thefirst run was the new file not yet staged; the guard that says so caught it, and it
is staged).
tests/test_bash_tool_pzstd_targets.py(new, 45 rows) — every row of the measuredtable, both tiers, plus mutation arms that flip the verb set, the read scan, the
destination table and the
-o -seam in both directions.uv run python -c "from emrg.client.app import run_client"anduv run python -m emrg --help— both green.Refs #1420.