Skip to content

adopt-project.py: plan writes its follow-ups once, so after a hand edit of the plan check and the split commit name a GNUmakefile the root no longer keeps, or say nothing of one it now keeps #1206

Open

Description

@brettheap

(lane openRepoShape-5, split out of #1132 as its item 5; RV1105's NIT 8 on #1105)

Problem

plan works out the follow-ups once and stores them in the plan's follow_ups: list: the CONTRACTS_DIR line (_makefile_follow_up, #977), the collision lines (_predict_collisions, whose one caller is plan), and the rest of follow_ups_for. check prints the stored items, and execute carries them into the split commit's message, which is also the pull request's body.

The plan is meant to be edited by hand:

  • contracts/path-classification.yaml (root-project-makefile) says that moving a Makefile to the code leg "is a one-line edit of the plan";
  • every review_required question is answered by editing leg:.

After either edit, the stored follow-ups still describe the plan as plan wrote it, not as it stands. execute's NEXT does not have this problem, because it reads the plan's current root paths (paths_for["root"]).

Measured on main 961640c68

1. A hand move of a kept GNUmakefile to the code leg. This used an untracked probe in the lane's scratch folder: test_zz_probe_n1132_stale.py, built on tests/test_adopt_contracts_dir.py's TREE and EDITS and run through the lane's runner. There were two sources: one keeps GNUmakefile, and one keeps Makefile + GNUmakefile. For each:

  • run plan;
  • replace - path: GNUmakefile\n leg: root\n with leg: code and a resolution:;
  • run check, then execute --local-remote-dir.
step result, for both sources
check exit 0, and it still prints follow-up: ... GNU make at the root reads the source's GNUmakefile, the first of ...
execute exit 0
the split commit's message carries the same item
git ls-tree --name-only of the split's root has no GNUmakefile

2. A question answered root. This is row answered-root of tests/test_adopt_make_bootstrap_followups.py, on #1132's branch. The source holds GNUmakefile/rules.mk.

  • plan asks about GNUmakefile/ (rule default, leg: null). It writes the root Makefile wires CONTRACTS_DIRto thecontracts/ of the spec leg, ..., which says nothing of GNUmakefile/, because the plan sent nothing GNU make reads to the root.
  • The question is answered leg: root.
  • check prints that same line, and the split commit's message carries it.
  • NEXT, which reads the plan as it stands, says that GNU make stops at GNUmakefile/.
  • In the clone, make -n bootstrap exits 2 with make: *** GNUmakefile: Is a directory. Stop..

So the plan's CONTRACTS_DIR line says the root's Makefile wires the variable, at a root where make reads no Makefile at all.

The collision lines come from the same stored list and would go stale the same way, for example after a hand move of a kept Makefile. That case was not measured.

Recommended fix

Have check and execute work out the follow-ups from the plan as it stands, and stop storing them, as #600 did for the read-across lines. Since #600, cross_leg_references takes no mount path, and contracts_from_code_leg prints the path from the code leg to the spec leg beside the CURRENT mounts. Work out the CONTRACTS_DIR line and the collision lines the same way, from the current entries:

  • follow_ups_for over the plan's entries;
  • _predict_collisions over its root paths, with attributes_link read as execute reads it.

A plan written before the change still holds a follow_ups: list. Read it as nothing, or refuse it by name; never print it beside the recomputed list.

The second-best fix is for check to compare the stored list with a recomputed one, and to report each item that differs. That still leaves two lists to keep in agreement.

Related cases found while porting #1132 (main, or #1132's branch, as marked)

  1. A kept makefile/ DIRECTORY gets "until the two are merged". _makefile_follow_up asks _root_keeps(surviving, "Makefile", fold=True), which takes makefile/ for Makefile on a disk that ignores case. So, rendered on adopt-project.py and the assembly-root template: follow-ups from RV1105's and RV1121's reviews of #1105 and #1121 (make bootstrap in the template, a kept GNUmakefile/ directory, python3 on Windows, GNU_MAKE_READS twice, stale follow-ups) #1132's branch for a root that keeps only makefile/, the line says the shape's Makefile is written "as shape/Makefile once case is ignored, as a macOS or Windows disk does, where the source's makefile is that path, until the two are merged". A directory cannot be merged with a Makefile. (adopt-project.py and the assembly-root template: follow-ups from RV1105's and RV1121's reviews of #1105 and #1121 (make bootstrap in the template, a kept GNUmakefile/ directory, python3 on Windows, GNU_MAKE_READS twice, stale follow-ups) #1132 adds the clause that says GNU make stops at makefile/, and leaves this part as main has it.)

  2. A directory kept only through deeper entries is not seen. _root_keeps(surviving, name) asks only for name and for name/. Measured on adopt-project.py and the assembly-root template: follow-ups from RV1105's and RV1121's reviews of #1105 and #1121 (make bootstrap in the template, a kept GNUmakefile/ directory, python3 on Windows, GNU_MAKE_READS twice, stale follow-ups) #1132's branch:

    • _root_keeps({"GNUmakefile/rules.mk"}, "GNUmakefile") is False;
    • _make_reads_first of that root is None;
    • the CONTRACTS_DIR line is then "the root Makefile wires ...".

    But GNU make finds the directory GNUmakefile/ all the same and stops there. plan folds such a folder into one GNUmakefile/ entry, so this needs a hand-split entry or a policy that writes file entries.

  3. A kept Makefile/ DIRECTORY is treated as a Makefile of the source's own. _root_keeps({"Makefile/"}, "Makefile") is True. So the shape's Makefile goes beside it as shape/Makefile, and the line says "the source keeps a Makefile of its own at the root, so that block waits in shape/Makefile until the two are merged". GNU make 4.3 at such a root exits 2 with make: *** Makefile: Is a directory. Stop.. Measured in a scratch folder that held only a Makefile/ directory.

Verify

  • A row per edit: move a kept GNUmakefile to code, and answer a GNUmakefile/ question root. After each edit, check's follow-ups and the split commit's message must match a fresh plan of a source whose rules give the edited answer directly. That source is tests/test_adopt_make_bootstrap_followups.py's a-rule-keeps-it row for the directory.
  • A row for a stored follow_ups: list in an old plan.
  • Rows for the three related cases, or a named ruling that leaves them as they are.

Lane: openreposhape-5 (openRepoShape-5)

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