Skip to content

A sweep over shell source must strip comments AND decide about string literals: four guards got this wrong in one week #1123

Description

@OffgridwithJD

A grep over shell source cannot tell an author's description of code from the code. Four guards and sweeps got this wrong in one week, in both directions, and every one of them was silent.

The four

1. selftest/070 flagged its own subject's prose. A comment in run_all_versions.sh reading "reads only the | sort form" contained the literal string the guard greps for. The guard was right about the pattern and wrong about the line. Fixed in #1118 by reading code only.

2. grep -l pgc_setup counted a comment as a call. My comment in #1117 claimed twenty suites never call pgc_setup. Six of them name it only in a comment saying they skip it deliberately. The real number is 28, and the wrong one would have put six suites on the wrong side of the claim the comment itself was making.

3. The same trap dropped three suites from #1121's population. grep -cE 'pgc_setup' as an exclusion removed exactly the suites that document skipping it:

decode_interrupts:37  # No cluster needed; pgc_setup is skipped deliberately.
hilbert_curve:214     # No cluster, so pgc_setup is skipped deliberately
wal_envelope:35       # No cluster needed; pgc_setup is skipped deliberately.

The comment saying "I skip this" read as "I call this". 233 records missed — hilbert_curve alone is 184, the largest suite in the set.

4. A record-pattern matched echo strings. My affected-set sweep flagged run_all_versions.sh, whose only matches are the word check inside its own output strings. The runner never calls pgc_record at all.

Two directions, both silent

Instances 1 and 4 count prose as code and inflate a set with files that are fine. Instances 2 and 3 count prose as code in an exclusion and drop files that are not fine. Neither announces itself: an over-inclusive sweep looks like a strict guard, and an under-inclusive one looks like a clean tree.

The shape of the rule

Stripping comments is only half the decision. String literals are the other half, and it is a separate choice:

Any sweep over shell source must strip comments and declare whether string literals count.

We have consistently made only the first decision, and instance 4 is what the second one costs.

There is also a third lever that removes the question entirely where it applies: choose a population that cannot contain prose about itself. Switching my sweep from test/*.sh to the registered suites dropped run_all_versions.sh out of scope, because the runner is not a suite. That fixed instance 4 without any pattern work.

What I could not measure, and am not claiming

I tried to count how many existing sweeps are exposed — an unanchored pattern can match a comment, a ^-anchored one cannot, because a comment line begins with #. My classifier extracted the wrong capture (the outer "$(grep ... rather than the grep's own pattern) and reported 54 exposed while including obviously immune cases like '^pgc_reconcile_records()'. So I have no trustworthy population count and am not publishing one.

What I can say: harness_selftest is green on ab8feef, so none of the existing sweeps is wrong today. This is a latent class with four realised instances in adjacent work, not a live failure.

Suggested

  1. A short note in CONTEXT.md beside the existing sweep guidance: strip comments, decide about strings, and prefer a population that excludes prose about itself.
  2. If someone wants the count I could not get: classify each sweep's own pattern (not the surrounding substitution) as anchored or not, and assert that every unanchored sweep strips comments.

Filed after @jdatcmd and I hit instances 2 and 3 within an hour of each other from opposite sides — which is what suggested the cause is the tool rather than either of us.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions