Skip to content

adopt-project.py: under a one-byte locale such as Latin-1, a clone check makes again for a plan's clone record that neither spelling names is compared with both, so it is kept and the next run reads it rather than cloning again (#1112) - #1153

Merged
brettheap merged 7 commits into
mainfrom
l3-1112-at-the-record-both-spellings
Nov 8, 2026

Conversation

@brettheap

@brettheap brettheap commented Oct 22, 2026 •

Copy link
Copy Markdown
Contributor

Stacked on #1106: its base is #1106's branch. Once #1106 lands, lane 3 brings main into this branch and moves its base to main.

What changed

A follow-up outside v1.2.0 (Brett Heap, 2026-10-21T10:32:04Z). It lands only after the v1.2.0 tag, on a later merge word.

The bug (#1112). Take a one-byte locale such as Latin-1, a plan written in UTF-8 whose record is a clone plan made, and a work directory outside ASCII (Thing plus U+00E9).

The fix (adopt-project.py), following the issue's Direction:

  • A new helper, _recorded_paths(recorded). It lists the paths _recorded_path asks, in its order, with ~ expanded: first the spelling _recorded_spelling gives, then the plan's UTF-8 bytes where this locale spells the text otherwise.
  • _recorded_path now asks its paths through _recorded_paths. Its behaviour is unchanged: the spelling as written is asked first, and where neither path is there, the path returned is the spelling as written.
  • A new helper beside them, _at_the_record(target, path, there, recorded), decides whether a clone _recorded_source makes again is at the record. It compares the target's real path with every path _recorded_path asked:
    • where neither spelling names anything, each path in _recorded_paths;
    • where a spelling is there, that one and each asked before it, which names nothing (paths[:paths.index(path) + 1]).
  • _recorded_source calls the helper, and its docstring has a paragraph for adopt-project.py: under a one-byte locale, check compares a clone it made again for a plan's clone record only with the spelling as written, so it removes that clone instead of keeping it and every run clones again #1112.

A decision the Direction leaves open: never a spelling asked after the one there. The Direction says to compare with both spellings. Where a spelling is there, I compare with that one and with any asked before it, but not with one asked after it.

Tests (tests/test_adopt_one_byte_locale.py). The three new tests run check twice in a child under the Latin-1 locale the file already builds. A new gh stand-in clones a repository on disk.

Fix round

RV1153, lane 3's review of 590ab96e, posted on this PR, found no MUST, one SHOULD and two NITs. This round, ff33ba12, takes all three:

Measurements

Each check ran under a Latin-1 locale built with localedef, where a child reports iso8859-1 ISO-8859-1 0. The plan was written in UTF-8 mode for the clone at <run>/Thing<U+00E9>/source-readonly/Thing, recording cloned: true and repository: testorg/Thing. The clone was then moved away. Both runs used check --work-dir <run>/Thing<U+00E9>, with a stub gh that clones a repository on disk.

Code Run 1 Run 2
#1106's head 504a1dd6 exit 0, NOTE "no longer exists ... cloned again", gh calls 1, clone removed at the end exit 0, the same NOTE, gh calls 2, clone removed again
590ab96e exit 0, the NOTE, gh calls 1, clone kept exit 0, plan ok, no NOTE, gh calls still 1, clone kept
This head ff33ba12 exit 0, the NOTE, gh calls 1, clone kept exit 0, plan ok, no NOTE, gh calls still 1, clone kept

The reverse case (N1): the same plan, a file at the UTF-8 bytes where the clone was, and check --work-dir <run>/Thing<0xE9>, the spelling as written, which is free.

Code Run 1 Run 2
590ab96e exit 0, NOTE "no longer whole", gh calls 1, clone removed at the end exit 0, the same NOTE, gh calls 2, clone removed again
This head ff33ba12 exit 0, the NOTE, gh calls 1, clone kept exit 0, plan ok, no NOTE, gh calls still 1, clone kept

With a file at the spelling as written (M2's case), #1106's head, 590ab96e and this head give the same result: each run clones again, gh calls reach 2, and nothing is left.

Out of scope

#1112 has no "Needs a ruling" or "Documented limit" section. Nothing else was changed.

Tests

Each file ran on its own at ff33ba12, under a stub gh that exits 97, GH_TOKEN=bogus, an empty GH_CONFIG_DIR, GIT_SSH_COMMAND=false, GIT_CONFIG_GLOBAL=/dev/null and EMAIL set, with /usr/bin/python3 -m pytest -q -p no:cacheprovider (Python 3.12.3):

Test file Result
tests/test_adopt_one_byte_locale.py (-rs) 13 passed in 17.83s, none skipped
tests/test_adopt_unenterable_source_and_locale.py 13 passed in 10.15s
tests/test_adopt_c_locale_bytes.py 17 passed in 39.87s
tests/test_adopt_source_clone_record.py 100 passed in 38.11s
tests/test_scaffold_leg_path_collisions.py 104 passed in 4.84s
tests/test_repo_hygiene.py 100 passed in 2.82s
tests/test_shape_doctor.py 445 passed, 3 skipped in 767.13s (the counts of the run at 590ab96e)
tests/test_path_guard_hygiene.py, at the merge of this head into origin/main 038dafba 49 passed in 11.75s
tests/test_adopt_one_byte_locale.py (-rs), at that merge 13 passed in 17.43s, none skipped
tests/test_adopt_unenterable_source_and_locale.py, at that merge 13 passed in 9.02s

Mutants

Each mutant was applied to adopt-project.py, and tests/test_adopt_one_byte_locale.py was run. The file was then restored, and cmp found it identical.

Mutant Result Killed by
M1: _at_the_record compares with the path _recorded_path gave alone (asked = [path]), as on #1106's head KILLED, 2 failed test_a_clone_made_again_at_the_record_is_kept_under_a_one_byte_locale and test_a_clone_made_again_at_the_spelling_asked_first_is_kept (each second run said "cloned again")
M2: every path compared even where one is there (asked = paths) KILLED, 1 failed test_a_clone_made_again_beside_the_spelling_as_written_is_not_kept (the second run gave exit 2, REFUSED adopt-work-dir-occupied)
M3: _recorded_path returns the last path, not the spelling as written, where nothing is there KILLED, 1 failed test_the_record_is_asked_as_written_then_as_utf8[neither-latin1]
M4 (N1): where a spelling is there, that one alone (asked = [path] if there else paths), as at 590ab96e KILLED, 1 failed test_a_clone_made_again_at_the_spelling_asked_first_is_kept (the second run said "cloned again")

#1106's M4 (_keep_the_clone_at_the_record) is unchanged by this PR.

Merges

git merge-tree --write-tree at this head, ff33ba12:

Closes #1112

Lane: openreposhape-3 (openRepoShape-3)

🤖 Generated with Claude Code

Summary by Sourcery

Fix clone-record matching for non-UTF-8 locales so regenerated clones are retained and reused on subsequent checks.

Bug Fixes:

  • Prevent check from repeatedly recloning source repositories under one-byte locales by recognizing equivalent recorded path spellings when deciding whether a newly made clone is at the plan’s recorded location.

Enhancements:

  • Preserve the existing path-selection behavior while consistently handling both locale-specific and UTF-8 path representations, including occupied and previously queried spellings.

Tests:

  • Add Latin-1 integration coverage for retaining regenerated clones at the recorded path, removing clones beside an occupied spelling, and retaining clones at a spelling queried first.

Brett Heap and others added 3 commits October 21, 2026 11:05
…ads a plan written in UTF-8 whose source is there, asking the plan's UTF-8 bytes where the spelling as written names nothing, rather than refusing `plan-source-gone` (#1078)

Under a locale of one byte to a character, Latin-1 say, with
`PYTHONUTF8=0`, `os.fsencode` spells every character of a plan's
non-ASCII `source.local_path`, so #1072's `_recorded_spelling` asked the
path only as written: the e-acute of `Thing` + U+00E9 as the one byte
0xE9, where the name `plan` read in UTF-8 is 0xC3 0xA9. `check` refused
`plan-source-gone`, "names nothing that exists", of a source that is
there; recorded as a clone `plan` made, it "no longer exists" and `gh
repo clone` was asked.

`_recorded_path` now asks the spelling as written first and, where that
names nothing, the plan's UTF-8 bytes too. `_recorded_source` and
`_keep_the_clone_at_the_record` both ask it, so a clone `check --source`
made at the record is kept under Latin-1 as under C. Under C and POSIX
the lookup is #1072's, and every refusal's words are unchanged: where
neither spelling names anything, the path is the spelling as written.

The new tests build a Latin-1 locale with `localedef` from a charmap and
a locale source they write, and skip with localedef's words where none
can be built.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…source_and_locale.py: the clone at the record is asked under the C locale and Latin-1 through one helper, `kept_clones` (#1078)

SonarCloud counted the Latin-1 test's body as a 14-line copy of the C
test's: both built the record and a clone elsewhere, ran
`KEEP_THE_CLONE` in a child and parsed its answer. `kept_clones` does
that once, in the file that holds `KEEP_THE_CLONE`, and returns whether
the locale spells the record, the clones left and the one made
elsewhere. Each test keeps its own premise and its assert.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…heck` makes again for a plan's clone record that neither spelling names is compared with both spellings, so the clone at the plan's UTF-8 bytes is kept and the next run reads it rather than cloning again (#1112)

`_recorded_source` decided whether the clone it makes again is the plan's
record by comparing its target with the spelling as written alone. Under
Latin-1, a plan written in UTF-8 whose clone was in a work directory
outside ASCII was cloned again at the UTF-8 bytes `--work-dir` names, put
on the list of clones to remove, removed when `check` ended, and cloned
again by every run.

The new `_recorded_paths` lists the paths `_recorded_path` asks, in its
order, and `_recorded_path` now asks them through it. Where neither names
anything, the target's real path is compared with each of them. Where one
is there, the comparison is with that one alone, as before: a clone kept
beside a spelling as written that is there would be refused as
`adopt-work-dir-occupied` by the next run.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@sourcery-ai

sourcery-ai Bot commented Oct 22, 2026 •

Copy link
Copy Markdown

Reviewer's Guide

Implementation

Factor the recorded-path spellings into _recorded_paths, reuse that ordering in _recorded_path, and add _at_the_record so a recreated clone is retained when its real path matches a spelling _recorded_path asks, but not one asked after the spelling that is already there.

  • adopt-project.py L1949-1964, L1982-2012, L6852-6863 (+1 small edits)

Tests

Add Latin-1 regression coverage for retaining recreated clones at either spelling _recorded_path asks first, while confirming a clone beside an occupied spelling is not retained.

  • tests/test_adopt_one_byte_locale.py L22-28, L51-60, L261-378 (+2 small edits)

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@brettheap

Copy link
Copy Markdown
Contributor Author

(lane openRepoShape-3, 2026-10-22T15:38:24Z) Lane 3's independent review of #1153 (RV1153), head 590ab96. PARTIAL: stopped at a usage checkpoint. No MUST so far.

SHOULD

  • S1. New CRITICAL Sonar issue python:S3776 on _recorded_source (adopt-project.py:6567), cognitive complexity 16 > 15, caused by the conditional at :6681. The gate still passes. Fix: move :6677-6681 into a module-level helper _at_the_record(target, path, there, recorded) that computes realpath(target) and compares it with [path] if there else _recorded_paths(recorded); _recorded_source calls it. SonarQube's snippet analysis reports S3776 at 16 for the method as it is, and 0 issues with the helper.

NIT

Checked

Lane 3's plan: S1 and N1 go into a fix round on #1153 in the next usage window: the _at_the_record helper, with N1's wider comparison folded in and a test for the reverse case if the measured behaviour holds there, and M1 and M2 re-pointed. N2: the Sourcery block comes off and #1154 joins the merge list. RV1153 then finishes test_shape_doctor.py and the merged-tree tests. #1153 lands only after #1106, after the v1.2.0 tag.

Lane: openreposhape-3 (openRepoShape-3)

…plan's UTF-8 bytes hold its clone no longer whole and `--work-dir` names the free spelling as written, the clone `check` makes again there is kept and the next run reads it, and the comparison moves into `_at_the_record` (#1112)

The fix round on #1153 from its review, RV1153:

- S1: the comparison of a clone made again with the record moves out of
  `Plan._recorded_source` into a module-level helper, `_at_the_record`,
  beside `_recorded_paths`, so the method's cognitive complexity falls
  from 16 back to 15 (SonarQube python:S3776).
- N1: the helper compares the clone's target with every path
  `_recorded_path` asked: where a spelling is there, that one and each
  asked before it, which names nothing. Under Latin-1, the UTF-8 bytes
  holding a file where the clone was and `--work-dir` naming the free
  byte 0xE9, the clone made at 0xE9 was removed when `check` ended and
  every run cloned it again; now it is kept and the next run reads it.
  A spelling asked after the one there is still not the record (M2's
  case, a file at 0xE9, is unchanged).
- A test of that reverse case: `check` run twice clones once and keeps
  the clone.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@brettheap

Copy link
Copy Markdown
Contributor Author

(lane openRepoShape-3, 2026-10-22T19:46:27Z) Lane 3's independent review of #1153, delta round (RV1153), head ff33ba1. Verdict: READY. No new MUST or SHOULD.

Lane 3's plan: #1153 is READY. It lands only after #1106, which lands after the v1.2.0 tag on a later word. Then lane 3 brings main into this branch, moves its base to main, and lands it by the stacked-PR merges.

Lane: openreposhape-3 (openRepoShape-3)

This was referenced Oct 22, 2026
@brettheap brettheap added this to the v1.2.1 milestone Oct 23, 2026
This was referenced Oct 23, 2026
Brett Heap and others added 2 commits November 8, 2026 18:18
#1153 (for #1112) was stacked on PR #1106's branch. #1106 has landed on main as the squash 801a7d6, and its final head bd97901 is 504a1dd with main be21751 merged in by update-branch. This is the first of the estate's three stacked-PR merges: it brings that final head in, so the branch holds everything #1106 held at the end. #1153's own change is not touched.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…anch's side of #1106's files

#1106 landed on main as the squash 801a7d6. This branch already holds #1106's change, through its final head bd97901, so where a hunk conflicts in adopt-project.py or tests/test_adopt_one_byte_locale.py (two of #1106's three files; the third, tests/test_adopt_unenterable_source_and_locale.py, merged clean) this merge takes this branch's side (-X ours), which is #1106's change plus #1153's. Every other hunk of main comes in as main has it. This is the second of the estate's three stacked-PR merges; it makes the branch a descendant of main again, so the pull request shows only #1153's own change.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@sonarqubecloud

sonarqubecloud Bot commented Nov 8, 2026

Copy link
Copy Markdown

@brettheap
brettheap marked this pull request as ready for review November 8, 2026 20:46
@sourcery-ai

sourcery-ai Bot commented Nov 8, 2026

Copy link
Copy Markdown

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 6 days by commenting @sourcery-ai review. Upgrade to get a review now.

@brettheap
brettheap merged commit 27397a4 into main Nov 8, 2026
5 checks passed
@brettheap
brettheap deleted the l3-1112-at-the-record-both-spellings branch November 8, 2026 20:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant