diff --git a/park b/park index f8f04a53..034b85f2 100755 --- a/park +++ b/park @@ -599,7 +599,7 @@ submodule_paths() { # working clone BESIDE the holder is where a person works, so it is what is # read - the same reason the holder's own `park` target dispatches there (#80). report_repos() { - local leg member branch + local leg member branch legbranch if [ "$ESTATE_KIND" = "family" ]; then printf 'holder\t%s\t%s\n' "$(tracking_branch_of "$ESTATE_ROOT")" \ "$ESTATE_ROOT" @@ -608,12 +608,16 @@ report_repos() { branch="$(tracking_branch_of "$ESTATE_DIR/$member")" printf 'root\t%s\t%s\n' "$branch" "$ESTATE_DIR/$member" while IFS= read -r leg; do - # A LEG'S branch here is the ROOT's `tracking_branch:`, which - # is NOT what `scripts/bootstrap.py` places a leg on when its - # row declares its own `branch:` (`leg_branch_of`): such a - # leg's unpushed commits are counted against the wrong - # branch (#242). - printf 'leg\t%s\t%s\n' "$branch" "$ESTATE_DIR/$member/$leg" + # A LEG'S branch is the `branch:` its own row declares, else + # the ROOT's `tracking_branch:` — what `scripts/bootstrap.py` + # places it on (#235, #238; #242 here). Its unpushed commits + # are on THAT branch, so that is the one the report counts. + # `|| true`: a `project.yaml` that cannot be read fails + # `leg_branch_of`, and under `set -e` that would end this + # subshell and drop this leg's row and every row after it, + # silently; the root's branch is the answer then. + legbranch="$(leg_branch_of "$ESTATE_DIR/$member" "$leg" || true)" + printf 'leg\t%s\t%s\n' "${legbranch:-$branch}" "$ESTATE_DIR/$member/$leg" done < <(submodule_paths "$ESTATE_DIR/$member") done < <(family_members "$ESTATE_ROOT/family.yaml") return 0 @@ -621,12 +625,16 @@ report_repos() { branch="$(tracking_branch_of "$ESTATE_ROOT")" printf 'root\t%s\t%s\n' "$branch" "$ESTATE_ROOT" while IFS= read -r leg; do - printf 'leg\t%s\t%s\n' "$branch" "$ESTATE_ROOT/$leg" + # The leg's own `branch:`, else the root's, and `|| true` for the + # reason the family loop gives. + legbranch="$(leg_branch_of "$ESTATE_ROOT" "$leg" || true)" + printf 'leg\t%s\t%s\n' "${legbranch:-$branch}" "$ESTATE_ROOT/$leg" done < <(submodule_paths "$ESTATE_ROOT") } # Commits on the TRACKING BRANCH that origin has never seen. The branch is -# the root's own `tracking_branch:` and not a hard-coded `main`: a project +# the root's own `tracking_branch:` — or, for a leg, the `branch:` its own row +# declares (`report_repos`, #242) — and not a hard-coded `main`: a project # that ships from `trunk` would otherwise have been asked a question about a # branch it does not have and answered "nothing", which is worse than not # asking. BOTH refs must exist locally or there is no question to ask — a diff --git a/status b/status index d26b89d8..2df1569d 100755 --- a/status +++ b/status @@ -731,9 +731,9 @@ declared_legs() { # not-carried report's enumeration, with two rows it does not need: `missing`, # for a member with no working clone beside the holder, and `unmounted`, for a # leg declared and not there. Both are things out of sync with the estate as -# recorded, which is what this command is for. A LEG'S BRANCH COLUMN DIFFERS -# from `park`'s: here it is the branch the leg is placed on (`leg_branch_of`), -# where `park`'s report gives every leg the root's (#242). +# recorded, which is what this command is for. A LEG'S BRANCH COLUMN IS THE +# SAME as `park`'s: the `branch:` the leg's own row declares, else the root's +# `tracking_branch:` (`leg_branch_of`; #235, #242). # # `members/` INSIDE A FAMILY HOLDER IS DELIBERATELY NOT HERE, for # park's reason: that copy is pinned and DETACHED, for `bootstrap` and diff --git a/tests/test_park_resume_commands.py b/tests/test_park_resume_commands.py index a1b98997..18c2ac4e 100644 --- a/tests/test_park_resume_commands.py +++ b/tests/test_park_resume_commands.py @@ -1387,6 +1387,158 @@ def test_park_says_when_it_left_nothing_behind(estate, home): assert "0 thing(s) above are not park's to carry" in result.stdout +def clone_leg_member(dest: Path, base: Path, member: str, + **seed) -> dict[str, Path]: + """`seed_leg_member`'s root, cloned into `dest` with its legs mounted. + + Built per test under `base` rather than into the session's `remotes`, so a + leg has a `develop` to sit on only where a test asks for one (`seed` goes + to `seed_leg_member`). The legs arrive DETACHED at their pins, as every + fresh clone's do. + """ + base.mkdir() + bare, _ = seed_leg_member(base, member, **seed) + root = dest / member + git("-c", "protocol.file.allow=always", "clone", "-q", + "--recurse-submodules", str(bare), str(root), cwd=dest) + return {"root": root, "spec": root / "spec", "code": root / "code"} + + +def commits_on(leg: Path, branch: str, count: int) -> None: + """`count` commits on `leg`'s local `branch` that origin has not seen.""" + git("checkout", "-q", "-B", branch, f"origin/{branch}", cwd=leg) + for number in range(count): + (leg / f"{branch}-{number}.md").write_text("never pushed\n", + encoding="utf-8") + commit_all(leg, f"commit {number} on {branch}, not pushed") + + +def family_folder_naming_delta(home: Path) -> Path: + """A family folder whose holder (a probe Makefile) names ONE member, + `Delta`, and whose working clone `clone_leg_member` puts beside it.""" + folder = home / "projects" / FAMILY + holder = folder / FAMILY + holder.mkdir(parents=True) + (holder / "family.yaml").write_text( + FAMILY_YAML.format( + id=FAMILY.lower(), name=FAMILY, org=ORG, + members=(f"\n - project: Delta\n id: delta\n" + f" repository: {ORG}/Delta\n" + f" path: members/Delta\n")), encoding="utf-8") + (holder / "Makefile").write_text(PROBE_MAKEFILE, encoding="utf-8") + return folder + + +def test_park_reports_a_leg_on_the_branch_its_own_row_declares(home, tmp_path): + """#242. `scripts/bootstrap.py` places a leg on `str(leg.get("branch") or + branch)`, so a leg whose row in `project.yaml` declares `branch: develop` + keeps its work on `develop`. The report asked about the ROOT's + `tracking_branch` for every leg, found nothing ahead on `main`, and ended + without naming the leg: work `park` did not carry, missed silently.""" + root = clone_leg_member(home / "projects", tmp_path / "remotes", + "Delta", leg_branch="develop") + commits_on(root["spec"], "develop", 3) + + result = run(PARK, "Delta", home=home) + assert result.returncode == 0, result.stderr + result.stdout + assert f"unpushed develop leg {root['spec']}" in result.stdout, ( + "a leg's unpushed commits on its own `branch:` were not reported:\n" + + result.stdout) + assert "3 commit(s) on develop that origin has not seen" in result.stdout + assert "A leg goes up as a pull request too" in result.stdout + assert f"unpushed main leg {root['spec']}" not in result.stdout, ( + "the leg was asked about the root's branch") + + +def test_park_reports_a_family_members_leg_on_its_own_declared_branch( + home, tmp_path): + """#242, in a family: the holder's report reads the member's working clone + beside it, and that member's legs, which are built by a second loop. Its + leg rows carried the member's `tracking_branch` too.""" + folder = family_folder_naming_delta(home) + member = clone_leg_member(folder, tmp_path / "remotes", "Delta", + leg_branch="develop", code_leg=True) + commits_on(member["spec"], "develop", 2) + commits_on(member["code"], "main", 1) + + result = run(PARK, FAMILY, home=home) + assert result.returncode == 0, result.stderr + result.stdout + assert f"unpushed develop leg {member['spec']}" in result.stdout, ( + "a member's leg on its own `branch:` was not reported:\n" + + result.stdout) + assert "2 commit(s) on develop that origin has not seen" in result.stdout + assert f"unpushed main leg {member['spec']}" not in result.stdout + # The leg that declares none is the member's `tracking_branch`'s: the + # family loop's fallback, which nothing else here reaches. + assert f"unpushed main leg {member['code']}" in result.stdout, ( + result.stdout) + assert "1 commit(s) on main that origin has not seen" in result.stdout + + +@pytest.mark.skipif(hasattr(os, "geteuid") and os.geteuid() == 0, + reason="mode 000 does not bind root, so the manifest " + "stays readable and there is nothing to survive") +def test_park_reports_a_leg_when_its_members_manifest_cannot_be_read( + home, tmp_path): + """#242, the review of #249: `leg_branch_of` fails when `project.yaml` + exists and cannot be read, and `report_repos` runs under `set -e`, so an + unguarded `legbranch="$(leg_branch_of ...)"` ended the report's subshell: + that leg's row and every row after it were dropped, `make park`'s exit + code was untouched and `park` said nothing. The leg is reported against + the member's own branch instead (main's `park` did, because it never read + the file for a leg).""" + folder = family_folder_naming_delta(home) + member = clone_leg_member(folder, tmp_path / "remotes", "Delta") + commits_on(member["spec"], "main", 2) + manifest = member["root"] / "project.yaml" + manifest.chmod(0o000) + try: + result = run(PARK, FAMILY, home=home) + finally: + manifest.chmod(0o644) + assert result.returncode == 0, result.stderr + result.stdout + assert f"unpushed main leg {member['spec']}" in result.stdout, ( + "an unreadable manifest dropped the leg's row from the report:\n" + + result.stdout) + assert "2 commit(s) on main that origin has not seen" in result.stdout + + +def test_park_still_reports_a_leg_with_no_branch_of_its_own_on_the_roots( + home, tmp_path): + """UNCHANGED for a row with no `branch:`: bootstrap places that leg on the + root's `tracking_branch`, so that is the branch its unpushed commits are + counted on. Green before the fix of #242 and after it.""" + root = clone_leg_member(home / "projects", tmp_path / "remotes", "Echo") + commits_on(root["spec"], "main", 1) + + result = run(PARK, "Echo", home=home) + assert result.returncode == 0, result.stderr + result.stdout + assert f"unpushed main leg {root['spec']}" in result.stdout + assert "1 commit(s) on main that origin has not seen" in result.stdout + + +def test_park_asks_each_leg_of_one_root_about_its_own_branch(home, tmp_path): + """One root, two legs: `spec` declares `branch: develop` and `code` + declares none. Each is read against the branch bootstrap puts it on, so + the report names `spec` on `develop` and `code` on `main`, with their own + counts and neither leg's branch lent to the other.""" + root = clone_leg_member(home / "projects", tmp_path / "remotes", "Foxtrot", + leg_branch="develop", code_leg=True) + commits_on(root["spec"], "develop", 3) + commits_on(root["code"], "main", 1) + + result = run(PARK, "Foxtrot", home=home) + assert result.returncode == 0, result.stderr + result.stdout + assert f"unpushed develop leg {root['spec']}" in result.stdout, ( + result.stdout) + assert f"unpushed main leg {root['code']}" in result.stdout, ( + result.stdout) + assert "3 commit(s) on develop that origin has not seen" in result.stdout + assert "1 commit(s) on main that origin has not seen" in result.stdout + assert f"unpushed main leg {root['spec']}" not in result.stdout + assert f"unpushed develop leg {root['code']}" not in result.stdout + + def test_park_never_reads_the_pinned_copy(estate, home): """`members/` is pinned and DETACHED; nobody's work is there, and a report that named it would be reporting on the wrong copy."""