[brettheap]

Lane: openreposhape-4 (openRepoShape-4)
Claim: #331 (comment)

What

validate-family.py now finds member-duplicate under --no-members too, and
finds it in any case: IRRS and irrs are one finding that names both rows
as written.

Why

member-duplicate was raised in _member_project_name, which only the
members-checked loop reaches, and _check_members returns before that loop
under --no-members: the run a private family's CI falls back to when its
members cannot be checked out. A family.yaml of two rows project: IRRS
passed it (family ok, exit 0), so a duplicate row passed the one gate that
ran. The comparison was exact besides, so rows IRRS and irrs with distinct
ids passed in every mode, though members/IRRS and members/irrs are one
directory on macOS and Windows. family.py add refuses a case variant now, so
only a hand edit makes such rows, and this is the gate that finds the hand edit.

The file is one of the copies a holder pins, so this reaches a holder as a
shape update (update-shape.py), not by itself.

The rulings, as implemented

1. Its own function. _check_member_names_unique(members, report), defined
   just above _check_member_ids_unique, reads members: alone and is called
   from _check_members before the --no-members return, as
   member-duplicate-id is: one added call line, placed BEFORE the
   _check_member_ids_unique(...) call so it does not touch the line family.py: add refuses member-path-registered when a .gitmodules section under another name already gives the mount path, and validate-family.py finds gitmodules-path-twice, so add no longer commits two sections for one path that a recursive clone resolves to the later one, and a holder that already has such a ghost section fails its own gate (#300) #317
   adds after it. _member_project_name stops making the finding but still
   returns None for a name it has seen, so that row's other checks stay
   skipped as they were. The finding is made once, in every mode, and no other
   finding moves.
2. In any case. Two rows whose project: differs only in case are one
   finding, member-duplicate, naming both rows as written and saying they are
   one directory on a case-insensitive disk. The fold is str.casefold(), as
   the standard's other same-path-in-any-case checks use it
   (shape_materialize.py, adopt-project.py): under --no-members any
   string is a project:, and a disk folds more than ASCII case (the long s,
   the sigmas). On every name family.py can write (ASCII) it agrees with the
   lower() that Family.member uses (family.py add compares --member with the existing rows case-sensitively, so --member inkrouter/irrs on a family that already pins InkRouter/IRRS mounts the same repository again as members/irrs, both gates pass a family that pins one project twice, and bump --member irss's refusal recommends exactly that add #206). An exact repeat keeps its text
   byte for byte (tests/test_family.py pins it). Each later row is one
   finding against the first row that holds its name.
3. No other finding changes. member-duplicate-id (family.py: add refuses a case-variant of a member the family already pins as member-already-present naming the row that pins it, bump and remove find a member in any case, and validate-family.py refuses two rows with one id, so --member inkrouter/irrs beside InkRouter/IRRS no longer mounts one project twice past both gates and bump --member irss no longer recommends that add (#206) #221) is untouched,
   its docstring included (it still leaves an exactly-repeated name to
   member-duplicate, which is still true).
4. _check_members's docstring is not edited (family.py: add refuses member-path-registered when a .gitmodules section under another name already gives the mount path, and validate-family.py finds gitmodules-path-twice, so add no longer commits two sections for one path that a recursive clone resolves to the later one, and a holder that already has such a ghost section fails its own gate (#300) #317 rewrites it).

The case finding ends: "... and a holder there can keep only one of them.
Whether the rows pin one project or two, one of them has to go: keep the row
whose repository: is the member's and drop the other." It names no command:
family.py remove cannot take out a row that was never mounted (measured:
error: pathspec 'members/irrs' did not match any file(s) known to git), which
is the only kind a macOS or Windows holder can hold.

Tests

tests/test_validate_family_duplicates.py, 20 rows, on the family fixture of
tests/test_family.py:

• one row twice under --no-members: member-duplicate, exit 1, the finding
  alone, beside the SKIPPED (--no-members) line;
• the same row twice with members checked and under --pins, the repeat once
  byte for byte and once pinned to 40 zeros: one finding, not two, no refusal;
• IRRS and irrs with distinct ids under --no-members, in both orders:
  both rows named as written, not the exact finding's text, no
  member-duplicate-id;
• IRRS / irrs spelt with the long s, and SAS in capital sigmas against
  small ones (pairs lower() keeps apart), under --no-members: one finding;
• the finding's closing sentence says "one project or two" and names no
  command, and no longer says a family pins each project once;
• IRRS, irrs, Irrs, irrs: three findings in file order, each against
  the first row that holds the name;
• distinct names (IRRS, IRRS2, IRR, IRSS) ok under --no-members, and
  the unmodified holder ok in all three modes;
• a row with no project: and a row that is no mapping are skipped, no
  traceback;
• on a case-sensitive disk only: two real mounts, members/IRRS and
  members/irrs (IRQS, id irqs), in members-checked, --pins and
  --no-members runs, each one finding. macOS and Windows hold the two
  directories as one, so that row skips there; every other row builds its rows
  in family.yaml and runs on all three.

Run (container py-bench, git 2.43.0, git-filter-repo 2.47.0 on PATH, a gh
shim that exits 97, GIT_* identity exported, no network):

/usr/bin/python3 -m pytest tests/test_validate_family_duplicates.py tests/test_family.py tests/test_update_shape.py tests/test_update_shape_add.py tests/test_shape_doctor.py -q

Result on head 3d8d65e: 455 passed, 3 skipped in 6m45s, exit 0 (458
collected; my 20 rows: 20 passed, 0 skipped on Linux). It was 450 passed,
3 skipped at f8650c8 (453 collected). The 3 skips are
tests/test_shape_doctor.py:5330, "the PowerShell line is printed, and
powershell.exe exists, only on a Windows host". macOS and Windows are CI's
to run: the one disk-case row skips there by design, and the two fold rows
run the validator in UTF-8 mode (the Windows job also sets PYTHONUTF8=1).

Mutants

Each against tests/test_validate_family_duplicates.py alone (20 rows), over
the fix-round head 3d8d65e's bytes:

• the comparison exact again (setdefault(project, project)): 10 fail (both
  case rows, the long-s and sigma rows, the wording row, the not-the-exact-
  finding row, the later-row row, and the three mounted rows); it was 7 of 15
  at f8650c8;
• the call moved after the --no-members return: 9 fail (every --no-members
  row that expects a finding, the mounted one included); the members-checked
  rows still pass; it was 6 of 15 at f8650c8;
• the finding made in _member_project_name as well: 4 fail (all four
  one-finding-not-two rows); it was 2 of 15 at f8650c8;
• _member_project_name's exact-repeat skip deleted (the review's
  SHOULD-1): survived every row at f8650c8; now 2 fail, the two other-pin
  rows, which exit 2 with REFUSED member-commit-unresolvable and lose the
  finding (the identical rows still pass, as expected: a byte-for-byte copy
  checks out as cleanly as the row it copies);
• casefold() swapped back to lower(): 2 fail (the long-s and sigma rows);
• the old closing sentence ("A family pins each project once.") put back:
  1 fails (the wording row).

For the merge with #317

#317 (#300) adds _check_gitmodules_paths and a call line after
_check_member_ids_unique(...) in _check_members, and rewrites that
docstring and adds a bullet to the module docstring's WHAT IS CHECKED list.
This PR's one module-docstring line is its own bullet ahead of EVERY MEMBER,
away from the line #317 edits. git merge-tree --write-tree of the fix-round
head 3d8d65e with #317's current head f4cde3d is clean (exit 0, tree
a8dab23), with the three calls in the order names, ids, .gitmodules; of
f8650c8 with f4cde3d it was clean too (tree 8df7dcc). With current main
(a1b3a4b) the fix-round head is clean as well (tree f9708b0).

After #317 lands, _check_members's docstring will say "the two checks that
read a file alone" over three. This PR leaves that docstring alone; the
wording for whoever lands second is: "The three checks that read a file alone
-- family.yaml's names (#331) and ids (#206), and .gitmodules's paths
(#300) -- come first, so --no-members asks them too."

Fix round (review of f8650c8, MERGEABLE WITH NITS)

• SHOULD-1. The exact-repeat skip in _member_project_name was held by no
  test: the finding moved out of it, and a mutant that deleted the skip passed
  every row. The one-finding-not-two rows now also run a repeat pinned to 40
  zeros, which fails with the skip gone (mutant above).
• NIT-1. casefold() for lower(), with the comment corrected (the long
  s and the final sigma fold too; the fold is no one disk's own) and two rows.
• NIT-5. The case finding's closing sentence no longer assumes one
  project; see above.
• NIT-3, in part. WHAT IS CHECKED now lists the check, true at this head
  alone. _check_members's docstring waits for family.py: add refuses member-path-registered when a .gitmodules section under another name already gives the mount path, and validate-family.py finds gitmodules-path-twice, so add no longer commits two sections for one path that a recursive clone resolves to the later one, and a holder that already has such a ghost section fails its own gate (#300) #317.
• NIT-6. family.py: add refuses member-path-registered when a .gitmodules section under another name already gives the mount path, and validate-family.py finds gitmodules-path-twice, so add no longer commits two sections for one path that a recursive clone resolves to the later one, and a holder that already has such a ghost section fails its own gate (#300) #317's head in this body is f4cde3d now.
• NIT-2, NIT-4, OUT-1..3 are left as the review graded them: a same-id case
  variant keeps both findings; the members-checked run and the doctor answer
  an unmounted case variant with member-gitlink-absent, not this finding
  (_member_project_name could also skip a row whose FOLDED name it has seen,
  a follow-up); family.py remove cannot remove a never-mounted row;
  --no-members --help is stale since family.py add compares --member with the existing rows case-sensitively, so --member inkrouter/irrs on a family that already pins InkRouter/IRRS mounts the same repository again as members/irrs, both gates pass a family that pins one project twice, and bump --member irss's refusal recommends exactly that add #206; and --no-members asks nothing
  about project: itself.

Left out

• A case variant with the SAME id reports two findings, member-duplicate
  (the name) and member-duplicate-id (the project), where it reported the
  second alone before. Both are true and member-duplicate-id was ruled
  untouched here. (That is the state a pre-family.py add compares --member with the existing rows case-sensitively, so --member inkrouter/irrs on a family that already pins InkRouter/IRRS mounts the same repository again as members/irrs, both gates pass a family that pins one project twice, and bump --member irss's refusal recommends exactly that add #206 add --member inkrouter/irrs
  beside InkRouter/IRRS left.)
• --no-members still does not ask member-row (a row that is no mapping) or
  member-project (a row with no project:): family ok is printed over
  both. This change skips such rows, as _check_member_ids_unique does.
• A Refusal raised by a later member check (for example member-gitlink-absent
  on an irrs row nobody mounted) drops the findings already collected, so a
  members-checked run over case variants with only one mount shows the refusal
  and not this finding.

Closes #331

🤖 Generated with Claude Code

Summary by Sourcery

Ensure family validation rejects duplicate member directory names consistently across validation modes and case-insensitive filesystems.

Bug Fixes:

• Detect duplicate member project names during validation even when members are skipped with --no-members.
• Treat project names that differ only by case as duplicate member paths, including Unicode case-folding variants.

Enhancements:

• Preserve existing exact-duplicate reporting while consolidating name validation into a single pre-check that reports each later conflicting row once.

Tests:

• Add comprehensive validation coverage for exact duplicates, case variants, Unicode case folding, ordering, malformed rows, clean families, and mounted case-sensitive paths.

[sourcery-ai]

Sorry @brettheap, you've used your own review budget of 250,000 diff characters for the last 7 days.

You can request another review in 10 hours and 19 minutes by commenting @sourcery-ai review. Upgrade to get a review now.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud