//patches:audition --patch is not repeatable, so an A/B needs two invocations #88

New issue

Closed

opened 2026-09-09 22:28:45 +00:00 by bcox · 1 comment

bcox commented

2026-09-09 22:28:45 +00:00

Owner

--patch assigns rather than accumulates, so the second occurrence overwrites the first:

bazel run -c opt //patches:audition -- --patch lpg-kick --patch ladder-kick /tmp/kicks

renders only ladder-kick, silently. No error, no warning — the first name is simply gone.

Comparing two patches is the obvious thing to want from this tool, and it is exactly what the flag looks like it supports. Making it repeatable is a small change in patches/audition.cc: accumulate into a vector and filter the registry against it, rather than holding one name.

Found while auditioning lpg-kick against ladder-kick at the close of ladder-gate; worked around by running the tool twice.

`--patch` assigns rather than accumulates, so the second occurrence overwrites the first: ``` bazel run -c opt //patches:audition -- --patch lpg-kick --patch ladder-kick /tmp/kicks ``` renders only `ladder-kick`, silently. No error, no warning — the first name is simply gone. Comparing two patches is the obvious thing to want from this tool, and it is exactly what the flag looks like it supports. Making it repeatable is a small change in `patches/audition.cc`: accumulate into a vector and filter the registry against it, rather than holding one name. Found while auditioning `lpg-kick` against `ladder-kick` at the close of `ladder-gate`; worked around by running the tool twice.

bcox commented

2026-09-09 23:56:08 +00:00

Author

Owner

Fixed on worktree-audition-flags, together with #69, #63 and #81.

--patch accumulates into a std::vector<std::string> and the registry is filtered against membership, so the command in this issue renders both:

lpg-kick             1.50 s  peak   -7.0 dBFS  range any
ladder-kick          1.35 s  peak   -7.8 dBFS  range any

Two notes from doing it.

The file already disagreed with itself. Eight lines below the flag, a repeated positional argument gets audition: one output directory, not two — an explicit refusal rather than last-one-wins. --patch silently keeping its last occurrence was the outlier, so accumulating aligns it with the convention already in the file rather than inventing one.

A typo now costs one run, not one run per name. With the flag repeatable, validating only the first selection would mean finding a second typo on the next run. UnknownSelections reports every name the bank lacks, in the order given:

audition: no patch named lpg_kick
audition: no patch named ladderkick
The bank is:
  hello
  ...

The parse, the selection and the output directory moved to patches/audition_options.h behind a cc_library, because a cc_binary is not testable and rendering the bank is the slowest possible way to find a mistyped name. //patches:audition_options_test covers all of it; every assertion was verified by injecting the bug it names, including this one — the assigning parser fails RepeatedPatchFlagKeepsEveryNameGiven and nothing else, and fixing the parser alone leaves the first-unknown-only report failing for its own reason.

Fixed on `worktree-audition-flags`, together with #69, #63 and #81. `--patch` accumulates into a `std::vector<std::string>` and the registry is filtered against membership, so the command in this issue renders both: ``` lpg-kick 1.50 s peak -7.0 dBFS range any ladder-kick 1.35 s peak -7.8 dBFS range any ``` Two notes from doing it. **The file already disagreed with itself.** Eight lines below the flag, a repeated positional argument gets `audition: one output directory, not two` — an explicit refusal rather than last-one-wins. `--patch` silently keeping its last occurrence was the outlier, so accumulating aligns it with the convention already in the file rather than inventing one. **A typo now costs one run, not one run per name.** With the flag repeatable, validating only the first selection would mean finding a second typo on the next run. `UnknownSelections` reports every name the bank lacks, in the order given: ``` audition: no patch named lpg_kick audition: no patch named ladderkick The bank is: hello ... ``` The parse, the selection and the output directory moved to `patches/audition_options.h` behind a `cc_library`, because a `cc_binary` is not testable and rendering the bank is the slowest possible way to find a mistyped name. `//patches:audition_options_test` covers all of it; every assertion was verified by injecting the bug it names, including this one — the assigning parser fails `RepeatedPatchFlagKeepsEveryNameGiven` and nothing else, and fixing the parser alone leaves the first-unknown-only report failing for its own reason.