From e2110f8f5697f6601be781fa332e1ce4a0f8474b Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Fri, 16 Oct 2026 12:12:32 +0000 Subject: [PATCH 1/8] tests/conftest.py: the session turns git's automatic maintenance off, in command scope and in a global config that includes the person's own, so a `copytree` of a fixture no longer meets a detached repack deleting its loose objects (#645) `shutil.copytree` walks a repository's `objects/` one file at a time, and the suite copies a repository a fixture has just committed in, fetched into or pushed to in 139 places: `project`, `test_family.py`'s `holder`, ten copies of bare remotes and two of a `.git` directory. Every `git commit`, `merge` and `fetch`, and the `receive-pack` a push starts in the remote, runs `git maintenance run --auto` as it finishes. Since git 2.47 that run detaches and holds `objects/maintenance.lock`; since git 2.54 its default `geometric` strategy repacks a small repository and deletes the loose objects it packed. A copy taken in that moment dies with `[Errno 2]`, as `holder` did on the Linux runner. A session autouse fixture, `no_auto_maintenance`, sets `gc.auto=0` and `maintenance.auto=false` in two scopes: - command scope (`GIT_CONFIG_COUNT`), appended after the entries the environment already holds: every git the suite or a tool under test starts inherits it, a `git submodule`'s children too, and it outranks every config file; - a global config file (`GIT_CONFIG_GLOBAL`) that includes the global config git would read without it and sets the two keys after it, because git starts the `receive-pack` of a push to a local path or a `file://` URL with `GIT_CONFIG_COUNT` and `GIT_CONFIG_PARAMETERS` removed. `tests/test_no_auto_maintenance.py` asks git what it starts and reads, through trace2 events and a `post-receive` hook: a commit, a fetch and a merge start no maintenance, with the session's global file and with a test's own; the scaffold and the remotes it pushes to start none; a remote's `receive-pack` reads both settings off; the entries are appended; and a person's own global config is still read beneath them. Lane: openreposhape-5 (openRepoShape-5) Co-Authored-By: Claude Opus 5.5 --- tests/conftest.py | 108 +++++++++++++++- tests/test_no_auto_maintenance.py | 204 ++++++++++++++++++++++++++++++ 2 files changed, 311 insertions(+), 1 deletion(-) create mode 100644 tests/test_no_auto_maintenance.py diff --git a/tests/conftest.py b/tests/conftest.py index 4bc356f8..d357227c 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -82,7 +82,10 @@ def clone_just_pushed(remote: Path, dest: Path, loose objects and deletes them mid-walk, and the clone dies `failed to copy file to '/.git/objects/..': No such file or directory` (the macOS runner, twice in 90 minutes on 2026-10-12). Upload-pack reads the - object store, which a repack does not break. + object store, which a repack does not break. The session now keeps that + maintenance from starting (`no_auto_maintenance` below, #645), and the + clone still reads through git: a test that gives git a global config of + its own takes the session's away from the pushes it makes. `--no-local` does that for the root. The two leg clones `--recurse-submodules` makes are git's own, from the plain path @@ -135,6 +138,109 @@ def cloned_just_pushed(remote: Path, dest: Path, branch: str | None = None, return Path(dest) +# --- git's automatic maintenance, OFF for every git the session starts (#645) +# +# A COPY IS A WALK OF THE FILES TOO. `shutil.copytree` reads `objects/` one +# file at a time, as git's local clone does (`clone_just_pushed` above), and +# this suite copies a repository a fixture has just committed in, fetched +# into or pushed to in over a hundred places: `project`, `test_family.py`'s +# `holder`, a bare remote. +# `git commit`, `merge` and `fetch`, and the `receive-pack` a push starts in +# the remote, each run `git maintenance run --auto` as they finish. Since git +# 2.47 that run DETACHES, holding `objects/maintenance.lock`, and the command +# returns while it goes on; since 2.54 its default strategy (`geometric`) +# repacks a repository of a few hundred loose objects and deletes the ones +# it packed. A copy taken in that moment fails `[Errno 2] No such file or +# directory` on one of them: `holder`, on the Linux runner (#645). +# +# So the session turns it off, as every git command in this estate is told +# to: `gc.auto=0`, for a git that runs `git gc --auto` itself (2.43's +# `receive-pack` does), and `maintenance.auto=false`. In TWO scopes, because +# neither reaches every git alone: +# +# - COMMAND SCOPE (`GIT_CONFIG_COUNT`), APPENDED to what the environment +# holds (`appended_to_command_scope`; the Windows job holds two): every git +# this process and every tool under test starts inherits it, and so does +# each git a `git submodule` starts. It outranks every config file, a +# repository's own and a test's own global file included. +# - A GLOBAL FILE (`GIT_CONFIG_GLOBAL`), for the one git that never sees +# command scope: a push to a local path or a `file://` URL starts +# `receive-pack` in the remote with `GIT_CONFIG_COUNT` and +# `GIT_CONFIG_PARAMETERS` removed, and that process runs the remote's +# maintenance. The file INCLUDES the global config git would read without +# it -- the file `GIT_CONFIG_GLOBAL` already named, or +# `$XDG_CONFIG_HOME/git/config` then `~/.gitconfig` -- so a person's own +# settings still apply, and sets the two keys after them. +# +# WHAT IT LEAVES: a git whose environment takes away both -- a +# `GIT_CONFIG_COUNT` overwritten rather than appended to, together with a +# `GIT_CONFIG_GLOBAL` of a test's own, or an environment built from nothing +# -- and a push made under a global file of a test's own. No repository the +# suite copies is made, or pushed to, that way today. +# `tests/test_no_auto_maintenance.py` holds both scopes to account. + +#: The two settings, in the order they are appended and written. +NO_AUTO_MAINTENANCE = (("gc.auto", "0"), ("maintenance.auto", "false")) + +#: The session's global config file, directly under the basetemp. +GLOBAL_CONFIG_NAME = "gitconfig" + + +def _config_quoted(path: str) -> str: + """`path` as a double-quoted git config value, in which a backslash and a + double quote are escapes.""" + return '"' + path.replace("\\", "\\\\").replace('"', '\\"') + '"' + + +def appended_to_command_scope(environ: Mapping[str, str], + entries=NO_AUTO_MAINTENANCE) -> dict: + """The `GIT_CONFIG_COUNT`, `GIT_CONFIG_KEY_` and `GIT_CONFIG_VALUE_` + that put `entries` in git's command scope AFTER the ones `environ` holds, + which stay as they are: a later entry of a key is the one git reads.""" + held = int(environ.get("GIT_CONFIG_COUNT") or 0) + added = {"GIT_CONFIG_COUNT": str(held + len(entries))} + for index, (key, value) in enumerate(entries, held): + added[f"GIT_CONFIG_KEY_{index}"] = key + added[f"GIT_CONFIG_VALUE_{index}"] = value + return added + + +def session_global_config(named: str | None) -> bytes: + """The global config file `no_auto_maintenance` writes: an include of + what git reads as its global config without one -- `named`, the file + `GIT_CONFIG_GLOBAL` held, or else git's two default files in git's own + order -- and then `NO_AUTO_MAINTENANCE`. LF, and every path POSIX-style: + `~/` is expanded by git, from the `HOME` of the git that reads it.""" + if named: + included = [Path(os.path.abspath(named)).as_posix()] + else: + xdg = os.environ.get("XDG_CONFIG_HOME") + included = [(Path(xdg) / "git" / "config").as_posix() if xdg + else "~/.config/git/config", "~/.gitconfig"] + lines = ["[include]"] + lines += [f"\tpath = {_config_quoted(path)}" for path in included] + for key, value in NO_AUTO_MAINTENANCE: + section, name = key.split(".") + lines += [f"[{section}]", f"\t{name} = {value}"] + return ("\n".join(lines) + "\n").encode("utf-8") + + +@pytest.fixture(scope="session", autouse=True) +def no_auto_maintenance(tmp_path_factory): + """Git's automatic maintenance off in `os.environ` for the whole session, + in both scopes the block above describes; returns the global file. Autouse + and session scope, so it is in place before any session or module fixture + builds a repository, and put back as it was at the end.""" + config = tmp_path_factory.getbasetemp() / GLOBAL_CONFIG_NAME + config.write_bytes(session_global_config( + os.environ.get("GIT_CONFIG_GLOBAL"))) + with pytest.MonkeyPatch.context() as patch: + for name, value in {**appended_to_command_scope(os.environ), + "GIT_CONFIG_GLOBAL": str(config)}.items(): + patch.setenv(name, value) + yield config + + def load_without_bytecode(path: Path, name: str): """`path` run as a module called `name`, and no bytecode written for it (#544; it is `load()` of `test_bootstrap_spelling.py`, #456, moved here). diff --git a/tests/test_no_auto_maintenance.py b/tests/test_no_auto_maintenance.py new file mode 100644 index 00000000..33d55c11 --- /dev/null +++ b/tests/test_no_auto_maintenance.py @@ -0,0 +1,204 @@ +# SPDX-License-Identifier: Apache-2.0 +"""Git's automatic maintenance is off for every git the suite starts (#645). + +The suite copies a repository file by file (`shutil.copytree`) a moment +after a fixture committed in it, fetched into it or pushed to it, and the +maintenance git starts as each of those finishes can be packing that +repository in the background, deleting the loose objects the copy is +reading. `tests/conftest.py`'s `no_auto_maintenance` turns `gc.auto` and +`maintenance.auto` off for the session, in git's command scope and in a +global config file. Each test here asks git what it starts or reads, so +none of them waits for a race and each holds on any git: + +- a commit, a fetch and a merge run as a fixture runs them start no `git + maintenance` and no `git gc`, with the session's global file and with a + test's own (git's trace2 events name every child a git starts); +- `scaffold-project.py` into local bare remotes, a tool that starts its own + gits, starts no `git maintenance` anywhere, the `receive-pack` its pushes + start in the remotes included; +- the `receive-pack` a push into a local remote starts, which never sees + command scope, reads both settings off: a hook in the remote reports them; +- the two are appended to the command scope the environment holds, and a + person's own global config is still read, in git's order, beneath them. +""" + +from __future__ import annotations + +import json +import os +import subprocess +from pathlib import Path + +import pytest + +from conftest import (NO_AUTO_MAINTENANCE, ORG, PROJECT, SCAFFOLD, + appended_to_command_scope, git, run_script, + session_global_config) + +#: The verbs auto maintenance runs as: `git maintenance run --auto`, and +#: `git gc --auto`, which a git started itself before `maintenance` existed. +MAINTENANCE_VERBS = ("maintenance", "gc") + +#: The two settings as a `receive-pack` must read them, one `key=value` a +#: line: spelled out, not read from `NO_AUTO_MAINTENANCE`, so a change to +#: that table is a change to this test too. +BOTH_OFF = b"gc.auto=0\nmaintenance.auto=false\n" + + +def traced(monkeypatch, tmp_path: Path) -> Path: + """The file git's trace2 events go to for the rest of the test, from + every git the test starts and every git those start: one JSON object a + line.""" + trace = tmp_path / "trace2.json" + monkeypatch.setenv("GIT_TRACE2_EVENT", str(trace)) + return trace + + +def events(trace: Path) -> list: + return [json.loads(line) + for line in trace.read_bytes().decode("utf-8").splitlines() + if line.strip()] + + +def ran(trace: Path) -> set: + """The verb of every git that wrote to `trace`.""" + return {event["name"] for event in events(trace) + if event.get("event") == "cmd_name"} + + +def started(trace: Path, verbs=MAINTENANCE_VERBS) -> list: + """The argv of every git child a git in `trace` started as one of + `verbs`: `["git", "maintenance", "run", "--auto", ...]`.""" + found = [] + for event in events(trace): + argv = event.get("argv") or [] + if event.get("event") == "child_start" and argv[1:2] and \ + argv[1] in verbs: + found.append(argv) + return found + + +def asked(*args: str, cwd: Path) -> str: + """`git config `'s answer, outside any repository.""" + return git("config", *args, cwd=cwd, check=False).stdout.strip() + + +@pytest.fixture +def behind(tmp_path) -> Path: + """A clone of a local bare remote that has moved on by a commit, as a + fixture leaves one: `fetch` brings something and `merge` makes a + merge commit.""" + remote, work, other = (tmp_path / "remote.git", tmp_path / "work", + tmp_path / "other") + git("init", "-q", "--bare", "-b", "main", str(remote), cwd=tmp_path) + git("init", "-q", "-b", "main", str(other), cwd=tmp_path) + git("commit", "-q", "--allow-empty", "-m", "one", cwd=other) + git("push", "-q", str(remote), "main", cwd=other) + git("clone", "-q", "--no-local", str(remote), str(work), cwd=tmp_path) + git("commit", "-q", "--allow-empty", "-m", "two", cwd=other) + git("push", "-q", str(remote), "main", cwd=other) + return work + + +@pytest.mark.parametrize("own_global", [False, True], + ids=["the-session-global", "a-tests-own-global"]) +def test_a_commit_a_fetch_and_a_merge_start_no_maintenance( + behind, tmp_path, monkeypatch, own_global): + """On the base each of the three started `git maintenance run --auto` + (git 2.43). With a global config of the test's own, the command scope + alone keeps it off.""" + if own_global: + empty = tmp_path / "own.gitconfig" + empty.write_bytes(b"") + monkeypatch.setenv("GIT_CONFIG_GLOBAL", str(empty)) + trace = traced(monkeypatch, tmp_path) + git("commit", "-q", "--allow-empty", "-m", "three", cwd=behind) + git("fetch", "-q", "origin", cwd=behind) + git("merge", "-q", "--no-ff", "-m", "merge", "origin/main", cwd=behind) + assert {"commit", "fetch", "merge"} <= ran(trace), ran(trace) + assert started(trace) == [] + + +def test_a_tool_and_the_remotes_it_pushes_to_start_no_maintenance( + tmp_path, monkeypatch): + """The scaffold commits in three working trees and pushes each into a + bare remote, where `receive-pack` runs that remote's maintenance. On the + base its commits started `git maintenance run --auto`. A `git gc --auto` + child is not counted: 2.43's `receive-pack` starts one whatever the + settings say, and `gc.auto=0`, which the next test shows it reads, makes + it do nothing.""" + trace = traced(monkeypatch, tmp_path) + result = run_script( + SCAFFOLD, "--org", ORG, "--project", PROJECT, + "--elected-by", "Test Human", "--elected-on", "2026-09-02", + "--local-remote-dir", str(tmp_path / "remotes"), + "--work-dir", str(tmp_path / "work")) + assert result.returncode == 0, result.stderr + result.stdout + assert {"commit", "receive-pack"} <= ran(trace), ran(trace) + assert started(trace, ("maintenance",)) == [] + + +def test_the_receive_pack_of_a_push_to_a_local_remote_reads_both_off( + tmp_path): + """Git starts `receive-pack` for a push to a path with no command scope, + so only the session's global file reaches it. Its `post-receive` hook + runs under it and writes down what git reads there. On the base it read + neither.""" + remote, work = tmp_path / "remote.git", tmp_path / "work" + git("init", "-q", "--bare", "-b", "main", str(remote), cwd=tmp_path) + hook = remote / "hooks" / "post-receive" + hook.write_bytes( + b"#!/bin/sh\nfor key in gc.auto maintenance.auto; do\n" + b' printf "%s=%s\\n" "$key" "$(git config --get "$key")"\n' + b"done > read-by-receive-pack\n") + hook.chmod(0o755) + git("init", "-q", "-b", "main", str(work), cwd=tmp_path) + git("commit", "-q", "--allow-empty", "-m", "one", cwd=work) + git("push", "-q", str(remote), "main", cwd=work) + assert (remote / "read-by-receive-pack").read_bytes() == BOTH_OFF + + +def test_the_two_are_appended_to_the_command_scope_the_environment_holds( + tmp_path): + """The Windows job holds two entries in `GIT_CONFIG_COUNT` for the whole + run, and a count that started again at 0 would drop both.""" + held = {"GIT_CONFIG_COUNT": "2", + "GIT_CONFIG_KEY_0": "test.heldone", "GIT_CONFIG_VALUE_0": "one", + "GIT_CONFIG_KEY_1": "test.heldtwo", "GIT_CONFIG_VALUE_1": "two"} + added = appended_to_command_scope(held) + assert added["GIT_CONFIG_COUNT"] == "4", added + env = {**os.environ, **held, **added} + lines = [subprocess.run(["git", "config", "--get", key], cwd=str(tmp_path), + capture_output=True, text=True, env=env, + check=False).stdout + for key in ("test.heldone", "test.heldtwo", + *(key for key, _ in NO_AUTO_MAINTENANCE))] + assert lines == ["one\n", "two\n", + *(f"{value}\n" for _, value in NO_AUTO_MAINTENANCE)] + + +@pytest.mark.parametrize("included", ["~/.gitconfig", "~/.config/git/config"]) +def test_a_persons_own_global_config_is_still_read_beneath_the_two( + included, no_auto_maintenance, tmp_path, monkeypatch): + """The session's file includes the global config git would read without + it, `~/` expanded from the `HOME` of the git that reads it, and sets the + two settings after it: a person's own values are read, and their own + `maintenance.auto = true` does not turn maintenance back on. A case is + skipped where the file includes another path in its place, which a test + cannot write: the session began with `GIT_CONFIG_GLOBAL` set, or, for + the second, with `XDG_CONFIG_HOME` set.""" + if no_auto_maintenance.read_bytes() != session_global_config(None): + pytest.skip("the session began with GIT_CONFIG_GLOBAL set, and its " + "file includes that one") + if included.startswith("~/.config/") and os.environ.get("XDG_CONFIG_HOME"): + pytest.skip("XDG_CONFIG_HOME is set, and the file includes the git " + "config under it") + monkeypatch.setenv("HOME", str(tmp_path)) + own = tmp_path.joinpath(*included[2:].split("/")) + own.parent.mkdir(parents=True, exist_ok=True) + own.write_bytes(b"[openreposhape]\n\tprobe = " + included.encode() + + b"\n[maintenance]\n\tauto = true\n[gc]\n\tauto = 6700\n") + assert asked("--get", "openreposhape.probe", cwd=tmp_path) == included + for key, value in NO_AUTO_MAINTENANCE: + assert asked("--global", "--includes", "--get", key, + cwd=tmp_path) == value, key From accc7d2afe0610db22ae4886c43adc7c6e09d495 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Fri, 16 Oct 2026 12:33:34 +0000 Subject: [PATCH 2/8] tests/conftest.py: the note on the session's maintenance settings names `test_doctor_pin_restore_merges.py`'s `the_standard_moved` beside `holder`, and says a `git config --global` read without `--includes` sees the two alone (#645) Lane 2 met the same race on #623's Linux run (run 38895011713): a setup error in `test_copies_neither_side_edited_are_taken_whole_with_the_pin`, `shutil.Error ... standard5/openRepoShape/.git/objects/1e ... [Errno 2]`. `the_standard_moved` (`tests/test_doctor_pin_restore_merges.py:99`) copies the session's `standard`, which `test_shape_doctor.py` commits in, and at :102 copies `moved` just after `moved_on` commits in it. Both commits run under `{**os.environ, ...}`, so the session's command scope already turns their maintenance off; the note now names the site. `git config --global ` reads the global file without following its includes unless `--includes` is given, so under the session's file it sees `gc.auto` and `maintenance.auto` and nothing of `~/.gitconfig`. The one such read in this repository is `setup-project.py`'s Windows-only `core.autocrlf` warning, and no test reads that from a person's own config. A read with no scope, the kind git itself makes, follows the include. Comment only. Lane: openreposhape-5 (openRepoShape-5) Co-Authored-By: Claude Opus 5.5 --- tests/conftest.py | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index d357227c..5960f06b 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -144,14 +144,16 @@ def cloned_just_pushed(remote: Path, dest: Path, branch: str | None = None, # file at a time, as git's local clone does (`clone_just_pushed` above), and # this suite copies a repository a fixture has just committed in, fetched # into or pushed to in over a hundred places: `project`, `test_family.py`'s -# `holder`, a bare remote. +# `holder`, `test_doctor_pin_restore_merges.py`'s copies of the session's +# `standard` (`the_standard_moved`), a bare remote. # `git commit`, `merge` and `fetch`, and the `receive-pack` a push starts in # the remote, each run `git maintenance run --auto` as they finish. Since git # 2.47 that run DETACHES, holding `objects/maintenance.lock`, and the command # returns while it goes on; since 2.54 its default strategy (`geometric`) # repacks a repository of a few hundred loose objects and deletes the ones # it packed. A copy taken in that moment fails `[Errno 2] No such file or -# directory` on one of them: `holder`, on the Linux runner (#645). +# directory` on one of them: `holder` and `the_standard_moved`, on the +# Linux runner (#645). # # So the session turns it off, as every git command in this estate is told # to: `gc.auto=0`, for a git that runs `git gc --auto` itself (2.43's @@ -170,7 +172,10 @@ def cloned_just_pushed(remote: Path, dest: Path, branch: str | None = None, # maintenance. The file INCLUDES the global config git would read without # it -- the file `GIT_CONFIG_GLOBAL` already named, or # `$XDG_CONFIG_HOME/git/config` then `~/.gitconfig` -- so a person's own -# settings still apply, and sets the two keys after them. +# settings still apply, and sets the two keys after them. A `git config +# --global ` read without `--includes` sees the two alone: in this +# repository that is `setup-project.py`'s Windows-only `core.autocrlf` +# warning, which no test reads from a person's own config. # # WHAT IT LEAVES: a git whose environment takes away both -- a # `GIT_CONFIG_COUNT` overwritten rather than appended to, together with a From 4149cbb5fce2ccef18a88896f0bccb093a058665 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Fri, 16 Oct 2026 17:22:19 +0000 Subject: [PATCH 3/8] tests/conftest.py and four test files: one helper builds the `url..insteadOf` rewrite that `clone_just_pushed`, `read_through_git` and the adopt tests' `pasted` each built, and the `--init` of a mount git keeps no repository for goes through it, so a submodule's clone no longer walks a just-pushed remote's files (#662) `insteadof_prefixes(remotes, typed=False)` is the one list of prefixes: the directory resolved, in both of a Windows path's spellings, each with a separator after it, and the directory as typed first when asked. `insteadof_entries` pairs each with the one `url..insteadOf` key, and `insteadof_args` is the same as the `-c` arguments of one git command. `clone_just_pushed` passes the arguments, `read_through_git` appends the entries to the `GIT_CONFIG_*` the environment already holds (as `appended_to_command_scope` does for the session's own, so a pasted line keeps `no_auto_maintenance` in command scope; it overwrote the count before), and `pasted` in `test_adopt_e2e.py`, a third copy of the rule, takes the arguments too. `submodule_update_through_git(root, remotes, *options)` is the wrapper beside `clone_just_pushed` for the `--init` of an empty mount. A trace of each `submodule update` site (GIT_TRACE, before and after) shows which ones clone: `test_family_bump_remove_rollback.py` at 337 and 908 (both legs of a member), `test_doctor_legs_members_next.py` at 631 (the `code` leg, its store removed by `unbootstrap`), 987 (the member and its two legs) and 1054 (the member's two legs, through `with_its_own_legs`) ran `git clone` of a plain path and no `pack-objects`; they now run the transport (`pack-objects` and `index-pack` once per clone). `test_doctor_gitlink_repairs.py:343` and `test_doctor_unfinished_merge.py:145,171` clone and fetch nothing, so they are unchanged, and so is the deliberate failure probe at `test_doctor_legs_members_next.py:607`. `tests/test_clone_just_pushed.py` holds the helper to its list for a typed and a resolved directory, given with and without a trailing separator, asks git (`ls-remote --get-url`) what the `-c` form and the environment form do to a plain path and to a neighbour that starts with the same text, shows the environment form keeps the entries the environment holds, and reads an `--init` through the wrapper as one pack against a plain `--init` that leaves the remote's loose objects loose. The change is test only. Lane: openreposhape-5 (openRepoShape-5) Co-Authored-By: Claude Sonnet 5.5 --- tests/conftest.py | 116 ++++++++++++---- tests/test_adopt_e2e.py | 24 ++-- tests/test_clone_just_pushed.py | 153 +++++++++++++++++++++- tests/test_doctor_legs_members_next.py | 38 +++--- tests/test_family_bump_remove_rollback.py | 10 +- 5 files changed, 280 insertions(+), 61 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index 5960f06b..81446dc0 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -68,6 +68,60 @@ FILE_PROTOCOL = ["-c", "protocol.file.allow=always"] +# --- one rewrite, for every read of a just-pushed local remote (#440, #662) -- +# +# A clone of a plain path is git's local clone, which walks the source's +# `objects/` one file at a time; a clone of a `file://` URL asks upload-pack, +# which reads the object store (`clone_just_pushed` says why that matters). +# What turns the first into the second for the legs a submodule clones from +# the plain paths `.gitmodules` records is a `url..insteadOf` +# rewrite, and it is written HERE, once, in three forms: the prefixes, the +# `(key, prefix)` entries, and the `-c` arguments of one git command. +# `read_through_git` below puts the entries in the environment instead. + +def insteadof_prefixes(remotes: Path | str, *, + typed: bool = False) -> list[str]: + """The prefixes a `url..insteadOf` rewrite names so that git + reads the bare remotes under `remotes` through the transport and not + through their files: the directory with a separator after it, so a + rewrite of `/` does not reach a neighbour called + `-other`, and a `remotes` given with or without a trailing + separator gives the same list. + + RESOLVED, always: `scaffold-project.py` writes `.gitmodules`'s leg URLs + from the directory resolved, so that is where a submodule's clone starts, + spelled with the system's separator and with a slash, which are two + prefixes on Windows and one elsewhere. TYPED, on request: the directory + as the caller spelled it, made absolute and a symlink in it kept, which is + how a line a tool prints names a remote; it comes first. None is listed + twice.""" + resolved = Path(remotes).resolve() + directories = [Path(os.path.abspath(remotes)), resolved] if typed \ + else [resolved] + return list(dict.fromkeys( + spelling for directory in directories + for spelling in (os.path.join(str(directory), ""), + directory.as_posix().rstrip("/") + "/"))) + + +def insteadof_entries(remotes: Path | str, *, + typed: bool = False) -> list[tuple[str, str]]: + """`(key, prefix)` for each of `insteadof_prefixes`: the key is the one + `url..insteadOf` that sends them all + to the same `file://` URL.""" + uri = Path(remotes).resolve().as_uri().rstrip("/") + "/" + return [(f"url.{uri}.insteadOf", prefix) + for prefix in insteadof_prefixes(remotes, typed=typed)] + + +def insteadof_args(remotes: Path | str, *, typed: bool = False) -> list[str]: + """`insteadof_entries` as the `-c =` pairs of one git + command. A `-c` is not stored: a clone made with it keeps the plain path + it was given as its `origin`, and a submodule its `.gitmodules` URL.""" + return [arg for key, prefix in insteadof_entries(remotes, typed=typed) + for arg in ("-c", f"{key}={prefix}")] + + def clone_just_pushed(remote: Path, dest: Path, branch: str | None = None, *, recurse: bool = True, bare: bool = False, config: Sequence[str] = (), @@ -93,7 +147,9 @@ def clone_just_pushed(remote: Path, dest: Path, `url..insteadOf` for the one command: a `-c` is not stored, so the clone's own `origin` and `submodule..url` keep the plain paths. The prefix is the directory RESOLVED, as `scaffold-project.py` writes it into - `.gitmodules`, in both of a Windows path's spellings. + `.gitmodules`, in both of a Windows path's spellings + (`insteadof_prefixes`, shared with `read_through_git` and + `submodule_update_through_git`). `branch` is `-b `: `adopt-project.py execute` pushes its split branch into the SOURCE, a plain repository, which is then `remote`. @@ -110,12 +166,7 @@ def clone_just_pushed(remote: Path, dest: Path, names a path, and a project's name can be any text. """ remote = Path(remote) - remotes = remote.parent.resolve() - uri = remotes.as_uri().rstrip("/") + "/" - prefixes = dict.fromkeys([os.path.join(str(remotes), ""), - remotes.as_posix().rstrip("/") + "/"]) - rewrites = [arg for prefix in prefixes - for arg in ("-c", f"url.{uri}.insteadOf={prefix}")] + rewrites = insteadof_args(remote.parent) if bare: mode = ["--bare"] else: @@ -138,6 +189,30 @@ def cloned_just_pushed(remote: Path, dest: Path, branch: str | None = None, return Path(dest) +def submodule_update_through_git(root: Path, remotes: Path, + *options: str + ) -> subprocess.CompletedProcess: + """`git submodule update ` in `root`, failing the test where it + fails (`git` below), with each mount it CLONES read through git and not + through the remote's files (#662, #440). + + An `--init` of a mount git keeps no repository for is git's local clone of + the plain path `.gitmodules` records for it: the walk of the remote's + `objects/` that `clone_just_pushed` keeps off, taken by a submodule. + So the plain paths under `remotes` are rewritten to `file://` for the one + command, as `clone_just_pushed` rewrites them for its legs, and the file + transport is allowed, as every test that clones a local path allows it. + `options` follow `update`: `("-q", "--init")`, `("--init", "--recursive", + "--", "code")`. + + An `update` over a mount whose repository git already keeps FETCHES, and + a fetch asks upload-pack whatever the URL is: a call that only brings + such a mount to its gitlink needs neither the rewrite nor this wrapper. + """ + return git(*FILE_PROTOCOL, *insteadof_args(remotes), + "submodule", "update", *options, cwd=root) + + # --- git's automatic maintenance, OFF for every git the session starts (#645) # # A COPY IS A WALK OF THE FILES TOO. `shutil.copytree` reads `objects/` one @@ -1503,21 +1578,18 @@ def read_through_git(remotes: Path) -> dict: """The environment that makes every git a pasted line starts read the bare repositories in `remotes` through git, not through their files (#440): `clone_just_pushed`'s `url..insteadOf` rewrites, as - `GIT_CONFIG_COUNT` entries, because the line under test is run exactly - as it was printed and a submodule's own clone inherits the environment. - The directory as typed and as resolved, so the root's URL and the legs' - `.gitmodules` URLs are both rewritten.""" - uri = remotes.resolve().as_uri().rstrip("/") + "/" - prefixes = list(dict.fromkeys( - spelling for directory in (Path(os.path.abspath(remotes)), - remotes.resolve()) - for spelling in (os.path.join(str(directory), ""), - directory.as_posix().rstrip("/") + "/"))) - env = {"GIT_CONFIG_COUNT": str(len(prefixes))} - for index, prefix in enumerate(prefixes): - env[f"GIT_CONFIG_KEY_{index}"] = f"url.{uri}.insteadOf" - env[f"GIT_CONFIG_VALUE_{index}"] = prefix - return env + `GIT_CONFIG_*` entries, because the line under test is run exactly as it + was printed and a submodule's own clone inherits the environment. The + directory as typed and as resolved (`insteadof_entries`), so the root's + URL and the legs' `.gitmodules` URLs are both rewritten. + + The entries are APPENDED to the ones `os.environ` holds + (`appended_to_command_scope`), which every caller lays this over, so the + session's own (`no_auto_maintenance`'s, and the two the Windows job sets) + still reach the pasted line, as they reach every other git a test + starts.""" + return appended_to_command_scope( + os.environ, insteadof_entries(remotes, typed=True)) def pasted(lines, cwd: Path, diff --git a/tests/test_adopt_e2e.py b/tests/test_adopt_e2e.py index 8295c4ae..9072892d 100644 --- a/tests/test_adopt_e2e.py +++ b/tests/test_adopt_e2e.py @@ -11,7 +11,6 @@ from __future__ import annotations import importlib.util -import os import re import shlex import shutil @@ -21,8 +20,8 @@ import pytest -from conftest import (ADOPT, REPO, cloned_just_pushed, git, make_source_repo, - resolve, run_script, write_plan) +from conftest import (ADOPT, REPO, cloned_just_pushed, git, insteadof_args, + make_source_repo, resolve, run_script, write_plan) sys.path.insert(0, str(REPO / "scripts")) from repo_shape import load_yaml # noqa: E402 @@ -243,21 +242,16 @@ def pasted(line: str, cwd: Path, remotes: Path) -> subprocess.CompletedProcess: object files one by one, and maintenance a push left running can pack them away mid-copy (`failed to copy file`, on the macOS runner). So `--no-local` reads the root through git, and each leg's plain path under - `remotes`, as `.gitmodules` records it (the directory resolved, in both - of a Windows path's spellings), is read as its `file://` URI. Every - printed argument is kept as printed, `-c protocol.file.allow=always` - among them; a URI is the `file` transport too, so a line without it is - refused here as it is refused pasted.""" + `remotes`, as `.gitmodules` records it (`insteadof_args`: the directory + resolved, in both of a Windows path's spellings), is read as its + `file://` URI. Every printed argument is kept as printed, `-c + protocol.file.allow=always` among them; a URI is the `file` transport + too, so a line without it is refused here as it is refused pasted.""" argv = shlex.split(line) - resolved = remotes.resolve() - uri = resolved.as_uri().rstrip("/") + "/" - prefixes = dict.fromkeys([os.path.join(str(resolved), ""), - resolved.as_posix().rstrip("/") + "/"]) at = argv.index("clone") return subprocess.run( - [*argv[:at], *(arg for prefix in prefixes - for arg in ("-c", f"url.{uri}.insteadOf={prefix}")), - "clone", "--no-local", *argv[at + 1:]], + [*argv[:at], *insteadof_args(remotes), "clone", "--no-local", + *argv[at + 1:]], cwd=str(cwd), capture_output=True, encoding="utf-8", errors="replace", check=False) diff --git a/tests/test_clone_just_pushed.py b/tests/test_clone_just_pushed.py index 8cd617eb..a4bdb997 100644 --- a/tests/test_clone_just_pushed.py +++ b/tests/test_clone_just_pushed.py @@ -10,17 +10,29 @@ no loose object, which a clone that walked the remote's own files would not (`assert_store_read_through_git`). The option itself is asked of git beside it, and the last case is the clone that fails. + +#662 wrote the rewrite those clones carry once (`insteadof_prefixes`, its +`(key, prefix)` entries and its `-c` arguments) and gave its other two users +the same list: `read_through_git`, which puts it in the environment of a +pasted line, and `submodule_update_through_git`, for the `--init` of a mount a +test brings up after the clone. The cases at the end ask git what each form +does to a plain path, with `git ls-remote --get-url`, which expands a URL as a +fetch would and talks to nothing. """ from __future__ import annotations import os +import subprocess +from pathlib import Path import pytest -from conftest import (PROJECT, assert_read_through_git, +from conftest import (FILE_PROTOCOL, PROJECT, assert_read_through_git, assert_store_read_through_git, clone_just_pushed, - cloned_just_pushed, git) + cloned_just_pushed, git, insteadof_args, + insteadof_entries, insteadof_prefixes, read_through_git, + submodule_update_through_git) @pytest.fixture @@ -100,3 +112,140 @@ def test_a_clone_that_fails_fails_the_test_and_says_why(tmp_path): assert clone_just_pushed(missing, tmp_path / "clone").returncode != 0 with pytest.raises(AssertionError, match="git clone of .* failed"): cloned_just_pushed(missing, tmp_path / "clone") + + +# --- the rewrite, written once (#662) ---------------------------------------- + +def spellings(directory: Path) -> set[str]: + """The prefixes of an existing `directory` as a person would write them: + the system's separator after it, and a slash. One on POSIX, two on + Windows. Written out here and not read from the helper.""" + return {str(directory) + os.sep, directory.as_posix() + "/"} + + +def get_url(url: str, *config: str, env: dict | None = None, + cwd: Path) -> str: + """What git makes of `url` as a fetch would, `insteadOf` rewrites + included, and talks to nothing.""" + return subprocess.run(["git", *config, "ls-remote", "--get-url", url], + cwd=str(cwd), capture_output=True, text=True, + check=True, env=env).stdout.strip() + + +@pytest.fixture +def remotes(tmp_path) -> Path: + directory = tmp_path / "remotes" + directory.mkdir() + return directory + + +@pytest.mark.parametrize("typed", [False, True], ids=["resolved", "typed"]) +@pytest.mark.parametrize("trailing", ["", "/", os.sep], + ids=["none", "slash", "separator"]) +def test_a_directory_is_named_with_a_separator_after_it_given_or_not( + remotes, typed, trailing): + """The prefix ends with a separator whether the directory was given with + one or without, so a rewrite of `/` never reaches a neighbour + that begins with the same text, and each spelling is listed once. A + directory with no link in its path is typed and resolved at once.""" + prefixes = insteadof_prefixes(str(remotes) + trailing, typed=typed) + assert set(prefixes) == spellings(remotes.resolve()), prefixes + assert len(prefixes) == len(set(prefixes)), prefixes + assert prefixes == insteadof_prefixes(remotes, typed=typed), prefixes + + +def test_a_typed_directory_adds_the_spelling_it_was_given_first(tmp_path): + """TYPED: a symlink kept, as a line a tool prints names a remote. The + resolved directory is the one `.gitmodules` records, and stays.""" + real = tmp_path / "real" + real.mkdir() + link = tmp_path / "link" + try: + link.symlink_to(real, target_is_directory=True) + except (OSError, NotImplementedError): + pytest.skip("this account cannot make a symlink") + resolved = spellings(real.resolve()) + typed = spellings(Path(os.path.abspath(link))) + assert typed.isdisjoint(resolved), "premise: a link that is not its target" + assert set(insteadof_prefixes(link)) == resolved + prefixes = insteadof_prefixes(link, typed=True) + assert set(prefixes[:len(typed)]) == typed, prefixes + assert set(prefixes) == typed | resolved, prefixes + assert len(prefixes) == len(set(prefixes)), prefixes + # Both go to the one URL: the resolved directory's, where the legs are. + uri = real.resolve().as_uri().rstrip("/") + "/" + assert {key for key, _ in insteadof_entries(link, typed=True)} == { + f"url.{uri}.insteadOf"} + + +def test_every_entry_sends_its_prefix_to_the_resolved_directorys_uri( + remotes): + uri = remotes.resolve().as_uri().rstrip("/") + "/" + entries = insteadof_entries(remotes, typed=True) + assert {key for key, _ in entries} == {f"url.{uri}.insteadOf"}, entries + assert [prefix for _, prefix in entries] == insteadof_prefixes( + remotes, typed=True) + assert insteadof_args(remotes, typed=True) == [ + arg for key, prefix in entries for arg in ("-c", f"{key}={prefix}")] + + +def test_the_arguments_and_the_environment_send_a_plain_path_to_its_uri( + remotes, tmp_path): + """Both forms of the one rewrite do to a path under `remotes` what + `clone_just_pushed` needs, and leave a neighbour's path as it is.""" + plain = str(remotes / f"{PROJECT}.git") + neighbour = str(remotes) + f"-other/{PROJECT}.git" + uri = (remotes.resolve() / f"{PROJECT}.git").as_uri() + environ = {**os.environ, **read_through_git(remotes)} + assert get_url(plain, *insteadof_args(remotes), cwd=tmp_path) == uri + assert get_url(plain, env=environ, cwd=tmp_path) == uri + assert get_url(neighbour, *insteadof_args(remotes), + cwd=tmp_path) == neighbour + assert get_url(neighbour, env=environ, cwd=tmp_path) == neighbour + + +def test_the_environment_form_keeps_what_the_environment_holds( + remotes, tmp_path, monkeypatch): + """APPENDED, as `appended_to_command_scope` appends the session's own + settings: an entry the process already has is neither overwritten nor + counted away, so the pasted line sees it as every other git does.""" + monkeypatch.setenv("GIT_CONFIG_COUNT", "1") + monkeypatch.setenv("GIT_CONFIG_KEY_0", "url.held://base/.insteadOf") + monkeypatch.setenv("GIT_CONFIG_VALUE_0", "held://") + entries = read_through_git(remotes) + assert "GIT_CONFIG_KEY_0" not in entries, entries + assert entries["GIT_CONFIG_COUNT"] == str( + 1 + len(insteadof_prefixes(remotes, typed=True))), entries + environ = {**os.environ, **entries} + assert get_url("held://x", env=environ, cwd=tmp_path) == "held://base/x" + assert get_url(str(remotes / "A.git"), env=environ, cwd=tmp_path) == ( + remotes.resolve() / "A.git").as_uri() + + +def test_an_init_of_an_empty_mount_is_read_through_git_by_the_wrapper( + scaffolded, remote, tmp_path): + """#662. The mount is empty and git keeps no repository for it, so its + `--init` is a clone of the plain path `.gitmodules` records. Through + `submodule_update_through_git` the leg arrives as one pack with no loose + object, and the `submodule..url` git registers is still the plain + path, as `-c` stores nothing. The same `--init` of the other leg, run + plain, is git's local clone and leaves the remote's loose objects loose: + the control that makes the first half mean something.""" + clone = cloned_just_pushed(remote, tmp_path / "clone", recurse=False) + assert not any((clone / "spec").iterdir()), "premise: an empty mount" + assert not any((clone / "code").iterdir()), "premise: an empty mount" + + submodule_update_through_git(clone, scaffolded["remotes"], "-q", "--init", + "--", "spec") + assert any((clone / "spec").iterdir()), "the leg was not checked out" + assert_store_read_through_git(clone / "spec", "the leg, via the wrapper") + registered = git("config", "--local", "--get", "submodule.spec.url", + cwd=clone).stdout.strip() + assert registered.endswith(f"{PROJECT}-spec.git"), registered + assert not registered.startswith("file:"), registered + + git(*FILE_PROTOCOL, "submodule", "update", "-q", "--init", "--", "code", + cwd=clone) + assert any((clone / "code").iterdir()), "the control was not checked out" + with pytest.raises(AssertionError, match="loose objects"): + assert_store_read_through_git(clone / "code", "the leg, plain") diff --git a/tests/test_doctor_legs_members_next.py b/tests/test_doctor_legs_members_next.py index 8c908562..619034eb 100644 --- a/tests/test_doctor_legs_members_next.py +++ b/tests/test_doctor_legs_members_next.py @@ -48,7 +48,8 @@ import pytest -from conftest import WINDOWS_SKIP, cloned_just_pushed, git, rmtree +from conftest import (WINDOWS_SKIP, cloned_just_pushed, git, rmtree, + submodule_update_through_git) from test_shape_doctor import ( # noqa: F401 FAMILY_MEMBER, FAMILY_NAME, GIT_IDENTITY, PROJECT, commit_all, doctor, doctor_module, holder, project, record_gitlink, rows_of, @@ -614,7 +615,7 @@ def test_the_git_line_does_not_cure_a_mount_git_cannot_read_a_head_in( def test_a_checkout_wins_over_a_lockstep_fact_and_a_rerun_names_the_other( - standard, project): + standard, project, scaffolded_here): """One leg a checkout cures (an empty mount) and one it does not (a gitlink that is not the pin): the line is the checkout, over the first leg only, and it says to run the doctor again. The re-run names the @@ -628,8 +629,8 @@ def test_a_checkout_wins_over_a_lockstep_fact_and_a_rerun_names_the_other( ), first["next"] assert "doctor again" in why_of(first), why_of(first) # The fix, applied by hand (no sh needed): the empty leg is now cured. - git("-c", "protocol.file.allow=always", "submodule", "update", "--init", - "--recursive", "--", "code", cwd=project) + submodule_update_through_git(project, scaffolded_here["base"] / "remotes", + "--init", "--recursive", "--", "code") second = row_of(standard, project, "legs") assert second["status"] == "FINDING", second assert command_of(second) == pins_gate(module, standard, project, True), \ @@ -984,8 +985,9 @@ def test_an_empty_member_with_no_pin_is_still_a_checkout_first( assert command_of(row) == ( f"git -C {module.quote_arg(root.resolve())} {UPDATE} -- " f"members/{FAMILY_MEMBER}"), row["next"] - git("-c", "protocol.file.allow=always", "submodule", "update", "--init", - "--recursive", "--", f"members/{FAMILY_MEMBER}", cwd=root) + submodule_update_through_git(root, holder["base"] / "remotes", "--init", + "--recursive", "--", + f"members/{FAMILY_MEMBER}") again = row_of(standard, root, "members") assert again["status"] == "FINDING", again assert "validate-family.py" in command_of(again), again["next"] @@ -1047,12 +1049,14 @@ def detached_member(root: Path) -> Path: return member -def with_its_own_legs(root: Path) -> Path: +def with_its_own_legs(root: Path, holder: dict) -> Path: """`family.py add` does not recurse, so the fixture's member has empty - legs: fill them, as a recursive clone of the holder does. Each is then - detached at the commit the MEMBER records for it.""" - git("-c", "protocol.file.allow=always", "submodule", "update", "--init", - "--recursive", "--", f"members/{FAMILY_MEMBER}", cwd=root) + legs: fill them, as a recursive clone of the holder does, from the + remotes the `holder` fixture made. Each is then detached at the commit + the MEMBER records for it.""" + submodule_update_through_git(root, holder["base"] / "remotes", "--init", + "--recursive", "--", + f"members/{FAMILY_MEMBER}") return root / "members" / FAMILY_MEMBER @@ -1238,11 +1242,11 @@ def test_413b_without_it_the_holders_bootstrap_leaves_the_commit_in_no_ref( assert commit in git("reflog", cwd=member).stdout -def made_in_a_members_own_leg(root: Path) -> tuple[Path, str]: +def made_in_a_members_own_leg(root: Path, holder: dict) -> tuple[Path, str]: """A commit in the member, so the row is red, and one in the member's own detached `spec` leg, which the row never reads: the leg and the second commit's sha.""" - member = with_its_own_legs(root) + member = with_its_own_legs(root, holder) git("checkout", "-q", "--detach", "HEAD", cwd=member) commit_in(member, "work in the member") leg = member / "spec" @@ -1256,7 +1260,7 @@ def test_413c_the_members_git_line_says_it_checks_out_each_members_legs( in general words, with the keep line to run in that leg first.""" root = holder_copy(holder, tmp_path) holder_without(root, "bootstrap.py") - made_in_a_members_own_leg(root) + made_in_a_members_own_leg(root, holder) module = doctor_module(standard) row = row_of(standard, root, "members") assert command_of(row) == ( @@ -1275,7 +1279,7 @@ def test_413c_the_leg_keep_line_keeps_a_members_leg_commit_through_the_paste( standard, holder, tmp_path): root = holder_copy(holder, tmp_path) holder_without(root, "bootstrap.py") - leg, commit = made_in_a_members_own_leg(root) + leg, commit = made_in_a_members_own_leg(root, holder) module = doctor_module(standard) row = row_of(standard, root, "members") keep = re.search(r"git -C LEG branch NAME", why_of(row)) @@ -1294,7 +1298,7 @@ def test_413c_without_it_the_members_leg_commit_is_in_no_ref( standard, holder, tmp_path): root = holder_copy(holder, tmp_path) holder_without(root, "bootstrap.py") - leg, commit = made_in_a_members_own_leg(root) + leg, commit = made_in_a_members_own_leg(root, holder) result = paste(row_of(standard, root, "members"), tmp_path / "elsewhere") assert result.returncode == 0, result.stdout + result.stderr assert row_of(standard, root, "members")["status"] == "ok" @@ -2078,7 +2082,7 @@ def test_an_emptied_members_own_leg_is_kept_through_its_repository( kept for the member keeps a commit made in its own `spec` leg.""" root = holder_copy(holder, tmp_path) holder_without(root, "bootstrap.py") - leg, commit = made_in_a_members_own_leg(root) + leg, commit = made_in_a_members_own_leg(root, holder) git("submodule", "deinit", "-q", "-f", "--", f"members/{FAMILY_MEMBER}", cwd=root) module = doctor_module(standard) diff --git a/tests/test_family_bump_remove_rollback.py b/tests/test_family_bump_remove_rollback.py index 3dcfe47c..f3fb458a 100644 --- a/tests/test_family_bump_remove_rollback.py +++ b/tests/test_family_bump_remove_rollback.py @@ -61,8 +61,8 @@ import pytest -from conftest import (FILE_PROTOCOL, cloned_just_pushed, git, rmtree, - run_script) +from conftest import (cloned_just_pushed, git, rmtree, run_script, + submodule_update_through_git) #: The family fixture, its names, the offline environment and the holder's #: byte-for-byte state, imported from the modules that own them rather than #: rebuilt: a holder made, or compared, any other way would test a @@ -334,8 +334,8 @@ def test_a_remove_puts_back_the_members_legs_it_had_checked_out( from the stores inside the member's own, and the holder's work tree is byte for byte what it was, theirs included.""" env = isolated(offline, tmp_path) - git(*FILE_PROTOCOL, "submodule", "update", "-q", "--init", - cwd=holder / PATH) + submodule_update_through_git(holder / PATH, family["remotes"], "-q", + "--init") legs = sorted(name for name in ("spec", "code") if (holder / PATH / name / ".git").exists()) assert legs == ["code", "spec"], "premise: both legs checked out" @@ -905,7 +905,7 @@ def test_a_leg_no_store_holds_is_left_out_not_cloned( its checkout comes back only from its remote.""" env = isolated(offline, tmp_path) mount = holder / PATH - git(*FILE_PROTOCOL, "submodule", "update", "-q", "--init", cwd=mount) + submodule_update_through_git(mount, family["remotes"], "-q", "--init") assert git("config", "-f", ".gitmodules", "submodule.spec.path", cwd=mount).stdout.strip() == "spec", "fixture drift" leg_store = Path(git("rev-parse", "--absolute-git-dir", From 8cfc8eb17d620154050aa06db90f4add2f73d1c4 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Fri, 16 Oct 2026 18:39:42 +0000 Subject: [PATCH 4/8] tests/conftest.py and tests/test_no_auto_maintenance.py: the session's global file includes a `~/` path only where `HOME` is set and includes nothing over an empty `GIT_CONFIG_GLOBAL`, a test holds the fixture to session scope, and the receive-pack test names its own hooks, so a runner with no `HOME` no longer dies on every git (#645) The review of #713 (rv645) found two things worth fixing and three small ones: - With `HOME` unset, the include of `~/.config/git/config` and `~/.gitconfig` made every git the session started exit 128 ("could not expand include path"). `session_global_config` now writes a `~/` path only where `home_is_known()` (`HOME` is set, or this is Windows), and leaves out the `[include]` header when nothing is included. An `XDG_CONFIG_HOME` that is set is still included. The two `test_a_persons_own_global_config` cases skip where `HOME` was unset. - Nothing held `no_auto_maintenance` to session scope: at function scope all seven tests still passed. A new test reads the two variables from a session fixture built first, and compares them with what the test sees. - An empty `GIT_CONFIG_GLOBAL=` is git's "no global config"; the fixture treated it as unset and included `~/.gitconfig` again. It now includes nothing. - The receive-pack test set no `core.hooksPath` of its own, so a person's global one hid the remote's `post-receive` hook. It now sets the remote's own `hooks` directory in the remote's local config, which outranks a global one. Two tests are new for the `HOME` and empty-value cases, with an XDG variant. Lane: openreposhape-5 (openRepoShape-5) Co-Authored-By: Claude Sonnet 5.5 --- tests/conftest.py | 38 +++++++++---- tests/test_no_auto_maintenance.py | 89 ++++++++++++++++++++++++++++--- 2 files changed, 111 insertions(+), 16 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index 5960f06b..434ee808 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -175,7 +175,11 @@ def cloned_just_pushed(remote: Path, dest: Path, branch: str | None = None, # settings still apply, and sets the two keys after them. A `git config # --global ` read without `--includes` sees the two alone: in this # repository that is `setup-project.py`'s Windows-only `core.autocrlf` -# warning, which no test reads from a person's own config. +# warning, which no test reads from a person's own config. An EMPTY +# `GIT_CONFIG_GLOBAL` is git's "no global config", and nothing is included. +# A `~/` path is written only where `HOME` is set, or on Windows, where Git +# for Windows sets it: with `HOME` unset git cannot expand one, and every +# git then dies `could not expand include path`. # # WHAT IT LEAVES: a git whose environment takes away both -- a # `GIT_CONFIG_COUNT` overwritten rather than appended to, together with a @@ -210,19 +214,33 @@ def appended_to_command_scope(environ: Mapping[str, str], return added +def home_is_known() -> bool: + """Whether git can expand a `~/` path: `HOME` is set, or this is Windows, + where Git for Windows finds a home of its own.""" + return bool(os.environ.get("HOME")) or os.name == "nt" + + def session_global_config(named: str | None) -> bytes: """The global config file `no_auto_maintenance` writes: an include of - what git reads as its global config without one -- `named`, the file - `GIT_CONFIG_GLOBAL` held, or else git's two default files in git's own - order -- and then `NO_AUTO_MAINTENANCE`. LF, and every path POSIX-style: - `~/` is expanded by git, from the `HOME` of the git that reads it.""" - if named: - included = [Path(os.path.abspath(named)).as_posix()] + what git reads as its global config without one, and then + `NO_AUTO_MAINTENANCE`. LF, and every path POSIX-style: `~/` is expanded by + git, from the `HOME` of the git that reads it. + + `named` is what `GIT_CONFIG_GLOBAL` held. A name is the file included. An + empty value is git's "no global config" (2.43), so nothing is included. + None, unset, is git's two default files in git's own order, the + `~/` ones only where `home_is_known()`: with `HOME` unset git dies + `could not expand include path` over one, in every command it runs.""" + if named is not None: + included = [Path(os.path.abspath(named)).as_posix()] if named else [] else: + homed = (["~/.config/git/config", "~/.gitconfig"] + if home_is_known() else []) xdg = os.environ.get("XDG_CONFIG_HOME") - included = [(Path(xdg) / "git" / "config").as_posix() if xdg - else "~/.config/git/config", "~/.gitconfig"] - lines = ["[include]"] + # `XDG_CONFIG_HOME` stands in for the first, and needs no `HOME`. + included = ([(Path(xdg) / "git" / "config").as_posix(), *homed[1:]] + if xdg else homed) + lines = ["[include]"] if included else [] lines += [f"\tpath = {_config_quoted(path)}" for path in included] for key, value in NO_AUTO_MAINTENANCE: section, name = key.split(".") diff --git a/tests/test_no_auto_maintenance.py b/tests/test_no_auto_maintenance.py index 33d55c11..8eefa140 100644 --- a/tests/test_no_auto_maintenance.py +++ b/tests/test_no_auto_maintenance.py @@ -19,7 +19,10 @@ - the `receive-pack` a push into a local remote starts, which never sees command scope, reads both settings off: a hook in the remote reports them; - the two are appended to the command scope the environment holds, and a - person's own global config is still read, in git's order, beneath them. + person's own global config is still read, in git's order, beneath them; +- the file is one git can read where `HOME` is unset, and where + `GIT_CONFIG_GLOBAL` is empty, which is git's "no global config"; +- the fixture is in place before any session fixture builds a repository. """ from __future__ import annotations @@ -32,8 +35,8 @@ import pytest from conftest import (NO_AUTO_MAINTENANCE, ORG, PROJECT, SCAFFOLD, - appended_to_command_scope, git, run_script, - session_global_config) + appended_to_command_scope, git, home_is_known, + run_script, session_global_config) #: The verbs auto maintenance runs as: `git maintenance run --auto`, and #: `git gc --auto`, which a git started itself before `maintenance` existed. @@ -143,9 +146,11 @@ def test_the_receive_pack_of_a_push_to_a_local_remote_reads_both_off( """Git starts `receive-pack` for a push to a path with no command scope, so only the session's global file reaches it. Its `post-receive` hook runs under it and writes down what git reads there. On the base it read - neither.""" + neither. The remote's own `core.hooksPath` keeps the hook found where a + person's global config sets one of its own.""" remote, work = tmp_path / "remote.git", tmp_path / "work" git("init", "-q", "--bare", "-b", "main", str(remote), cwd=tmp_path) + git("config", "core.hooksPath", (remote / "hooks").as_posix(), cwd=remote) hook = remote / "hooks" / "post-receive" hook.write_bytes( b"#!/bin/sh\nfor key in gc.auto maintenance.auto; do\n" @@ -185,8 +190,10 @@ def test_a_persons_own_global_config_is_still_read_beneath_the_two( two settings after it: a person's own values are read, and their own `maintenance.auto = true` does not turn maintenance back on. A case is skipped where the file includes another path in its place, which a test - cannot write: the session began with `GIT_CONFIG_GLOBAL` set, or, for - the second, with `XDG_CONFIG_HOME` set.""" + cannot write: the session began with `GIT_CONFIG_GLOBAL` set, with + `HOME` unset, or, for the second, with `XDG_CONFIG_HOME` set.""" + if not home_is_known(): + pytest.skip("HOME was unset, and the file includes no `~/` path") if no_auto_maintenance.read_bytes() != session_global_config(None): pytest.skip("the session began with GIT_CONFIG_GLOBAL set, and its " "file includes that one") @@ -202,3 +209,73 @@ def test_a_persons_own_global_config_is_still_read_beneath_the_two( for key, value in NO_AUTO_MAINTENANCE: assert asked("--global", "--includes", "--get", key, cwd=tmp_path) == value, key + + +@pytest.mark.skipif(os.name == "nt", + reason="Git for Windows finds a home of its own") +@pytest.mark.parametrize("xdg", [False, True], ids=["no-xdg", "xdg"]) +def test_where_home_is_unset_the_file_is_one_git_can_read( + xdg, tmp_path, monkeypatch): + """With `HOME` unset git cannot expand a `~/` path, and an include of one + made every git the session started die `could not expand include path` + (a hermetic runner, `env -i`). The file leaves them out, and an + `XDG_CONFIG_HOME` that is set is still included.""" + monkeypatch.delenv("HOME", raising=False) + monkeypatch.delenv("XDG_CONFIG_HOME", raising=False) + if xdg: + under = tmp_path / "xdg" + (under / "git").mkdir(parents=True) + (under / "git" / "config").write_bytes( + b"[openreposhape]\n\tprobe = xdg\n") + monkeypatch.setenv("XDG_CONFIG_HOME", str(under)) + config = tmp_path / "session.gitconfig" + config.write_bytes(session_global_config(None)) + monkeypatch.setenv("GIT_CONFIG_GLOBAL", str(config)) + for key, value in NO_AUTO_MAINTENANCE: + assert asked("--global", "--includes", "--get", key, + cwd=tmp_path) == value, key + assert asked("--global", "--includes", "--get", "openreposhape.probe", + cwd=tmp_path) == ("xdg" if xdg else "") + + +def test_an_empty_GIT_CONFIG_GLOBAL_is_no_global_config_and_includes_nothing( + tmp_path, monkeypatch): + """Git reads an empty `GIT_CONFIG_GLOBAL` as "no global config" (2.43), + and the file the fixture writes over it does not bring `~/.gitconfig` + back.""" + home = tmp_path / "home" + home.mkdir() + (home / ".gitconfig").write_bytes(b"[openreposhape]\n\tprobe = home\n") + monkeypatch.setenv("HOME", str(home)) + monkeypatch.delenv("XDG_CONFIG_HOME", raising=False) + config = tmp_path / "session.gitconfig" + config.write_bytes(session_global_config("")) + monkeypatch.setenv("GIT_CONFIG_GLOBAL", str(config)) + assert asked("--global", "--includes", "--get", "openreposhape.probe", + cwd=tmp_path) == "" + for key, value in NO_AUTO_MAINTENANCE: + assert asked("--global", "--includes", "--get", key, + cwd=tmp_path) == value, key + + +@pytest.fixture(scope="session") +def seen_by_a_session_fixture() -> dict: + """The session's two variables as a SESSION fixture sees them when it is + built, which is when a session fixture that makes a repository (`standard`, + `scaffolded`) makes it. Pytest builds the higher scope first, and within a + scope an autouse fixture first and then the arguments in order.""" + return {name: os.environ.get(name) + for name in ("GIT_CONFIG_GLOBAL", "GIT_CONFIG_COUNT")} + + +def test_the_settings_are_in_place_before_a_session_fixture_builds_a_repo( + seen_by_a_session_fixture, no_auto_maintenance): + """`no_auto_maintenance` at function scope, or without `autouse`, is + built after a session fixture and every test here but this one still + passes: the fixture must be session scope and autouse. A session fixture + sees the file and the count that the test sees.""" + assert seen_by_a_session_fixture == { + name: os.environ.get(name) + for name in ("GIT_CONFIG_GLOBAL", "GIT_CONFIG_COUNT")} + assert seen_by_a_session_fixture["GIT_CONFIG_GLOBAL"] == \ + str(no_auto_maintenance) From 979bb39445f95e2e23e64f51504269945765867f Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Fri, 16 Oct 2026 18:47:40 +0000 Subject: [PATCH 5/8] tests/test_no_auto_maintenance.py: the two new global-file tests share one helper that writes the session file, reads both settings back and returns the probe, so the fix round adds no duplicated test body (#645) The `HOME`-unset test and the empty-`GIT_CONFIG_GLOBAL` test each wrote `session_global_config(...)` to a file, set `GIT_CONFIG_GLOBAL` and looped over the two settings. That block is now `probe_read_from_the_session_file`, so SonarCloud's duplication gate has nothing to count. No assertion changes: the new file still gives 11 passed, and the same probes fail. Lane: openreposhape-5 (openRepoShape-5) Co-Authored-By: Claude Sonnet 5.5 --- tests/test_no_auto_maintenance.py | 34 ++++++++++++++++--------------- 1 file changed, 18 insertions(+), 16 deletions(-) diff --git a/tests/test_no_auto_maintenance.py b/tests/test_no_auto_maintenance.py index 8eefa140..c7116dd0 100644 --- a/tests/test_no_auto_maintenance.py +++ b/tests/test_no_auto_maintenance.py @@ -86,6 +86,21 @@ def asked(*args: str, cwd: Path) -> str: return git("config", *args, cwd=cwd, check=False).stdout.strip() +def probe_read_from_the_session_file(named, tmp_path, monkeypatch) -> str: + """`session_global_config(named)` written to a file that is the whole + global config for the rest of the test, read back with its includes: the + two settings are asserted, and what the `openreposhape.probe` key reads + is returned (empty where no included file sets it).""" + config = tmp_path / "session.gitconfig" + config.write_bytes(session_global_config(named)) + monkeypatch.setenv("GIT_CONFIG_GLOBAL", str(config)) + for key, value in NO_AUTO_MAINTENANCE: + assert asked("--global", "--includes", "--get", key, + cwd=tmp_path) == value, key + return asked("--global", "--includes", "--get", "openreposhape.probe", + cwd=tmp_path) + + @pytest.fixture def behind(tmp_path) -> Path: """A clone of a local bare remote that has moved on by a commit, as a @@ -228,14 +243,8 @@ def test_where_home_is_unset_the_file_is_one_git_can_read( (under / "git" / "config").write_bytes( b"[openreposhape]\n\tprobe = xdg\n") monkeypatch.setenv("XDG_CONFIG_HOME", str(under)) - config = tmp_path / "session.gitconfig" - config.write_bytes(session_global_config(None)) - monkeypatch.setenv("GIT_CONFIG_GLOBAL", str(config)) - for key, value in NO_AUTO_MAINTENANCE: - assert asked("--global", "--includes", "--get", key, - cwd=tmp_path) == value, key - assert asked("--global", "--includes", "--get", "openreposhape.probe", - cwd=tmp_path) == ("xdg" if xdg else "") + assert probe_read_from_the_session_file( + None, tmp_path, monkeypatch) == ("xdg" if xdg else "") def test_an_empty_GIT_CONFIG_GLOBAL_is_no_global_config_and_includes_nothing( @@ -248,14 +257,7 @@ def test_an_empty_GIT_CONFIG_GLOBAL_is_no_global_config_and_includes_nothing( (home / ".gitconfig").write_bytes(b"[openreposhape]\n\tprobe = home\n") monkeypatch.setenv("HOME", str(home)) monkeypatch.delenv("XDG_CONFIG_HOME", raising=False) - config = tmp_path / "session.gitconfig" - config.write_bytes(session_global_config("")) - monkeypatch.setenv("GIT_CONFIG_GLOBAL", str(config)) - assert asked("--global", "--includes", "--get", "openreposhape.probe", - cwd=tmp_path) == "" - for key, value in NO_AUTO_MAINTENANCE: - assert asked("--global", "--includes", "--get", key, - cwd=tmp_path) == value, key + assert probe_read_from_the_session_file("", tmp_path, monkeypatch) == "" @pytest.fixture(scope="session") From deb029c926d9336ee2dc6c5d3590bca3b3040834 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Fri, 16 Oct 2026 19:05:04 +0000 Subject: [PATCH 6/8] tests/test_clone_just_pushed.py and tests/conftest.py: `read_through_git` is asked about a remotes directory typed through a symlink, so dropping its `typed=True` no longer passes, and its docstring says the environment it appends to is the one it is laid over at the same moment (#662) rv662's mutant N3 (`typed=False` in `read_through_git`) passed every test: `typed` matters only where the caller spells the directory through a symlink, and the symlink case called `insteadof_prefixes` directly. The new case makes `link` to the remotes directory and asks git (`ls-remote --get-url`) what the environment `read_through_git(link)` does to `link/A.git` and to `remotes/A.git`: both go to the resolved directory's file URI. The mutant now fails it (1 failed, 18 passed). A small helper, `a_link_to`, makes the link for this case and the existing symlink case, and skips where the account cannot. `read_through_git` reads `os.environ` when it is called, so its result is only right laid over `os.environ` at the same moment, as `pasted` does; the docstring now says so. Lane: openreposhape-5 (openRepoShape-5) Co-Authored-By: Claude Sonnet 5.5 --- tests/conftest.py | 8 ++++---- tests/test_clone_just_pushed.py | 31 ++++++++++++++++++++++++++----- 2 files changed, 30 insertions(+), 9 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index 834b9d3e..58d73d53 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1602,10 +1602,10 @@ def read_through_git(remotes: Path) -> dict: URL and the legs' `.gitmodules` URLs are both rewritten. The entries are APPENDED to the ones `os.environ` holds - (`appended_to_command_scope`), which every caller lays this over, so the - session's own (`no_auto_maintenance`'s, and the two the Windows job sets) - still reach the pasted line, as they reach every other git a test - starts.""" + (`appended_to_command_scope`), which every caller lays this over at the + same moment, so the session's own (`no_auto_maintenance`'s, and the two + the Windows job sets) still reach the pasted line, as they reach every + other git a test starts.""" return appended_to_command_scope( os.environ, insteadof_entries(remotes, typed=True)) diff --git a/tests/test_clone_just_pushed.py b/tests/test_clone_just_pushed.py index a4bdb997..f85d4ed6 100644 --- a/tests/test_clone_just_pushed.py +++ b/tests/test_clone_just_pushed.py @@ -154,16 +154,23 @@ def test_a_directory_is_named_with_a_separator_after_it_given_or_not( assert prefixes == insteadof_prefixes(remotes, typed=typed), prefixes +def a_link_to(directory: Path, tmp_path: Path) -> Path: + """`tmp_path/link`, a symlink to `directory`, or a skip where this + account cannot make one.""" + link = tmp_path / "link" + try: + link.symlink_to(directory, target_is_directory=True) + except (OSError, NotImplementedError): + pytest.skip("this account cannot make a symlink") + return link + + def test_a_typed_directory_adds_the_spelling_it_was_given_first(tmp_path): """TYPED: a symlink kept, as a line a tool prints names a remote. The resolved directory is the one `.gitmodules` records, and stays.""" real = tmp_path / "real" real.mkdir() - link = tmp_path / "link" - try: - link.symlink_to(real, target_is_directory=True) - except (OSError, NotImplementedError): - pytest.skip("this account cannot make a symlink") + link = a_link_to(real, tmp_path) resolved = spellings(real.resolve()) typed = spellings(Path(os.path.abspath(link))) assert typed.isdisjoint(resolved), "premise: a link that is not its target" @@ -204,6 +211,20 @@ def test_the_arguments_and_the_environment_send_a_plain_path_to_its_uri( assert get_url(neighbour, env=environ, cwd=tmp_path) == neighbour +def test_the_environment_form_reads_a_path_typed_through_a_symlink( + remotes, tmp_path): + """`read_through_git` is the one user of `typed=True`: the line a tool + prints names the remotes as the person typed them, a symlink and all, and + only the TYPED spelling matches that path. Both go to the resolved + directory's URI.""" + link = a_link_to(remotes, tmp_path) + environ = {**os.environ, **read_through_git(link)} + assert get_url(str(link / "A.git"), env=environ, cwd=tmp_path) == ( + remotes.resolve() / "A.git").as_uri() + assert get_url(str(remotes / "A.git"), env=environ, cwd=tmp_path) == ( + remotes.resolve() / "A.git").as_uri() + + def test_the_environment_form_keeps_what_the_environment_holds( remotes, tmp_path, monkeypatch): """APPENDED, as `appended_to_command_scope` appends the session's own From b6e15591d50f9cf8cc4612f93994909798988cd7 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Sat, 17 Oct 2026 11:02:58 +0000 Subject: [PATCH 7/8] tests/test_doctor_legs_members_next.py and 24 other files: tests append what they tell git to the `GIT_CONFIG_COUNT` entries the environment holds, so a pasted line keeps the session's maintenance settings and the Windows job's two (#758) `PASTE_ENV` was a copy of `os.environ` taken when the module was imported, with `GIT_CONFIG_COUNT` set to 1. On deb029c9 a line pasted through `paste()` listed `protocol.file.allow` in git's command scope and nothing else, and no global file: the copy held neither of #645's two settings, which the session fixture puts in `os.environ` at session start, nor the two entries the Windows job sets. `paste_env()` builds the environment at each call and appends `protocol.file.allow` to the entries `os.environ` holds. The other tests that gave git a setting through `GIT_CONFIG_COUNT` do the same, through one call to `appended_to_command_scope` each: - the six hand-built appends beside it (`test_adopt_git_reads.py`, `test_adopt_nested_attributes.py`, `test_adopt_ignored_shape_files.py`, `test_adopt_root_symlinks.py`, `test_bootstrap_after_bump.py`, `test_family_bump_remove_rollback.py`); - the environments that set a count of their own, which replaced the entries the session and the job hold: `ALLOW_FILE_PROTOCOL` (now `allow_file_protocol()`), `QUOTE_PATH_OFF`, `SYMLINKS_OFF` and `SYMLINKS_ON`, `NO_MAINTENANCE`, and eight one-off environments. Five of the six `git clone --no-local` calls go through `cloned_just_pushed`, and the two that copy a member (`clone_member`) and the private bare remote (`leg_remote`) assert what the clone's options give; the sixth is a `--filter` clone of a file URI, which the helper cannot take. `tests/test_no_auto_maintenance.py` pastes a line under an environment that holds four entries and a global file, and a hygiene test fails any test file that writes a count as a number or hands `appended_to_command_scope` a dict written out. Two comments about git's maintenance now say what i645 measured. Lane: openreposhape-5 (openRepoShape-5) Co-Authored-By: Claude Sonnet 5.5 --- tests/conftest.py | 13 +- tests/test_adopt_empty_symlinks.py | 29 ++-- tests/test_adopt_git_reads.py | 17 +- tests/test_adopt_ignored_shape_files.py | 11 +- tests/test_adopt_nested_attributes.py | 10 +- tests/test_adopt_plan_yaml_quoting.py | 10 +- tests/test_adopt_root_symlinks.py | 45 ++---- tests/test_adopt_split_branch.py | 18 ++- tests/test_bootstrap_after_bump.py | 9 +- tests/test_bump_leg_dry_run_truth.py | 27 ++-- tests/test_clone_just_pushed.py | 18 ++- tests/test_doctor_legs_members_next.py | 72 +++++---- tests/test_doctor_next_is_a_command.py | 18 +-- tests/test_family.py | 27 ++-- tests/test_family_bump_remove_rollback.py | 9 +- tests/test_family_bump_to_checks.py | 4 +- tests/test_family_dry_run.py | 42 +++-- tests/test_family_siblings_placement.py | 10 +- tests/test_no_auto_maintenance.py | 182 +++++++++++++++++++++- tests/test_non_utf8_git_reads.py | 33 ++-- tests/test_scaffold_free_text_values.py | 10 +- tests/test_scaffold_reuse_seeded_legs.py | 17 +- tests/test_setup_project_py.py | 14 +- tests/test_setup_sh.py | 12 +- tests/test_siblings_family_name.py | 6 +- 25 files changed, 435 insertions(+), 228 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index 58d73d53..cfee5c9d 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -225,10 +225,13 @@ def submodule_update_through_git(root: Path, remotes: Path, # the remote, each run `git maintenance run --auto` as they finish. Since git # 2.47 that run DETACHES, holding `objects/maintenance.lock`, and the command # returns while it goes on; since 2.54 its default strategy (`geometric`) -# repacks a repository of a few hundred loose objects and deletes the ones +# repacks once its estimate, taken from `objects/17` alone, is above 256 +# loose objects, so two loose objects there are enough, and deletes the ones # it packed. A copy taken in that moment fails `[Errno 2] No such file or -# directory` on one of them: `holder` and `the_standard_moved`, on the -# Linux runner (#645). +# directory` on one of them (`holder` and `the_standard_moved`, on the +# Linux runner, #645), and a listing of `.git/objects` taken before and +# after it differs: `test_family_bump_remove_rollback.py`'s `objects +# changed`, 37 loose objects packed into two. # # So the session turns it off, as every git command in this estate is told # to: `gc.auto=0`, for a git that runs `git gc --auto` itself (2.43's @@ -261,7 +264,9 @@ def submodule_update_through_git(root: Path, remotes: Path, # `GIT_CONFIG_GLOBAL` of a test's own, or an environment built from nothing # -- and a push made under a global file of a test's own. No repository the # suite copies is made, or pushed to, that way today. -# `tests/test_no_auto_maintenance.py` holds both scopes to account. +# `tests/test_no_auto_maintenance.py` holds both scopes to account, and fails a +# test file that sets `GIT_CONFIG_COUNT` to a number of its own instead of +# appending (#758). #: The two settings, in the order they are appended and written. NO_AUTO_MAINTENANCE = (("gc.auto", "0"), ("maintenance.auto", "false")) diff --git a/tests/test_adopt_empty_symlinks.py b/tests/test_adopt_empty_symlinks.py index 2b8213d7..dd4075dd 100644 --- a/tests/test_adopt_empty_symlinks.py +++ b/tests/test_adopt_empty_symlinks.py @@ -28,6 +28,7 @@ from __future__ import annotations import importlib.util +import os import re import shutil import subprocess @@ -37,8 +38,8 @@ import pytest -from conftest import ADOPT, as_human, git, make_source_repo, run_script, \ - write_plan +from conftest import (ADOPT, appended_to_command_scope, as_human, git, + make_source_repo, run_script, write_plan) needs_filter_repo = pytest.mark.skipif( shutil.which("git-filter-repo") is None, @@ -61,10 +62,13 @@ #: The issue's small source: a spec, the code, and a README the root keeps. TREE = {"src/app.py": "print(1)\n", "docs/spec.md": "# Spec\n", "README.md": "# Thing\n"} -#: `core.symlinks=false` for every git a run starts, as Git for Windows has it -#: by default. -SYMLINKS_OFF = {"GIT_CONFIG_COUNT": "1", "GIT_CONFIG_KEY_0": "core.symlinks", - "GIT_CONFIG_VALUE_0": "false"} + + +def symlinks_off() -> dict: + """`core.symlinks=false` for every git a run starts, as Git for Windows + has it by default: appended, when it is called, to the git command scope + the environment holds (#758).""" + return appended_to_command_scope(os.environ, [("core.symlinks", "false")]) @pytest.fixture(scope="module") @@ -273,8 +277,8 @@ def test_check_reports_it_where_git_makes_no_symlinks_too(tmp_path): file and the adoption verified: `check` reads the tree, not the disk, so `core.symlinks=false` changes nothing it says.""" plan = planned(source_with_links(tmp_path, "docs/empty-link"), - tmp_path / "plan.yaml", env=SYMLINKS_OFF) - off, on = check(plan, env=SYMLINKS_OFF), check(plan) + tmp_path / "plan.yaml", env=symlinks_off()) + off, on = check(plan, env=symlinks_off()), check(plan) assert off.returncode == on.returncode == 1 assert findings_of(off) == findings_of(on) assert len(findings_of(off)) == 1 @@ -342,20 +346,21 @@ def test_execute_refuses_it_before_any_leg_exists( @needs_filter_repo -@pytest.mark.parametrize("env", [ - pytest.param(None, id="symlinks-on"), - pytest.param(SYMLINKS_OFF, id="symlinks-off"), +@pytest.mark.parametrize("off", [ + pytest.param(False, id="symlinks-on"), + pytest.param(True, id="symlinks-off"), ]) @pytest.mark.parametrize("link, answers", [ pytest.param("docs/empty-link", {}, id="in-docs"), pytest.param("empty-link", {"empty-link": "drop"}, id="top-level-drop"), ]) def test_execute_refuses_it_end_to_end_and_the_remote_dir_stays_empty( - tmp_path, env, link, answers): + tmp_path, off, link, answers): """The issue's runs as commands: on main, both leg repositories and exit 2 with nothing said (symlinks on), or `adoption verified` over a leg no clone with symlinks on can check out (symlinks off). Refused before a leg is made, so the EMPTY `--local-remote-dir` stays empty.""" + env = symlinks_off() if off else None source = source_with_links(tmp_path, link) plan = planned(source, tmp_path / "plan.yaml", answers, env=env) (tmp_path / REMOTES).mkdir() diff --git a/tests/test_adopt_git_reads.py b/tests/test_adopt_git_reads.py index a327e56f..e2ec94c2 100644 --- a/tests/test_adopt_git_reads.py +++ b/tests/test_adopt_git_reads.py @@ -31,7 +31,7 @@ import pytest -from conftest import REPO, git, make_source_repo +from conftest import REPO, appended_to_command_scope, git, make_source_repo from test_adopt_template_values import (ADOPT_REFUSED, PLAN_FILE, PLAN_OK, PROJECT, REMOTES, VERIFIED, WORK, adopted, answer, @@ -103,10 +103,8 @@ def command_scope(key: str, value: bytes) -> dict: The value goes in as the bytes it is, through `os.fsdecode`, which is how `sys.argv` and `os.environ` carry a byte that is not UTF-8 on POSIX. A Windows environment is text, so the caller does not use this there.""" - held = int(os.environ.get("GIT_CONFIG_COUNT") or 0) - return {"GIT_CONFIG_COUNT": str(held + 1), - f"GIT_CONFIG_KEY_{held}": key, - f"GIT_CONFIG_VALUE_{held}": os.fsdecode(value)} + return appended_to_command_scope(os.environ, + [(key, os.fsdecode(value))]) def global_user_name(base: Path, name: bytes) -> dict: @@ -192,11 +190,10 @@ def test_the_user_name_given_is_appended_to_the_config_the_environment_holds( drop both, and every test after it would run with git's own line-end and long-path defaults. The two slots here are the job's, under keys no config of a machine this runs on sets.""" - for variable, value in {"GIT_CONFIG_COUNT": "2", - "GIT_CONFIG_KEY_0": "test.heldone", - "GIT_CONFIG_VALUE_0": "one", - "GIT_CONFIG_KEY_1": "test.heldtwo", - "GIT_CONFIG_VALUE_1": "two"}.items(): + held = appended_to_command_scope( + {}, [("test.heldone", "one"), ("test.heldtwo", "two")]) + assert held["GIT_CONFIG_COUNT"] == "2", held + for variable, value in held.items(): monkeypatch.setenv(variable, value) extra = global_user_name(tmp_path, LATIN1_NAME) assert extra["GIT_CONFIG_COUNT"] == "3", extra diff --git a/tests/test_adopt_ignored_shape_files.py b/tests/test_adopt_ignored_shape_files.py index 46841f0e..29992959 100644 --- a/tests/test_adopt_ignored_shape_files.py +++ b/tests/test_adopt_ignored_shape_files.py @@ -41,9 +41,9 @@ import pytest -from conftest import (ADOPT, REPO, clear_ambient_pin_sources, - cloned_just_pushed, git, make_source_repo, resolve, - run_script, write_plan) +from conftest import (ADOPT, REPO, appended_to_command_scope, + clear_ambient_pin_sources, cloned_just_pushed, git, + make_source_repo, resolve, run_script, write_plan) sys.path.insert(0, str(REPO / "scripts")) from repo_shape import parse_yaml # noqa: E402 @@ -582,9 +582,8 @@ def command_scope(key: str, value: str) -> dict: `core.autocrlf=false` there, which outranks any config file, so a clone's own `--config` would not reach its checkout. The last entry of a key is the one git reads.""" - held = int(os.environ.get("GIT_CONFIG_COUNT") or 0) - return {**os.environ, "GIT_CONFIG_COUNT": str(held + 1), - f"GIT_CONFIG_KEY_{held}": key, f"GIT_CONFIG_VALUE_{held}": value} + return {**os.environ, + **appended_to_command_scope(os.environ, [(key, value)])} @needs_filter_repo diff --git a/tests/test_adopt_nested_attributes.py b/tests/test_adopt_nested_attributes.py index 629b209c..8e77a4bd 100644 --- a/tests/test_adopt_nested_attributes.py +++ b/tests/test_adopt_nested_attributes.py @@ -45,9 +45,9 @@ import pytest -from conftest import (ADOPT, REPO, clone_just_pushed, findings, git, - make_source_repo, planned, refused_before_any_leg, - run_script) +from conftest import (ADOPT, REPO, appended_to_command_scope, + clone_just_pushed, findings, git, make_source_repo, + planned, refused_before_any_leg, run_script) sys.path.insert(0, str(REPO / "scripts")) from repo_shape import parse_yaml # noqa: E402 @@ -378,9 +378,7 @@ def command_scope(key: str, value: str) -> dict: """`key=value` in git's command scope (`GIT_CONFIG_COUNT`), appended to what the environment holds: the Windows job sets `core.autocrlf` there, and the last entry of a key is the one git reads.""" - held = int(os.environ.get("GIT_CONFIG_COUNT") or 0) - return {"GIT_CONFIG_COUNT": str(held + 1), - f"GIT_CONFIG_KEY_{held}": key, f"GIT_CONFIG_VALUE_{held}": value} + return appended_to_command_scope(os.environ, [(key, value)]) def checked(plan: Path, env: dict): diff --git a/tests/test_adopt_plan_yaml_quoting.py b/tests/test_adopt_plan_yaml_quoting.py index dd50350e..3629931e 100644 --- a/tests/test_adopt_plan_yaml_quoting.py +++ b/tests/test_adopt_plan_yaml_quoting.py @@ -66,6 +66,7 @@ import functools import importlib.util import itertools +import os import random import re import shutil @@ -74,8 +75,9 @@ import pytest -from conftest import (ADOPT, REPO, cloned_just_pushed, git, make_source_repo, - resolve, run_script, write_plan) +from conftest import (ADOPT, REPO, appended_to_command_scope, + cloned_just_pushed, git, make_source_repo, resolve, + run_script, write_plan) sys.path.insert(0, str(REPO / "scripts")) import repo_shape # noqa: E402 @@ -941,8 +943,8 @@ def test_plan_takes_an_elector_from_git_the_manifests_quotes_read_back( result = run_script( ADOPT, "plan", "--source", str(source), PROJECT_FLAG, PROJECT_NAME, "--org", TEST_ORG, "--elected-on", ELECTION_DATE, "--out", str(plan), - env={"GIT_CONFIG_COUNT": "1", "GIT_CONFIG_KEY_0": "user.name", - "GIT_CONFIG_VALUE_0": DOMAIN_USER}) + env=appended_to_command_scope(os.environ, + [("user.name", DOMAIN_USER)])) assert result.returncode == 0, result.stdout + result.stderr assert '\nelected_by: "CORP\\\\tom"\n' in text_of(plan) for path, leg in ANSWERS: diff --git a/tests/test_adopt_root_symlinks.py b/tests/test_adopt_root_symlinks.py index cd3661ff..a8261a24 100644 --- a/tests/test_adopt_root_symlinks.py +++ b/tests/test_adopt_root_symlinks.py @@ -62,9 +62,9 @@ import pytest -from conftest import (ADOPT, REPO, clear_ambient_pin_sources, - cloned_just_pushed, git, make_source_repo, run_script, - write_plan) +from conftest import (ADOPT, REPO, appended_to_command_scope, + clear_ambient_pin_sources, cloned_just_pushed, git, + make_source_repo, run_script, write_plan) sys.path.insert(0, str(REPO / "scripts")) from shape_materialize import ( # noqa: E402 @@ -142,7 +142,6 @@ BOTH_PUSHED = "and both legs are pushed." #: The materializer's refusal of a path that follows too many symlinks. SYMLINK_LOOP = "materialize-symlink-loop" -CONFIG_VALUE = "GIT_CONFIG_VALUE_0" #: Case twins of paths the shape writes, as a macOS or Windows disk reads #: them (S1), and the rest of what several cases name. GITHUB_TWIN = ".GitHub" @@ -186,12 +185,9 @@ #: `git` told to check a symlink out as a plain file, the way Git for #: Windows' default does, or `git init` on a filesystem without symlinks. -#: `GIT_CONFIG_*` outranks every configuration file. -CONFIG_COUNT = "GIT_CONFIG_COUNT" +#: `GIT_CONFIG_*` outranks every configuration file. Set by `told_git`, after +#: the entries the environment holds. CORE_SYMLINKS = "core.symlinks" -SYMLINKS_OFF = {CONFIG_COUNT: "1", "GIT_CONFIG_KEY_0": CORE_SYMLINKS, - CONFIG_VALUE: "false"} -SYMLINKS_ON = {**SYMLINKS_OFF, CONFIG_VALUE: "true"} #: What this runner lacks, if anything, for the end-to-end tests. NO_FILTER_REPO = shutil.which("git-filter-repo") is None @@ -223,16 +219,15 @@ def told_git(monkeypatch, value: str) -> None: """`git` told `core.symlinks=` through `GIT_CONFIG_*`, after whatever entries the runner's job sets there (the Windows job sets two), which stay as they were.""" - held = int(os.environ.get(CONFIG_COUNT) or 0) - monkeypatch.setenv(CONFIG_COUNT, str(held + 1)) - monkeypatch.setenv(f"GIT_CONFIG_KEY_{held}", CORE_SYMLINKS) - monkeypatch.setenv(f"GIT_CONFIG_VALUE_{held}", value) + for name, entry in appended_to_command_scope( + os.environ, [(CORE_SYMLINKS, value)]).items(): + monkeypatch.setenv(name, entry) def links_as_files(monkeypatch) -> None: """`git` told to check a symlink out as a plain file, after the runner's own `GIT_CONFIG_*`, as Git for Windows' default does.""" - told_git(monkeypatch, SYMLINKS_OFF[CONFIG_VALUE]) + told_git(monkeypatch, "false") def no_probe(_work_root): @@ -1296,8 +1291,7 @@ def test_execute_refuses_a_link_it_would_check_out_as_a_file( and the refusal is `execute`'s own, after that list (#531 changes what a finding says, not that order).""" _, plan = github_to_ci(tmp_path) - for key, value in SYMLINKS_OFF.items(): - monkeypatch.setenv(key, value) + links_as_files(monkeypatch) checked = check(plan) assert checked.returncode == 0, checked.stderr + checked.stdout refusal = refused_by(adopter, execute_args(tmp_path, plan)) @@ -1324,8 +1318,7 @@ def test_a_case_twin_checked_out_as_a_file_is_named_as_one( disk it means, as `check`'s finding does (S1).""" _, plan = planned(tmp_path, {CI_FILE: ON_PUSH}, {GITHUB_TWIN: "ci"}, {CI_DIR: ROOT, GITHUB_TWIN: ROOT}) - for key, value in SYMLINKS_OFF.items(): - monkeypatch.setenv(key, value) + links_as_files(monkeypatch) refusal = refused_by(adopter, execute_args(tmp_path, plan)) assert refusal.code == SYMLINKS_OFF_CODE assert refusal.detail.startswith( @@ -1337,10 +1330,9 @@ def test_a_case_twin_checked_out_as_a_file_is_named_as_one( def test_the_probe_answers_what_git_would_do(adopter, tmp_path, monkeypatch): - for key, value in SYMLINKS_ON.items(): - monkeypatch.setenv(key, value) + told_git(monkeypatch, "true") assert adopter._checks_out_symlinks(tmp_path) is True - monkeypatch.setenv(CONFIG_VALUE, "false") + told_git(monkeypatch, "false") assert adopter._checks_out_symlinks(tmp_path) is False assert list(tmp_path.iterdir()) == [] @@ -1348,8 +1340,7 @@ def test_the_probe_answers_what_git_would_do(adopter, tmp_path, monkeypatch): def test_with_symlinks_on_the_same_plan_passes_the_probe(adopter, tmp_path, monkeypatch): _, plan = github_to_ci(tmp_path) - for key, value in SYMLINKS_ON.items(): - monkeypatch.setenv(key, value) + told_git(monkeypatch, "true") loaded = adopter.Plan.load(plan) opened = loaded.open_source(None, tmp_path / READ) assert adopter._refuse_symlinks_checked_out_as_files( @@ -1375,8 +1366,7 @@ def test_an_adoption_with_no_link_beneath_a_shape_path_makes_no_probe( """An ordinary adoption runs exactly as before: nothing is made in the work directory to ask `git` a question this plan does not raise.""" monkeypatch.setattr(adopter, PROBE, no_probe) - for key, value in SYMLINKS_OFF.items(): - monkeypatch.setenv(key, value) + links_as_files(monkeypatch) _, plan = planned(tmp_path, files, links, legs) loaded = adopter.Plan.load(plan) opened = loaded.open_source(None, tmp_path / READ) @@ -1545,7 +1535,7 @@ def test_each_command_asks_its_own_work_directory(adopter, tmp_path, f"\tpath = {included.as_posix()}\n", encoding=UTF8) monkeypatch.setenv("GIT_CONFIG_GLOBAL", str(config)) told = (git_reads_symlinks_as(off), git_reads_symlinks_as(on)) - if told != (SYMLINKS_OFF[CONFIG_VALUE], ""): + if told != ("false", ""): pytest.skip(f"git here does not tell the two directories apart: " f"{told}, from this runner's own configuration") _, at = planned(base / "at", {}, {CLAUDE: NOWHERE_MD}, {CLAUDE: ROOT}) @@ -2068,8 +2058,7 @@ def test_with_symlinks_off_execute_adopts_beside_the_file( link as the source had it, as main does. Before, `check` refused it, and so did `execute`, before any leg.""" source, plan = planned(tmp_path, files, links, legs) - for key, value in SYMLINKS_OFF.items(): - monkeypatch.setenv(key, value) + links_as_files(monkeypatch) adopted_beside(execute(tmp_path, plan), source, links) diff --git a/tests/test_adopt_split_branch.py b/tests/test_adopt_split_branch.py index ce49c85d..17d103dc 100644 --- a/tests/test_adopt_split_branch.py +++ b/tests/test_adopt_split_branch.py @@ -29,15 +29,16 @@ from __future__ import annotations import importlib.util +import os import shutil from argparse import Namespace from pathlib import Path import pytest -from conftest import ADOPT, FILE_PROTOCOL, GITMODULES, as_human, findings, \ - git, make_source_repo, planned, put_gitmodules, refused_before_any_leg, \ - run_script, spelled +from conftest import ADOPT, FILE_PROTOCOL, GITMODULES, \ + appended_to_command_scope, as_human, findings, git, make_source_repo, \ + planned, put_gitmodules, refused_before_any_leg, run_script, spelled needs_filter_repo = pytest.mark.skipif( shutil.which("git-filter-repo") is None, @@ -499,11 +500,12 @@ def github_at(root: Path, monkeypatch) -> None: `url..insteadOf`, in the environment every `git` this process starts inherits. The org is in it because git takes the LONGEST `insteadOf` that matches, and a workstation may already rewrite - `https://github.com/` (to `git@github.com:`, say) in its own config.""" - monkeypatch.setenv("GIT_CONFIG_COUNT", "1") - monkeypatch.setenv("GIT_CONFIG_KEY_0", - f"url.{(root / ORG).as_posix()}/.insteadOf") - monkeypatch.setenv("GIT_CONFIG_VALUE_0", f"https://github.com/{ORG}/") + `https://github.com/` (to `git@github.com:`, say) in its own config. The + entry is appended to the command scope the environment holds.""" + for name, value in appended_to_command_scope(os.environ, [ + (f"url.{(root / ORG).as_posix()}/.insteadOf", + f"https://github.com/{ORG}/")]).items(): + monkeypatch.setenv(name, value) def a_github(tmp_path: Path, monkeypatch, branches=(SPLIT_BRANCH,)) -> Path: diff --git a/tests/test_bootstrap_after_bump.py b/tests/test_bootstrap_after_bump.py index 0acbdb59..aa36c906 100644 --- a/tests/test_bootstrap_after_bump.py +++ b/tests/test_bootstrap_after_bump.py @@ -89,8 +89,8 @@ import pytest -from conftest import (ORG, PROJECT, REPO, SCAFFOLD, cloned_just_pushed, git, - run_script) +from conftest import (ORG, PROJECT, REPO, SCAFFOLD, appended_to_command_scope, + cloned_just_pushed, git, run_script) BUMP = REPO / "scripts" / "bump-leg.py" MAIN = "refs/heads/main" @@ -148,10 +148,7 @@ def configured(key: str, value: str) -> dict: and `GIT_CONFIG_VALUE_` (git 2.31), never `git config`. APPENDED to what the environment holds: the Windows job sets two entries already (`.github/workflows/tests.yml`), and a count of 1 would drop both.""" - held = int(os.environ.get("GIT_CONFIG_COUNT") or 0) - return {"GIT_CONFIG_COUNT": str(held + 1), - f"GIT_CONFIG_KEY_{held}": key, - f"GIT_CONFIG_VALUE_{held}": value} + return appended_to_command_scope(os.environ, [(key, value)]) def failing_post_checkout(tmp_path: Path) -> dict: diff --git a/tests/test_bump_leg_dry_run_truth.py b/tests/test_bump_leg_dry_run_truth.py index 86a51187..384860fe 100644 --- a/tests/test_bump_leg_dry_run_truth.py +++ b/tests/test_bump_leg_dry_run_truth.py @@ -43,13 +43,13 @@ from __future__ import annotations import re -import subprocess import sys from pathlib import Path import pytest -from conftest import FILE_PROTOCOL, ORG, PROJECT, REPO, git, run_script +from conftest import (FILE_PROTOCOL, ORG, PROJECT, REPO, cloned_just_pushed, + git, run_script) from test_bump_leg import ( BUMP, SPEC_PIN, branched, bump, commit_spec_pin, committed, reference_both_legs, spec_ref, validate, @@ -90,28 +90,21 @@ def leg_remote(scaffolded, tmp_path_factory) -> dict: own bare repositories are only ever read, so a tag pushed to this one reaches no other module's leg. - `--no-local`, as `clone_just_pushed` reads a remote a scaffold has just - written (#440): through upload-pack, which a repack does not break. + Made with `cloned_just_pushed`, which reads a remote a scaffold has just + written through upload-pack, which a repack does not break (#440, #758). """ remotes = tmp_path_factory.mktemp("w534-remotes") - proc = subprocess.run( - ["git", *FILE_PROTOCOL, "clone", "-q", "--bare", "--no-local", - str(scaffolded["remotes"] / f"{PROJECT}-spec.git"), - str(remotes / f"{PROJECT}-spec.git")], - capture_output=True, text=True, check=False) - assert proc.returncode == 0, proc.stderr - return {"remotes": remotes, "bare": remotes / f"{PROJECT}-spec.git"} + bare = cloned_just_pushed(scaffolded["remotes"] / f"{PROJECT}-spec.git", + remotes / f"{PROJECT}-spec.git", bare=True) + assert git("rev-parse", "--is-bare-repository", + cwd=bare).stdout.strip() == "true" + return {"remotes": remotes, "bare": bare} def private_work(leg_remote: dict, tmp_path_factory, name: str) -> Path: """A clone of the private remote to make commits in and push from.""" work = tmp_path_factory.mktemp(f"w534-{name}") / f"{PROJECT}-spec" - proc = subprocess.run( - ["git", *FILE_PROTOCOL, "clone", "-q", "--no-local", - str(leg_remote["bare"]), str(work)], - capture_output=True, text=True, check=False) - assert proc.returncode == 0, proc.stderr - return work + return cloned_just_pushed(leg_remote["bare"], work, recurse=False) def tagged(work: Path, name: str, target: str) -> str: diff --git a/tests/test_clone_just_pushed.py b/tests/test_clone_just_pushed.py index f85d4ed6..823e52f3 100644 --- a/tests/test_clone_just_pushed.py +++ b/tests/test_clone_just_pushed.py @@ -28,11 +28,11 @@ import pytest -from conftest import (FILE_PROTOCOL, PROJECT, assert_read_through_git, - assert_store_read_through_git, clone_just_pushed, - cloned_just_pushed, git, insteadof_args, - insteadof_entries, insteadof_prefixes, read_through_git, - submodule_update_through_git) +from conftest import (FILE_PROTOCOL, PROJECT, appended_to_command_scope, + assert_read_through_git, assert_store_read_through_git, + clone_just_pushed, cloned_just_pushed, git, + insteadof_args, insteadof_entries, insteadof_prefixes, + read_through_git, submodule_update_through_git) @pytest.fixture @@ -230,9 +230,11 @@ def test_the_environment_form_keeps_what_the_environment_holds( """APPENDED, as `appended_to_command_scope` appends the session's own settings: an entry the process already has is neither overwritten nor counted away, so the pasted line sees it as every other git does.""" - monkeypatch.setenv("GIT_CONFIG_COUNT", "1") - monkeypatch.setenv("GIT_CONFIG_KEY_0", "url.held://base/.insteadOf") - monkeypatch.setenv("GIT_CONFIG_VALUE_0", "held://") + held = appended_to_command_scope( + {}, [("url.held://base/.insteadOf", "held://")]) + assert held["GIT_CONFIG_COUNT"] == "1", held + for name, value in held.items(): + monkeypatch.setenv(name, value) entries = read_through_git(remotes) assert "GIT_CONFIG_KEY_0" not in entries, entries assert entries["GIT_CONFIG_COUNT"] == str( diff --git a/tests/test_doctor_legs_members_next.py b/tests/test_doctor_legs_members_next.py index 619034eb..a1f387ff 100644 --- a/tests/test_doctor_legs_members_next.py +++ b/tests/test_doctor_legs_members_next.py @@ -24,10 +24,11 @@ Those tests are `WINDOWS_SKIP`: `sh` is a POSIX shell, and on Windows the reader's shell is PowerShell. The text-level assertions about what the line IS run on every OS. A git paste gets the file protocol per subprocess through -`GIT_CONFIG_COUNT`, `GIT_CONFIG_KEY_0` and `GIT_CONFIG_VALUE_0` -- the -fixtures' remotes are bare repositories on disk, which git refuses as a -submodule URL by default -- and never `GIT_ALLOW_PROTOCOL`, which would -override git's `user` policy for the whole run. +git's `GIT_CONFIG_COUNT` entries -- the fixtures' remotes are bare +repositories on disk, which git refuses as a submodule URL by default -- +appended at the call to the entries the environment holds (`paste_env`), and +never through `GIT_ALLOW_PROTOCOL`, which would override git's `user` policy +for the whole run. WHAT IS ASSERTED IS WHAT THE COMMAND DOES, and the explanation beside it is read through `why_of` alone, so that moving the ` # ` tail into a field of @@ -48,7 +49,8 @@ import pytest -from conftest import (WINDOWS_SKIP, cloned_just_pushed, git, rmtree, +from conftest import (WINDOWS_SKIP, appended_to_command_scope, + cloned_just_pushed, git, rmtree, submodule_update_through_git) from test_shape_doctor import ( # noqa: F401 FAMILY_MEMBER, FAMILY_NAME, GIT_IDENTITY, PROJECT, commit_all, doctor, @@ -56,17 +58,28 @@ scaffolded_here, standard, ) -#: What every pasted git line runs under: the identity a commit needs (this -#: container has none, and neither does a fresh CI runner), the file -#: protocol for ONE process tree, and no `PYTHONPATH` from the person -#: running the suite -- a template line that needed one from the reader is -#: exactly what these tests are not allowed to depend on. -PASTE_ENV = {**{k: v for k, v in os.environ.items() if k != "PYTHONPATH"}, - **GIT_IDENTITY, - "GIT_CONFIG_COUNT": "1", - "GIT_CONFIG_KEY_0": "protocol.file.allow", - "GIT_CONFIG_VALUE_0": "always", - "PYTHONDONTWRITEBYTECODE": "1"} + +def paste_env() -> dict: + """What every pasted git line runs under: the identity a commit needs + (this container has none, and neither does a fresh CI runner), the file + protocol for ONE process tree, and no `PYTHONPATH` from the person + running the suite -- a template line that needed one from the reader is + exactly what these tests are not allowed to depend on. + + BUILT AT THE CALL, from the `os.environ` of that moment, and the file + protocol is APPENDED to the `GIT_CONFIG_COUNT` entries it holds + (`appended_to_command_scope`): the session's own (`no_auto_maintenance` + keeps git's automatic maintenance off, #645) and the two the Windows job + sets. This was a dict built when the module was imported, at collection, + before the session fixture ran, so it held none of those, and its own + `GIT_CONFIG_COUNT` of 1 would have dropped the ones the environment did + hold (#758).""" + return {**{k: v for k, v in os.environ.items() if k != "PYTHONPATH"}, + **GIT_IDENTITY, + **appended_to_command_scope( + os.environ, [("protocol.file.allow", "always")]), + "PYTHONDONTWRITEBYTECODE": "1"} + #: The git line's own words after `git -C `: `--init` for a mount the #: clone never registered, `--recursive` for a member's own legs, and @@ -99,7 +112,7 @@ def paste(row: dict, cwd: Path) -> subprocess.CompletedProcess: cwd.mkdir(exist_ok=True) return subprocess.run(["sh", "-c", row["next"]], cwd=str(cwd), capture_output=True, text=True, encoding="utf-8", - errors="replace", check=False, env=PASTE_ENV, + errors="replace", check=False, env=paste_env(), stdin=subprocess.DEVNULL) @@ -440,7 +453,7 @@ def test_the_keep_line_in_the_explanation_keeps_a_detached_commit( assert keep, why_of(row) kept = subprocess.run(["sh", "-c", keep.group(0).replace("NAME", "kept")], capture_output=True, text=True, check=False, - env=PASTE_ENV) + env=paste_env()) assert kept.returncode == 0, kept.stdout + kept.stderr result = paste(row, tmp_path / "elsewhere") assert result.returncode == 0, result.stdout + result.stderr @@ -512,7 +525,7 @@ def test_a_plain_clones_empty_mounts_are_cured_by_the_pasted_line( `submodule.active`, so every other paste here would pass without it.""" plain = cloned_just_pushed( scaffolded_here["base"] / "remotes" / f"{PROJECT}.git", - tmp_path / "Plain", recurse=False, env=PASTE_ENV) + tmp_path / "Plain", recurse=False, env=paste_env()) row = row_of(standard, plain, "legs") assert row["status"] == "FINDING", row assert command_of(row).endswith("-- spec code"), row["next"] @@ -607,7 +620,7 @@ def test_the_git_line_does_not_cure_a_mount_git_cannot_read_a_head_in( result = subprocess.run( ["git", "-C", str(project), "submodule", "update", "--init", "--recursive", "--", "spec"], capture_output=True, text=True, - check=False, env=PASTE_ENV) + check=False, env=paste_env()) assert result.returncode != 0, result.stdout + result.stderr after = row_of(standard, project, "legs") assert after["status"] == "FINDING", after @@ -783,7 +796,7 @@ def test_a_pasted_members_git_line_keeps_a_detached_commit_on_the_kept_branch( keep = re.search(r"git -C \S+ branch NAME", why_of(row)) assert keep, why_of(row) subprocess.run(["sh", "-c", keep.group(0).replace("NAME", "kept")], - capture_output=True, check=True, env=PASTE_ENV) + capture_output=True, check=True, env=paste_env()) result = paste(row, tmp_path / "elsewhere") assert result.returncode == 0, result.stdout + result.stderr assert row_of(standard, root, "members")["status"] == "ok" @@ -1017,7 +1030,7 @@ def sh(line: str, cwd: Path) -> subprocess.CompletedProcess: cwd.mkdir(exist_ok=True) return subprocess.run(["sh", "-c", line], cwd=str(cwd), capture_output=True, text=True, encoding="utf-8", - errors="replace", check=False, env=PASTE_ENV, + errors="replace", check=False, env=paste_env(), stdin=subprocess.DEVNULL) @@ -1814,6 +1827,15 @@ def test_without_it_the_legs_own_submodule_commit_is_in_no_ref( assert commit in git("reflog", cwd=nested).stdout +def clone_member(first: Path, ghost: Path) -> None: + """`ghost`, a plain clone of the member at `first`, read through git + (`cloned_just_pushed`): its own legs stay empty, as the first member's + are, which a recursive clone would fill from the urls `.gitmodules` + gives.""" + cloned_just_pushed(first, ghost, recurse=False) + assert not any((ghost / "spec").iterdir()), "the clone filled a leg" + + def with_a_second_member_off_its_gitlink(root: Path) -> tuple[Path, str]: """A second member, `Ghost`, a clone of the first, recorded at a commit that is not its pin and detached at another past that one; and the @@ -1825,8 +1847,7 @@ def with_a_second_member_off_its_gitlink(root: Path) -> tuple[Path, str]: ghost = root / "members" / "Ghost" url = git("config", "-f", ".gitmodules", "--get", f"submodule.members/{FAMILY_MEMBER}.url", cwd=root).stdout - git("-c", "protocol.file.allow=always", "clone", "-q", "--no-local", - str(first), str(ghost), cwd=root) + clone_member(first, ghost) add_member_row(root, "Ghost") git("config", "-f", ".gitmodules", "submodule.members/Ghost.path", "members/Ghost", cwd=root) @@ -1926,8 +1947,7 @@ def with_a_second_member(root: Path, at_pin: bool, section: str) -> Path: ghost = root / "members" / "Ghost" url = git("config", "-f", ".gitmodules", "--get", f"submodule.members/{FAMILY_MEMBER}.url", cwd=root).stdout - git("-c", "protocol.file.allow=always", "clone", "-q", "--no-local", - str(first), str(ghost), cwd=root) + clone_member(first, ghost) add_member_row(root, "Ghost") if section != "none": git("config", "-f", ".gitmodules", "submodule.members/Ghost.path", diff --git a/tests/test_doctor_next_is_a_command.py b/tests/test_doctor_next_is_a_command.py index 9dad6ee8..aec22b83 100644 --- a/tests/test_doctor_next_is_a_command.py +++ b/tests/test_doctor_next_is_a_command.py @@ -63,7 +63,7 @@ import pytest -from conftest import REPO, WINDOWS_SKIP, git, rmtree +from conftest import REPO, WINDOWS_SKIP, appended_to_command_scope, git, rmtree #: The lane a lane's run sets, from the file that owns that rule. from test_commit_trailers import LANE #: The doctor's own fixtures and helpers, from the module that owns them. A @@ -834,19 +834,19 @@ def paste_into_zsh(line: str, cwd: Path, timeout: float = 120.0) -> tuple: The environment is the test's own, less `PYTHONPATH` (a NEXT carries what it needs, #238) and with file-protocol git for the one child that may clone from the suite's local remotes -- per process, never - `GIT_ALLOW_PROTOCOL` (see `tests/conftest.py`). It is read from - `os.environ` HERE, at the call, and is not doctor A's `PASTE_ENV`: that - dict is built when its module is imported, at collection, before the - session fixture puts the `gh` guard first on `PATH`, so a line pasted - with it would reach whatever `gh` the machine has. + `GIT_ALLOW_PROTOCOL` (see `tests/conftest.py`), appended to the command + scope the environment holds. It is read from `os.environ` HERE, at the + call, as doctor A's `paste_env()` is (#758): a dict built when its module + is imported, at collection, is made before the session fixture puts the + `gh` guard first on `PATH`, so a line pasted with it would reach + whatever `gh` the machine has. """ import pty # POSIX only: `tty` beneath it needs `termios` env = {name: value for name, value in os.environ.items() if name.upper() != "PYTHONPATH"} env.update({"TERM": "dumb", "PYTHONDONTWRITEBYTECODE": "1", - "GIT_CONFIG_COUNT": "1", - "GIT_CONFIG_KEY_0": "protocol.file.allow", - "GIT_CONFIG_VALUE_0": "always"}) + **appended_to_command_scope( + os.environ, [("protocol.file.allow", "always")])}) master, slave = pty.openpty() try: shell = subprocess.Popen(["zsh", "-f", "-i"], stdin=slave, diff --git a/tests/test_family.py b/tests/test_family.py index 960daf90..2af743ef 100644 --- a/tests/test_family.py +++ b/tests/test_family.py @@ -32,8 +32,8 @@ import pytest -from conftest import (NO_BYTECODE, REPO, cloned_just_pushed, git, rmtree, - run_script) +from conftest import (NO_BYTECODE, REPO, appended_to_command_scope, + cloned_just_pushed, git, rmtree, run_script) #: The trailer helpers and the two lines a lane's run lands, imported from #: the file that owns those rules rather than retyped — see #: `tests/test_commit_trailers.py`. @@ -71,12 +71,15 @@ #: it. Every copy of the holder a test makes stands on this branch. WORK_BRANCH = "family/members" -#: `git` reads one-off configuration out of the environment, which is how a -#: test gives a plain `git submodule update` permission to clone a local path -#: without the tool under test knowing anything about it. -ALLOW_FILE_PROTOCOL = {"GIT_CONFIG_COUNT": "1", - "GIT_CONFIG_KEY_0": "protocol.file.allow", - "GIT_CONFIG_VALUE_0": "always"} + +def allow_file_protocol() -> dict: + """`git` reads one-off configuration out of the environment, which is how + a test gives a plain `git submodule update` permission to clone a local + path without the tool under test knowing anything about it. Built at the + call, and appended to the git command scope the environment holds, so the + session's own settings (#645) still reach the child (#758).""" + return appended_to_command_scope(os.environ, + [("protocol.file.allow", "always")]) def scaffold_member(base, project: str) -> None: @@ -1410,7 +1413,7 @@ def bootstrapped(family, tmp_path_factory): target = cloned_just_pushed(family["remotes"] / f"{NAME}.git", tmp_path_factory.mktemp("family-clone") / NAME) result = run_script(target / "scripts" / "bootstrap.py", cwd=target, - env=ALLOW_FILE_PROTOCOL) + env=allow_file_protocol()) return {"root": target, "result": result} @@ -1791,7 +1794,7 @@ def estate(family, tmp_path): def siblings(holder, *extra, env=None): return run_script(holder / "scripts" / "siblings.py", *extra, cwd=holder, - env={**ALLOW_FILE_PROTOCOL, **(env or {})}) + env={**allow_file_protocol(), **(env or {})}) def layout(stdout: str) -> list[str]: @@ -1815,7 +1818,7 @@ def test_siblings_clones_each_member_beside_the_holder_on_its_branch(estate): """ proc = subprocess.run(["make", "siblings"], cwd=str(estate["holder"]), capture_output=True, text=True, check=False, - env={**os.environ, **ALLOW_FILE_PROTOCOL}) + env={**os.environ, **allow_file_protocol()}) assert proc.returncode == 0, proc.stderr + proc.stdout for project in MEMBERS: sibling = estate["folder"] / project @@ -1975,7 +1978,7 @@ def one(name: str, runner) -> dict: theirs = one("target", lambda holder: siblings(holder)) ours = one("shim", lambda holder: run_script( FAMILY, "siblings", "--family-root", str(holder), - env={**ALLOW_FILE_PROTOCOL, **NO_BYTECODE})) + env={**allow_file_protocol(), **NO_BYTECODE})) assert layout(ours["result"].stdout) == layout(theirs["result"].stdout) for project in MEMBERS: diff --git a/tests/test_family_bump_remove_rollback.py b/tests/test_family_bump_remove_rollback.py index f3fb458a..625881ba 100644 --- a/tests/test_family_bump_remove_rollback.py +++ b/tests/test_family_bump_remove_rollback.py @@ -61,8 +61,8 @@ import pytest -from conftest import (cloned_just_pushed, git, rmtree, run_script, - submodule_update_through_git) +from conftest import (appended_to_command_scope, cloned_just_pushed, git, + rmtree, run_script, submodule_update_through_git) #: The family fixture, its names, the offline environment and the holder's #: byte-for-byte state, imported from the modules that own them rather than #: rebuilt: a holder made, or compared, any other way would test a @@ -677,10 +677,7 @@ def foreign_store(store: Path, url: str, tmp_path: Path) -> None: def configured(env: dict, key: str, value: str) -> dict: """`env` with one more entry in git's `GIT_CONFIG_COUNT` protocol.""" - count = int(env.get("GIT_CONFIG_COUNT", "0")) - return {**env, "GIT_CONFIG_COUNT": str(count + 1), - f"GIT_CONFIG_KEY_{count}": key, - f"GIT_CONFIG_VALUE_{count}": value} + return {**env, **appended_to_command_scope(env, [(key, value)])} @pytest.mark.parametrize("store", ["absent", "without the commit"]) diff --git a/tests/test_family_bump_to_checks.py b/tests/test_family_bump_to_checks.py index e096c511..d8b7652f 100644 --- a/tests/test_family_bump_to_checks.py +++ b/tests/test_family_bump_to_checks.py @@ -40,6 +40,7 @@ import pytest +from conftest import cloned_just_pushed #: The family fixture and its names, from the module that owns them. from test_family import ( # noqa: F401 NAME, ORG, family, manifest, validate, @@ -232,8 +233,7 @@ def test_bump_to_a_commit_not_pushed_says_to_push_it_first( env = isolated(offline, tmp_path) remote = a_private_remote(holder, env, tmp_path) work = tmp_path / f"{MEMBER}-work" - raw(tmp_path, env, "clone", "-q", "--no-local", "-b", "main", "--", - str(remote), str(work)) + cloned_just_pushed(remote, work, "main", recurse=False, env=env) (work / "LOCAL.md").write_bytes(b"made here, not pushed yet\n") raw(work, env, "add", "--", "LOCAL.md") raw(work, env, "commit", "-q", "-m", "A commit only the clone has") diff --git a/tests/test_family_dry_run.py b/tests/test_family_dry_run.py index b024492e..970fe23a 100644 --- a/tests/test_family_dry_run.py +++ b/tests/test_family_dry_run.py @@ -49,7 +49,8 @@ import pytest -from conftest import cloned_just_pushed, git, run_script +from conftest import (appended_to_command_scope, cloned_just_pushed, git, + run_script) #: The trailer lines a lane's run lands, from the file that owns them. from test_commit_trailers import TRAILERS, trailers_of #: The family fixture, its names and the offline environment, imported from @@ -71,25 +72,34 @@ ) from test_family_member_path import isolated as member_path_env -#: GIT'S AUTOMATIC MAINTENANCE, OFF for every git this module starts, through -#: git's `GIT_CONFIG_COUNT` protocol and never a global file. A `git commit` -#: may start `git maintenance run --auto` in the background (a newer git on -#: the macOS runner does), and it took `.git/objects/maintenance.lock` while -#: a test snapshotted the holder and released it before the comparison: five -#: `objects changed` failures on PR #345's first macOS run, none of them the -#: tool's. With it off, no background job packs or locks objects mid-test. -NO_MAINTENANCE = {"GIT_CONFIG_COUNT": "3", - "GIT_CONFIG_KEY_0": "maintenance.auto", - "GIT_CONFIG_VALUE_0": "false", - "GIT_CONFIG_KEY_1": "gc.auto", "GIT_CONFIG_VALUE_1": "0", - "GIT_CONFIG_KEY_2": "gc.autoDetach", - "GIT_CONFIG_VALUE_2": "false"} +#: GIT'S AUTOMATIC MAINTENANCE, OFF for the gits `isolated()` below hands an +#: environment to: `family.py`'s children and the snapshot's gits. A `git +#: commit` may start `git maintenance run --auto` in the background (a newer +#: git on the macOS runner does), and it took `.git/objects/maintenance.lock` +#: while a test snapshotted the holder and released it before the +#: comparison: five `objects changed` failures on PR #345's first macOS run, +#: none of them the tool's. With it off, no background job packs or locks +#: objects mid-test. +#: +#: These are the `(key, value)` entries `isolated()` appends to git's command +#: scope (`GIT_CONFIG_COUNT`) at the call, after the ones the environment +#: holds. `isolated()` also names a global config of its own +#: (`member_path_env`'s `excluding()`), which takes the session's file away +#: from these gits, so the command scope is what keeps maintenance off for +#: them. The module's own setup commits go through `conftest.git`, which +#: passes no `env=` and inherits `os.environ`: the session fixture +#: (`no_auto_maintenance`, #645) keeps maintenance off for those, in both +#: scopes, and this constant does not reach them. +NO_MAINTENANCE = (("maintenance.auto", "false"), ("gc.auto", "0"), + ("gc.autoDetach", "false")) def isolated(offline, tmp_path: Path) -> dict: """`test_family_member_path.isolated`'s offline, identity-carrying - environment, with git's automatic maintenance off (`NO_MAINTENANCE`).""" - return {**member_path_env(offline, tmp_path), **NO_MAINTENANCE} + environment, with git's automatic maintenance off (`NO_MAINTENANCE`), + appended to the command scope `os.environ` holds when this is called.""" + return {**member_path_env(offline, tmp_path), + **appended_to_command_scope(os.environ, NO_MAINTENANCE)} #: The header `bump --dry-run` prints above the row it would write. diff --git a/tests/test_family_siblings_placement.py b/tests/test_family_siblings_placement.py index c5d6e449..a34f98d5 100644 --- a/tests/test_family_siblings_placement.py +++ b/tests/test_family_siblings_placement.py @@ -53,7 +53,7 @@ #: The family fixture, its names and the `make siblings` runner, imported #: from the module that owns them. from test_family import ( # noqa: F401 - ALLOW_FILE_PROTOCOL, FAMILY, MEMBERS, NAME, family, siblings, + FAMILY, MEMBERS, NAME, allow_file_protocol, family, siblings, ) #: The branch IRSS's remote is pointed at instead of `main`. @@ -514,7 +514,7 @@ def test_a_family_folder_reached_through_a_link_passes_and_clones_into_it( logical = link / NAME shim = run_script(FAMILY, "siblings", "--family-root", str(logical), - "--dry-run", env=ALLOW_FILE_PROTOCOL) + "--dry-run", env=allow_file_protocol()) assert shim.returncode == 0, shim.stdout + shim.stderr assert "WARNING" not in shim.stderr @@ -615,7 +615,7 @@ def test_family_py_siblings_runs_the_holders_own_copy_with_its_imports( b" Path(_shape.__file__).resolve())\n"))) result = run_script(FAMILY, "siblings", "--family-root", str(holder), - "--dry-run", env=ALLOW_FILE_PROTOCOL) + "--dry-run", env=allow_file_protocol()) assert result.returncode == 0, result.stdout + result.stderr library = (holder / "scripts" / "repo_shape.py").resolve() assert f"HOLDER COPY RAN, repo_shape from {library}" in result.stdout @@ -630,7 +630,7 @@ def test_family_py_siblings_refuses_a_holder_without_its_own_copy( (holder / "scripts" / "siblings.py").unlink() result = run_script(FAMILY, "siblings", "--family-root", str(holder), - env=ALLOW_FILE_PROTOCOL) + env=allow_file_protocol()) assert result.returncode == 2, result.stdout + result.stderr said = flat(result.stderr) assert said.startswith("REFUSED siblings-script-missing: family.py " @@ -659,7 +659,7 @@ def test_family_py_siblings_escapes_what_its_stdout_cannot_encode( of the first line, and the run goes on.""" holder = holder_in(diverged, tmp_path / NAME) result = run_script(FAMILY, "siblings", "--family-root", str(holder), - "--dry-run", env={**ALLOW_FILE_PROTOCOL, + "--dry-run", env={**allow_file_protocol(), "PYTHONIOENCODING": "ascii"}) assert result.returncode == 0, result.stdout + result.stderr assert "UnicodeEncodeError" not in result.stderr diff --git a/tests/test_no_auto_maintenance.py b/tests/test_no_auto_maintenance.py index c7116dd0..4b20730e 100644 --- a/tests/test_no_auto_maintenance.py +++ b/tests/test_no_auto_maintenance.py @@ -22,11 +22,16 @@ person's own global config is still read, in git's order, beneath them; - the file is one git can read where `HOME` is unset, and where `GIT_CONFIG_GLOBAL` is empty, which is git's "no global config"; -- the fixture is in place before any session fixture builds a repository. +- the fixture is in place before any session fixture builds a repository; +- a line pasted through the doctor tests' `paste()` runs under the entries the + environment holds when it is pasted, with `protocol.file.allow` appended + (#758), and no test file sets `GIT_CONFIG_COUNT` to a number of its own, + which would drop the entries the session and the Windows job set. """ from __future__ import annotations +import ast import json import os import subprocess @@ -35,8 +40,9 @@ import pytest from conftest import (NO_AUTO_MAINTENANCE, ORG, PROJECT, SCAFFOLD, - appended_to_command_scope, git, home_is_known, - run_script, session_global_config) + WINDOWS_SKIP, appended_to_command_scope, git, + home_is_known, run_script, session_global_config) +from test_doctor_legs_members_next import paste #: The verbs auto maintenance runs as: `git maintenance run --auto`, and #: `git gc --auto`, which a git started itself before `maintenance` existed. @@ -281,3 +287,173 @@ def test_the_settings_are_in_place_before_a_session_fixture_builds_a_repo( for name in ("GIT_CONFIG_GLOBAL", "GIT_CONFIG_COUNT")} assert seen_by_a_session_fixture["GIT_CONFIG_GLOBAL"] == \ str(no_auto_maintenance) + + +# --- the doctor tests' pasted lines, and the counts a test may not set (#758) +# +# `test_doctor_legs_members_next.py` pastes the NEXT a row prints into `sh`. +# Its environment was a copy of `os.environ` made when the module was +# imported, at collection, with `GIT_CONFIG_COUNT` set to 1: it held none of +# the session's settings, which `no_auto_maintenance` puts there at session +# start, and it dropped the entries the Windows job sets. + +@WINDOWS_SKIP +def test_a_pasted_line_keeps_the_entries_the_environment_holds_at_the_call( + no_auto_maintenance, tmp_path, monkeypatch): + """The environment holds what the Windows job sets (two entries) and what + the session fixture appends (the two settings), put there AFTER the doctor + tests were imported. `git config --show-origin --list`, pasted through + `paste()`, lists all four in git's command scope, in order, and then the + `protocol.file.allow` the paste adds for its own sake, and reads the two + settings from the session's global file too. On the base it listed that + one alone, and no global file: the import-time copy held none of it.""" + held = appended_to_command_scope( + {}, [("test.heldone", "one"), ("test.heldtwo", "two")]) + for name, value in {**held, **appended_to_command_scope( + held, NO_AUTO_MAINTENANCE)}.items(): + monkeypatch.setenv(name, value) + shown = paste({"next": "git config --show-origin --list"}, + tmp_path / "elsewhere") + assert shown.returncode == 0, shown.stderr + shown.stdout + from_the_command_line = [ + line.split("\t", 1)[1] for line in shown.stdout.splitlines() + if line.startswith("command line:")] + assert from_the_command_line == [ + "test.heldone=one", "test.heldtwo=two", + *(f"{key}={value}" for key, value in NO_AUTO_MAINTENANCE), + "protocol.file.allow=always"] + for key, value in NO_AUTO_MAINTENANCE: + assert f"file:{no_auto_maintenance}\t{key}={value}" in \ + shown.stdout.splitlines() + + +#: The test files (every `.py` beside this one but `conftest.py`, whose +#: `appended_to_command_scope` is where a count is worked out) that may set a +#: count of their own, and how many times. Each is a test that BUILDS an +#: environment holding entries, to see that the code under test keeps them: +#: this file's test of `appended_to_command_scope` writes its two out, and +#: the test above starts from none; `test_adopt_git_reads.py` and +#: `test_clone_just_pushed.py` do the same for `command_scope` and for +#: `read_through_git`. +OWN_COUNTS = {"test_no_auto_maintenance.py": 2, + "test_adopt_git_reads.py": 1, "test_clone_just_pushed.py": 1} + +COUNT = "GIT_CONFIG_COUNT" +APPENDER = "appended_to_command_scope" +#: The calls that put a value in the environment: `monkeypatch.setenv(name, +#: value)`, `env.setdefault(name, value)` and `os.putenv(name, value)`. Not +#: `env.get(name, default)`, which reads. +SETTERS = ("setenv", "setdefault", "putenv") + + +def is_the_count(node) -> bool: + """The string constant `"GIT_CONFIG_COUNT"`.""" + return isinstance(node, ast.Constant) and node.value == COUNT + + +def is_a_number(node) -> bool: + """A string constant of digits alone: `"1"`.""" + return (isinstance(node, ast.Constant) and isinstance(node.value, str) + and node.value.isdigit()) + + +def is_a_setter(node) -> bool: + """`setenv`, `setdefault` or `putenv`, called bare or on an object.""" + return (isinstance(node, ast.Name) and node.id in SETTERS) or ( + isinstance(node, ast.Attribute) and node.attr in SETTERS) + + +def is_the_appender(node) -> bool: + """`appended_to_command_scope`, called bare or through a module.""" + return (isinstance(node, ast.Name) and node.id == APPENDER) or ( + isinstance(node, ast.Attribute) and node.attr == APPENDER) + + +def counts_set_to_a_number(source: bytes) -> list: + """The line of each place `source` sets a git command scope of its own + instead of appending to the environment's: + + - `GIT_CONFIG_COUNT` set to a number it writes out: in a dict + (`{"GIT_CONFIG_COUNT": "1"}`), as a keyword + (`dict(GIT_CONFIG_COUNT="1")`), in a call that names the variable and + gives its value (`monkeypatch.setenv("GIT_CONFIG_COUNT", "1")`), or by + subscript (`env["GIT_CONFIG_COUNT"] = "1"`); + - `appended_to_command_scope` handed a dict written out (`{}`), where the + environment goes: it counts from nothing, and the entries the + environment holds are no longer in the count it returns. + + A count worked out from the one the environment holds is not one of + them.""" + lines = [] + for node in ast.walk(ast.parse(source)): + if isinstance(node, ast.Dict): + lines += [key.lineno for key, value in zip(node.keys, node.values) + if is_the_count(key) and is_a_number(value)] + elif isinstance(node, ast.Call): + lines += [keyword.value.lineno for keyword in node.keywords + if keyword.arg == COUNT and is_a_number(keyword.value)] + if (is_a_setter(node.func) and len(node.args) >= 2 + and is_the_count(node.args[0]) + and is_a_number(node.args[1])): + lines.append(node.lineno) + if (is_the_appender(node.func) and node.args + and isinstance(node.args[0], ast.Dict)): + lines.append(node.lineno) + elif isinstance(node, ast.Assign) and is_a_number(node.value): + lines += [target.lineno for target in node.targets + if isinstance(target, ast.Subscript) + and is_the_count(target.slice)] + return sorted(lines) + + +@pytest.mark.parametrize("source", [ + '{"GIT_CONFIG_COUNT": "1", "GIT_CONFIG_KEY_0": "protocol.file.allow"}', + 'dict(GIT_CONFIG_COUNT="2")', + 'monkeypatch.setenv("GIT_CONFIG_COUNT", "1")', + 'env.setdefault("GIT_CONFIG_COUNT", "2")', + 'env["GIT_CONFIG_COUNT"] = "3"', + 'appended_to_command_scope({}, [("k", "v")])', + 'conftest.appended_to_command_scope({"A": "1"}, [])', +], ids=["dict", "keyword", "setenv", "setdefault", "subscript", + "appended-to-nothing", "through-a-module"]) +def test_the_scan_sees_each_way_a_test_can_set_a_count(source): + assert counts_set_to_a_number(source.encode("utf-8")) == [1] + + +@pytest.mark.parametrize("source", [ + '{"GIT_CONFIG_COUNT": str(held + 1)}', + '{**held, "GIT_CONFIG_KEY_0": "k"}', + 'monkeypatch.setenv("GIT_CONFIG_COUNT", str(held))', + 'index = int(env.get("GIT_CONFIG_COUNT", "0"))', + 'env["GIT_CONFIG_COUNT"] = count', + '"GIT_CONFIG_COUNT is 1"', + 'appended_to_command_scope(os.environ, [("k", "v")])', + 'appended_to_command_scope(held, [("k", "v")])', +], ids=["worked-out", "a-splat", "a-call", "a-read-with-a-default", "a-name", + "a-sentence", "appended-to-the-environment", "appended-to-a-name"]) +def test_the_scan_leaves_a_count_that_is_worked_out(source): + assert counts_set_to_a_number(source.encode("utf-8")) == [] + + +def test_no_test_sets_a_git_config_count_of_its_own(): + """A `GIT_CONFIG_COUNT` of 1, laid over `os.environ`, replaces the one the + session fixture and the Windows job set: the entries it counted are no + longer git's, so a session setting or a job's `core.autocrlf` does not + reach the git the test starts. Every test gives git a setting by + appending to the entries the environment holds, built at the call by + `appended_to_command_scope` (`conftest.py`). Before #758 `PASTE_ENV`, + `ALLOW_FILE_PROTOCOL`, `NO_MAINTENANCE`, `QUOTE_PATH_OFF`, `SYMLINKS_OFF` + and eight one-off environments each wrote one.""" + found = {} + for path in sorted(Path(__file__).resolve().parent.glob("*.py")): + if path.name == "conftest.py": + continue + lines = counts_set_to_a_number(path.read_bytes()) + if len(lines) != OWN_COUNTS.get(path.name, 0): + found[path.name] = lines + assert not found, ( + "a `GIT_CONFIG_COUNT` written as a number, or `appended_to_command_" + "scope` handed a dict written out, replaces the entries the " + "environment holds; build it with `appended_to_command_scope` and " + "`os.environ` instead, or, for a test that builds an environment " + f"on purpose, change `OWN_COUNTS`: {found}") diff --git a/tests/test_non_utf8_git_reads.py b/tests/test_non_utf8_git_reads.py index c79bada2..110e2f87 100644 --- a/tests/test_non_utf8_git_reads.py +++ b/tests/test_non_utf8_git_reads.py @@ -49,7 +49,8 @@ import pytest -from conftest import ORG, PROJECT, REPO, cloned_just_pushed, git, run_script +from conftest import (ORG, PROJECT, REPO, appended_to_command_scope, + cloned_just_pushed, git, run_script) from test_bump_leg import branched, bump sys.path.insert(0, str(REPO / "scripts")) @@ -77,10 +78,11 @@ UTF8 = "caf\u00e9.yml" #: `core.quotePath=false`, as command-line config: it outranks every config -#: file, and a test needs no config file of its own to say it. -QUOTE_PATH_OFF = {"GIT_CONFIG_COUNT": "1", - "GIT_CONFIG_KEY_0": "core.quotePath", - "GIT_CONFIG_VALUE_0": "false"} +#: file, and a test needs no config file of its own to say it. The `(key, +#: value)` entries `tool_env` appends to the ones the environment holds, +#: when the test runs: the environment of a session is not the one of +#: collection. +QUOTE_PATH_OFF = (("core.quotePath", "false"),) def make_file(path: Path, data: bytes = b"on: [push]\n") -> None: @@ -293,11 +295,14 @@ def leave(name: str) -> Path: assert git_bytes("status", "--porcelain", "-z", cwd=standard) == b"" -def tool_env(**extra: str) -> dict: +def tool_env(entries=(), **extra: str) -> dict: """The environment a tool under test runs in: no bytecode written into the copy (an untracked `__pycache__` would be dirt, though the standard's - own `.gitignore` names it), and `extra` on top.""" - return {"PYTHONDONTWRITEBYTECODE": "1", **extra} + own `.gitignore` names it), `entries` appended to the git command scope + the environment holds (`appended_to_command_scope`), and `extra` on + top.""" + scope = appended_to_command_scope(os.environ, entries) if entries else {} + return {"PYTHONDONTWRITEBYTECODE": "1", **scope, **extra} def revision_of(standard: Path) -> str: @@ -339,7 +344,7 @@ def init_from(standard: Path, tmp_path, env: dict, *, dry_run: bool): #: not UTF-8; and its two controls, which passed on main. DIRTY_ROWS = [ pytest.param(NOT_UTF8, QUOTE_PATH_OFF, id="not-utf8-quote-path-false"), - pytest.param(NOT_UTF8, {}, id="not-utf8-default-quoting"), + pytest.param(NOT_UTF8, (), id="not-utf8-default-quoting"), pytest.param(UTF8, QUOTE_PATH_OFF, id="utf8-quote-path-false"), ] @@ -357,7 +362,7 @@ def test_a_dirty_standard_is_warned_about_whatever_its_file_is_named( the revision the copy is at, and goes on: exit 0, and a real run makes the remotes, which the traceback never reached.""" untracked(name) - result = run(standard, tmp_path, tool_env(**config), dry_run=dry_run) + result = run(standard, tmp_path, tool_env(config), dry_run=dry_run) assert result.returncode == 0, result.stderr + result.stdout assert f"{warning}{revision_of(standard)[:12]}" in result.stderr, ( result.stderr) @@ -367,7 +372,7 @@ def test_a_dirty_standard_is_warned_about_whatever_its_file_is_named( "a dry run creates no remote and a real run does") -@pytest.mark.parametrize("config", [QUOTE_PATH_OFF, {}], +@pytest.mark.parametrize("config", [QUOTE_PATH_OFF, ()], ids=["quote-path-false", "default-quoting"]) @pytest.mark.parametrize("run, warning", TOOLS) def test_a_clean_standard_prints_no_warning(standard, tmp_path, run, warning, @@ -375,7 +380,7 @@ def test_a_clean_standard_prints_no_warning(standard, tmp_path, run, warning, """THE LAST ROW: the standard's checkout is clean, `core.quotePath=false`, and the run is exit 0 with no warning. `bool(...)` of git's bytes must still be false for an empty answer.""" - result = run(standard, tmp_path, tool_env(**config), dry_run=True) + result = run(standard, tmp_path, tool_env(config), dry_run=True) assert result.returncode == 0, result.stderr + result.stdout assert "DIRTY" not in result.stderr, result.stderr @@ -420,8 +425,8 @@ def user_name(value: str, **extra: str) -> dict: `_creator_name` passes over (and `env_commit` replaces with its own fallback, where a name of whitespace alone would reach git as it is and be refused by `git commit`).""" - return {"GIT_AUTHOR_NAME": "", "GIT_CONFIG_COUNT": "1", - "GIT_CONFIG_KEY_0": "user.name", "GIT_CONFIG_VALUE_0": value, + return {"GIT_AUTHOR_NAME": "", + **appended_to_command_scope(os.environ, [("user.name", value)]), **extra} diff --git a/tests/test_scaffold_free_text_values.py b/tests/test_scaffold_free_text_values.py index 0d8c1449..53be4cd2 100644 --- a/tests/test_scaffold_free_text_values.py +++ b/tests/test_scaffold_free_text_values.py @@ -46,8 +46,8 @@ import pytest -from conftest import (ORG, REPO, SCAFFOLD, blank_unnamed_pin_sources, - run_script) +from conftest import (ORG, REPO, SCAFFOLD, appended_to_command_scope, + blank_unnamed_pin_sources, run_script) sys.path.insert(0, str(REPO / "scripts")) from repo_shape import load_yaml # noqa: E402 @@ -219,9 +219,9 @@ def user_name(monkeypatch, value: str) -> None: (`GIT_CONFIG_COUNT`, git 2.31), which no config file overrides, and `GIT_AUTHOR_NAME` is blank so the elector falls back that far.""" monkeypatch.setenv("GIT_AUTHOR_NAME", " ") - monkeypatch.setenv("GIT_CONFIG_COUNT", "1") - monkeypatch.setenv("GIT_CONFIG_KEY_0", "user.name") - monkeypatch.setenv("GIT_CONFIG_VALUE_0", value) + for name, entry in appended_to_command_scope( + os.environ, [("user.name", value)]).items(): + monkeypatch.setenv(name, entry) CONFIG_VALUES = [ diff --git a/tests/test_scaffold_reuse_seeded_legs.py b/tests/test_scaffold_reuse_seeded_legs.py index 4f64df28..a9eb541f 100644 --- a/tests/test_scaffold_reuse_seeded_legs.py +++ b/tests/test_scaffold_reuse_seeded_legs.py @@ -41,8 +41,9 @@ import pytest -from conftest import (ORG, PROJECT, PTY_SKIP, SCAFFOLD, WINDOWS_SKIP, git, - rmtree, run_in_a_terminal, run_script) +from conftest import (ORG, PROJECT, PTY_SKIP, SCAFFOLD, WINDOWS_SKIP, + appended_to_command_scope, git, rmtree, + run_in_a_terminal, run_script) from test_ruleset_hint_tracking_branch import refusing, template # noqa: F401 LEGS = ("spec", "code") @@ -568,11 +569,13 @@ def git_reads_github_from(base: str) -> dict: """What makes git read `https://github.com//` at `base` instead, with no credential helper and no askpass program, so git answers a leg as it would with no login of its own. The prefix is longer than an - operator's own `https://github.com/` rewrite, so it wins over one.""" - return {"GIT_CONFIG_COUNT": "2", - "GIT_CONFIG_KEY_0": f"url.{base}.insteadOf", - "GIT_CONFIG_VALUE_0": f"https://github.com/{ORG}/", - "GIT_CONFIG_KEY_1": "credential.helper", "GIT_CONFIG_VALUE_1": "", + operator's own `https://github.com/` rewrite, so it wins over one. The two + entries are appended, when this is called, to the command scope the + environment holds.""" + return {**appended_to_command_scope( + os.environ, [(f"url.{base}.insteadOf", + f"https://github.com/{ORG}/"), + ("credential.helper", "")]), "GIT_ASKPASS": ""} diff --git a/tests/test_setup_project_py.py b/tests/test_setup_project_py.py index d9b2b87c..fd7d0d7e 100644 --- a/tests/test_setup_project_py.py +++ b/tests/test_setup_project_py.py @@ -34,8 +34,9 @@ import pytest -from conftest import (FILE_PROTOCOL, REPO, assert_read_through_git, - assert_store_read_through_git, git, rmtree) +from conftest import (FILE_PROTOCOL, REPO, appended_to_command_scope, + assert_read_through_git, assert_store_read_through_git, + git, rmtree) from conftest import assert_refuses_for_want_of_a_terminal @@ -1021,7 +1022,8 @@ def test_an_empty_openreposhape_repo_means_unset(tmp_path): test) was found still claiming `checked_value` refuses "an empty value" for this variable, which is never reached for one. - Offline: `GIT_CONFIG_COUNT`/`_KEY_0`/`_VALUE_0` hand this one process a + Offline: git's `GIT_CONFIG_*` entries, appended to the ones the + environment holds, hand this one process a `url..insteadOf ` rewrite, so `git clone` resolves the DEFAULT repository to THIS checkout without a network - which is what makes the echoed clone line naming the default @@ -1045,9 +1047,9 @@ def test_an_empty_openreposhape_repo_means_unset(tmp_path): cwd=outside, script=outside / "setup-project.py", extra_env={ "OPENREPOSHAPE_REPO": "", - "GIT_CONFIG_COUNT": "1", - "GIT_CONFIG_KEY_0": "url.%s.insteadOf" % REPO.as_uri(), - "GIT_CONFIG_VALUE_0": default_repo, + **appended_to_command_scope(os.environ, [ + ("url.%s.insteadOf" % REPO.as_uri(), + default_repo)]), }) assert result.returncode == 0, result.stderr + result.stdout match = re.search(r"^ git clone .*$", result.stdout, re.MULTILINE) diff --git a/tests/test_setup_sh.py b/tests/test_setup_sh.py index 3778c80d..6dd6b031 100644 --- a/tests/test_setup_sh.py +++ b/tests/test_setup_sh.py @@ -22,7 +22,8 @@ from pathlib import Path -from conftest import REPO, WINDOWS_SKIP, assert_read_through_git, git +from conftest import (REPO, WINDOWS_SKIP, appended_to_command_scope, + assert_read_through_git, git) from conftest import assert_refuses_for_want_of_a_terminal @@ -651,7 +652,8 @@ def test_an_empty_openreposhape_repo_means_unset(tmp_path): alongside this test) was found still claiming the empty case was checked and refused rather than defaulted. - Offline: `GIT_CONFIG_COUNT`/`_KEY_0`/`_VALUE_0` hand this one process a + Offline: git's `GIT_CONFIG_*` entries, appended to the ones the + environment holds, hand this one process a `url..insteadOf ` rewrite, so `git clone` resolves the DEFAULT repository to THIS checkout without a network - which is what makes the echoed clone line naming the default @@ -671,9 +673,9 @@ def test_an_empty_openreposhape_repo_means_unset(tmp_path): cwd=outside, extra_env={ "OPENREPOSHAPE_REPO": "", - "GIT_CONFIG_COUNT": "1", - "GIT_CONFIG_KEY_0": "url.%s.insteadOf" % REPO.as_uri(), - "GIT_CONFIG_VALUE_0": default_repo, + **appended_to_command_scope(os.environ, [ + ("url.%s.insteadOf" % REPO.as_uri(), + default_repo)]), }) assert result.returncode == 0, result.stderr + result.stdout match = re.search(r"^ git clone .*$", result.stdout, re.MULTILINE) diff --git a/tests/test_siblings_family_name.py b/tests/test_siblings_family_name.py index d9cd697c..8f984e10 100644 --- a/tests/test_siblings_family_name.py +++ b/tests/test_siblings_family_name.py @@ -52,7 +52,7 @@ run, ) from test_family import ( - ALLOW_FILE_PROTOCOL, FAMILY, MEMBERS, ORG, WORK_BRANCH, scaffold_member, + FAMILY, MEMBERS, ORG, WORK_BRANCH, allow_file_protocol, scaffold_member, ) from test_stdout_encoding import child_environ, run_tool_bytes, shown @@ -96,7 +96,7 @@ def sib(holder: Path, *args: str, tmp_path: Path, standing in the holder, its stdout in `encoding`.""" return run(holder / "scripts" / "siblings.py", *args, encoding=encoding, tmp_path=tmp_path, cwd=holder, - changes={**ALLOW_FILE_PROTOCOL, **NO_CREDENTIAL, + changes={**allow_file_protocol(), **NO_CREDENTIAL, **(changes or {})}) @@ -665,7 +665,7 @@ def test_make_siblings_clones_from_the_readmes_layout(quill, tmp_path): proc = subprocess.run( [MAKE, "siblings"], cwd=str(holder), env=child_environ(tmp_path, "utf-8:strict", - {**ALLOW_FILE_PROTOCOL, **NO_CREDENTIAL}), + {**allow_file_protocol(), **NO_CREDENTIAL}), capture_output=True, stdin=subprocess.DEVNULL, check=False) out, err = (text.decode("utf-8", "replace") for text in (proc.stdout, proc.stderr)) From eaa72d9e49a25a5c71dcff3324bf2d2a5abd1cca Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Sat, 17 Oct 2026 17:44:49 +0000 Subject: [PATCH 8/8] tests/test_no_auto_maintenance.py and two other files: the scan sees a count whose key a name holds, an empty count and `conftest.py`, with one reader per node, and `configure` appends through `appended_to_command_scope`, so `Closes #758` leaves none of its seven appends built by hand (#758) The fix round for lane 2's rv818 review, after the retarget merge. - SHOULD 2: `test_adopt_reencoded_files.py`'s `configure`, the seventh append of #758's line 2, built its count by hand. It now sets what `appended_to_command_scope(os.environ, pairs)` returns. Same entries, same order. - SHOULD 3: the scan saw only the string `"GIT_CONFIG_COUNT"`. It now also sees a name the module binds to that string (`CONFIG_COUNT = "GIT_CONFIG_COUNT"`, annotated or not), as a dict key, a setter's first argument or a subscript. That was the base's own spelling in `test_adopt_root_symlinks.py`. Appended there, the scan now names line 2137; before this commit it passed. - SHOULD 4: `counts_set_to_a_number` (about 17 on S3776 by hand) is split: `in_a_dict`, `in_a_call` and `by_subscript`, one per node type, picked from `READERS` by the walk. - NIT 5: `conftest.py` is no longer skipped. It scans clean, and a count set there is now caught. - NIT 6: new "sees" cases for `os.putenv` and a bare `setenv`. The walk is `files_off_their_count`, and a new test runs it over a folder of its own: `conftest.py` and a helper module are read, a `.txt` is not, and a file `OWN_COUNTS` allows is named for fewer counts as well as more. - NIT 7, in part: an empty count (`""`) is a count of its own. Git 2.43 and 2.47.1 read it as no entries at all (measured). The docstring says the scan reads the count's value as a constant only. - NIT 8: the paste case deletes `GIT_CONFIG_PARAMETERS` first. With `'probe.param'='p'` in it, the case failed: git listed that entry after the count's. - NIT 10: the comment on `NO_MAINTENANCE` says its `maintenance.auto` and `gc.auto` repeat the session's two, which reach these gits in the command scope, and that only `gc.autoDetach` adds a setting. Lane: openreposhape-5 (openRepoShape-5) Co-Authored-By: Claude Opus 5.5 --- tests/test_adopt_reencoded_files.py | 15 ++- tests/test_family_dry_run.py | 9 +- tests/test_no_auto_maintenance.py | 179 ++++++++++++++++++++-------- 3 files changed, 144 insertions(+), 59 deletions(-) diff --git a/tests/test_adopt_reencoded_files.py b/tests/test_adopt_reencoded_files.py index 63bf4920..c363b052 100644 --- a/tests/test_adopt_reencoded_files.py +++ b/tests/test_adopt_reencoded_files.py @@ -39,8 +39,9 @@ import pytest -from conftest import (ADOPT, findings, git, make_source_repo, planned, - refused_before_any_leg, run_script) +from conftest import (ADOPT, appended_to_command_scope, findings, git, + make_source_repo, planned, refused_before_any_leg, + run_script) from test_adopt_nested_attributes import (EMPTY_TREE, GITHUB_ATTRIBUTES, REENCODED, ROOT, ROOT_ATTRIBUTES, SOURCE, adopted_by_execute, checked, @@ -101,12 +102,10 @@ def raw_plan(base: Path, files: dict, answers: dict) -> Path: def configure(monkeypatch: pytest.MonkeyPatch, pairs) -> None: """Each `(key, value)` of `pairs` in git's command scope, after what - the environment holds, for this process and every git it starts.""" - held = int(os.environ.get("GIT_CONFIG_COUNT") or 0) - for n, (key, value) in enumerate(pairs, held): - monkeypatch.setenv(f"GIT_CONFIG_KEY_{n}", key) - monkeypatch.setenv(f"GIT_CONFIG_VALUE_{n}", value) - monkeypatch.setenv("GIT_CONFIG_COUNT", str(held + len(pairs))) + the environment holds, for this process and every git it starts: + appended by `appended_to_command_scope` (#758).""" + for name, value in appended_to_command_scope(os.environ, pairs).items(): + monkeypatch.setenv(name, value) @pytest.fixture(scope="module") diff --git a/tests/test_family_dry_run.py b/tests/test_family_dry_run.py index f84b64a2..8d2662b6 100644 --- a/tests/test_family_dry_run.py +++ b/tests/test_family_dry_run.py @@ -85,9 +85,12 @@ #: scope (`GIT_CONFIG_COUNT`) at the call, after the ones the environment #: holds. `isolated()` also names a global config of its own #: (`member_path_env`'s `excluding()`), which takes the session's file away -#: from these gits, so the command scope is what keeps maintenance off for -#: them. The module's own setup commits go through `conftest.git`, which -#: passes no `env=` and inherits `os.environ`: the session fixture +#: from these gits. The session's two settings still reach them, in the +#: command scope `isolated()` appends to, so `maintenance.auto` and `gc.auto` +#: here repeat them and only `gc.autoDetach` adds a setting; the two stay so +#: that this constant alone keeps maintenance off for these gits. The +#: module's own setup commits go through `conftest.git`, which passes no +#: `env=` and inherits `os.environ`: the session fixture #: (`no_auto_maintenance`, #645) keeps maintenance off for those, in both #: scopes, and this constant does not reach them. NO_MAINTENANCE = (("maintenance.auto", "false"), ("gc.auto", "0"), diff --git a/tests/test_no_auto_maintenance.py b/tests/test_no_auto_maintenance.py index 4b20730e..5e4f5cce 100644 --- a/tests/test_no_auto_maintenance.py +++ b/tests/test_no_auto_maintenance.py @@ -306,7 +306,10 @@ def test_a_pasted_line_keeps_the_entries_the_environment_holds_at_the_call( `paste()`, lists all four in git's command scope, in order, and then the `protocol.file.allow` the paste adds for its own sake, and reads the two settings from the session's global file too. On the base it listed that - one alone, and no global file: the import-time copy held none of it.""" + one alone, and no global file: the import-time copy held none of it. + `GIT_CONFIG_PARAMETERS` is taken away first: git lists its entries + after the count's, and a run started under `git -c` holds some.""" + monkeypatch.delenv("GIT_CONFIG_PARAMETERS", raising=False) held = appended_to_command_scope( {}, [("test.heldone", "one"), ("test.heldtwo", "two")]) for name, value in {**held, **appended_to_command_scope( @@ -327,14 +330,13 @@ def test_a_pasted_line_keeps_the_entries_the_environment_holds_at_the_call( shown.stdout.splitlines() -#: The test files (every `.py` beside this one but `conftest.py`, whose -#: `appended_to_command_scope` is where a count is worked out) that may set a -#: count of their own, and how many times. Each is a test that BUILDS an -#: environment holding entries, to see that the code under test keeps them: -#: this file's test of `appended_to_command_scope` writes its two out, and -#: the test above starts from none; `test_adopt_git_reads.py` and -#: `test_clone_just_pushed.py` do the same for `command_scope` and for -#: `read_through_git`. +#: The files beside this one (every `.py`, `conftest.py` and the helper +#: modules too) that may set a count of their own, and how many times. Each +#: is a test that BUILDS an environment holding entries, to see that the code +#: under test keeps them: this file's test of `appended_to_command_scope` +#: writes its two out, and the test above starts from none; +#: `test_adopt_git_reads.py` and `test_clone_just_pushed.py` do the same for +#: `command_scope` and for `read_through_git`. OWN_COUNTS = {"test_no_auto_maintenance.py": 2, "test_adopt_git_reads.py": 1, "test_clone_just_pushed.py": 1} @@ -346,15 +348,38 @@ def test_a_pasted_line_keeps_the_entries_the_environment_holds_at_the_call( SETTERS = ("setenv", "setdefault", "putenv") -def is_the_count(node) -> bool: - """The string constant `"GIT_CONFIG_COUNT"`.""" - return isinstance(node, ast.Constant) and node.value == COUNT +def is_the_count(node, names=frozenset()) -> bool: + """The string constant `"GIT_CONFIG_COUNT"`, or one of `names`, the names + the module binds to it.""" + return (isinstance(node, ast.Constant) and node.value == COUNT) or ( + isinstance(node, ast.Name) and node.id in names) -def is_a_number(node) -> bool: - """A string constant of digits alone: `"1"`.""" +def is_a_written_count(node) -> bool: + """A string constant git reads as a count: digits alone (`"1"`), or + nothing (`""`), which git reads as no entries at all (2.43 and 2.47.1). + Constants only: a count built by a call (`str(1)`) is not read.""" return (isinstance(node, ast.Constant) and isinstance(node.value, str) - and node.value.isdigit()) + and (node.value.isdigit() or not node.value)) + + +def bound_names(node) -> list: + """The plain names an assignment binds: `A = B = ...`, `A: str = ...`.""" + if isinstance(node, ast.Assign): + return [target.id for target in node.targets + if isinstance(target, ast.Name)] + if isinstance(node, ast.AnnAssign) and isinstance(node.target, ast.Name): + return [node.target.id] + return [] + + +def names_of_the_count(tree) -> frozenset: + """Each name `tree` binds to `"GIT_CONFIG_COUNT"`: after `CONFIG_COUNT = + "GIT_CONFIG_COUNT"`, a `{CONFIG_COUNT: "1"}` sets the count as the string + itself would.""" + return frozenset(name for node in ast.walk(tree) + if is_the_count(getattr(node, "value", None)) + for name in bound_names(node)) def is_a_setter(node) -> bool: @@ -369,6 +394,40 @@ def is_the_appender(node) -> bool: isinstance(node, ast.Attribute) and node.attr == APPENDER) +def in_a_dict(node: ast.Dict, names: frozenset) -> list: + """`{"GIT_CONFIG_COUNT": "1"}`: the line of each such key.""" + return [key.lineno for key, value in zip(node.keys, node.values) + if is_the_count(key, names) and is_a_written_count(value)] + + +def in_a_call(node: ast.Call, names: frozenset) -> list: + """`dict(GIT_CONFIG_COUNT="1")`, `monkeypatch.setenv("GIT_CONFIG_COUNT", + "1")` and `appended_to_command_scope({}, ...)`.""" + lines = [keyword.value.lineno for keyword in node.keywords + if keyword.arg == COUNT and is_a_written_count(keyword.value)] + if (is_a_setter(node.func) and len(node.args) >= 2 + and is_the_count(node.args[0], names) + and is_a_written_count(node.args[1])): + lines.append(node.lineno) + if (is_the_appender(node.func) and node.args + and isinstance(node.args[0], ast.Dict)): + lines.append(node.lineno) + return lines + + +def by_subscript(node: ast.Assign, names: frozenset) -> list: + """`env["GIT_CONFIG_COUNT"] = "1"`.""" + if not is_a_written_count(node.value): + return [] + return [target.lineno for target in node.targets + if isinstance(target, ast.Subscript) + and is_the_count(target.slice, names)] + + +#: The nodes a count can be set in, and the reader of each. +READERS = {ast.Dict: in_a_dict, ast.Call: in_a_call, ast.Assign: by_subscript} + + def counts_set_to_a_number(source: bytes) -> list: """The line of each place `source` sets a git command scope of its own instead of appending to the environment's: @@ -377,32 +436,22 @@ def counts_set_to_a_number(source: bytes) -> list: (`{"GIT_CONFIG_COUNT": "1"}`), as a keyword (`dict(GIT_CONFIG_COUNT="1")`), in a call that names the variable and gives its value (`monkeypatch.setenv("GIT_CONFIG_COUNT", "1")`), or by - subscript (`env["GIT_CONFIG_COUNT"] = "1"`); + subscript (`env["GIT_CONFIG_COUNT"] = "1"`), the variable spelled out + or held in a name the module binds to it; - `appended_to_command_scope` handed a dict written out (`{}`), where the environment goes: it counts from nothing, and the entries the environment holds are no longer in the count it returns. A count worked out from the one the environment holds is not one of - them.""" + them. The scan reads the count's value as a constant only: a value built + by a call (`str(len(pairs))`), or held in a variable, is not read.""" + tree = ast.parse(source) + names = names_of_the_count(tree) lines = [] - for node in ast.walk(ast.parse(source)): - if isinstance(node, ast.Dict): - lines += [key.lineno for key, value in zip(node.keys, node.values) - if is_the_count(key) and is_a_number(value)] - elif isinstance(node, ast.Call): - lines += [keyword.value.lineno for keyword in node.keywords - if keyword.arg == COUNT and is_a_number(keyword.value)] - if (is_a_setter(node.func) and len(node.args) >= 2 - and is_the_count(node.args[0]) - and is_a_number(node.args[1])): - lines.append(node.lineno) - if (is_the_appender(node.func) and node.args - and isinstance(node.args[0], ast.Dict)): - lines.append(node.lineno) - elif isinstance(node, ast.Assign) and is_a_number(node.value): - lines += [target.lineno for target in node.targets - if isinstance(target, ast.Subscript) - and is_the_count(target.slice)] + for node in ast.walk(tree): + reader = READERS.get(type(node)) + if reader: + lines += reader(node, names) return sorted(lines) @@ -410,12 +459,19 @@ def counts_set_to_a_number(source: bytes) -> list: '{"GIT_CONFIG_COUNT": "1", "GIT_CONFIG_KEY_0": "protocol.file.allow"}', 'dict(GIT_CONFIG_COUNT="2")', 'monkeypatch.setenv("GIT_CONFIG_COUNT", "1")', + 'setenv("GIT_CONFIG_COUNT", "1")', 'env.setdefault("GIT_CONFIG_COUNT", "2")', + 'os.putenv("GIT_CONFIG_COUNT", "1")', 'env["GIT_CONFIG_COUNT"] = "3"', + '{"GIT_CONFIG_COUNT": ""}', + 'CONFIG_COUNT = "GIT_CONFIG_COUNT"; SYMLINKS_OFF = {CONFIG_COUNT: "1"}', + 'COUNT: str = "GIT_CONFIG_COUNT"; monkeypatch.setenv(COUNT, "1")', + 'KEY = "GIT_CONFIG_COUNT"; env[KEY] = "2"', 'appended_to_command_scope({}, [("k", "v")])', 'conftest.appended_to_command_scope({"A": "1"}, [])', -], ids=["dict", "keyword", "setenv", "setdefault", "subscript", - "appended-to-nothing", "through-a-module"]) +], ids=["dict", "keyword", "setenv", "a-bare-setenv", "setdefault", "putenv", + "subscript", "nothing", "a-name-bound-to-it", "an-annotated-name", + "a-name-by-subscript", "appended-to-nothing", "through-a-module"]) def test_the_scan_sees_each_way_a_test_can_set_a_count(source): assert counts_set_to_a_number(source.encode("utf-8")) == [1] @@ -427,30 +483,57 @@ def test_the_scan_sees_each_way_a_test_can_set_a_count(source): 'index = int(env.get("GIT_CONFIG_COUNT", "0"))', 'env["GIT_CONFIG_COUNT"] = count', '"GIT_CONFIG_COUNT is 1"', + 'KEY = "GIT_CONFIG_KEY_0"; env = {KEY: "1"}', 'appended_to_command_scope(os.environ, [("k", "v")])', 'appended_to_command_scope(held, [("k", "v")])', ], ids=["worked-out", "a-splat", "a-call", "a-read-with-a-default", "a-name", - "a-sentence", "appended-to-the-environment", "appended-to-a-name"]) + "a-sentence", "a-name-bound-to-another-key", + "appended-to-the-environment", "appended-to-a-name"]) def test_the_scan_leaves_a_count_that_is_worked_out(source): assert counts_set_to_a_number(source.encode("utf-8")) == [] +def files_off_their_count(directory: Path) -> dict: + """Each `.py` file in `directory` whose places that set a count of their + own are not as many as `OWN_COUNTS` allows it, with their lines: more + drops the entries the environment holds, and fewer is an allowance no + test needs any more.""" + found = {} + for path in sorted(directory.glob("*.py")): + lines = counts_set_to_a_number(path.read_bytes()) + if len(lines) != OWN_COUNTS.get(path.name, 0): + found[path.name] = lines + return found + + +def test_the_walk_reads_every_python_file_and_holds_each_to_its_count( + tmp_path): + """`conftest.py` and a helper module are read as a test file is, a file + that is not Python is not read, and a file `OWN_COUNTS` allows a count + is named when it sets fewer than that as well as more.""" + one = b'{"GIT_CONFIG_COUNT": "1"}\n' + for name, source in {"conftest.py": one, "gh_guard.py": one, + "notes.txt": one, "test_adopt_git_reads.py": one, + "test_clone_just_pushed.py": b"", + "test_no_auto_maintenance.py": one * 3}.items(): + (tmp_path / name).write_bytes(source) + assert files_off_their_count(tmp_path) == { + "conftest.py": [1], "gh_guard.py": [1], + "test_clone_just_pushed.py": [], + "test_no_auto_maintenance.py": [1, 2, 3]} + + def test_no_test_sets_a_git_config_count_of_its_own(): """A `GIT_CONFIG_COUNT` of 1, laid over `os.environ`, replaces the one the session fixture and the Windows job set: the entries it counted are no longer git's, so a session setting or a job's `core.autocrlf` does not reach the git the test starts. Every test gives git a setting by appending to the entries the environment holds, built at the call by - `appended_to_command_scope` (`conftest.py`). Before #758 `PASTE_ENV`, - `ALLOW_FILE_PROTOCOL`, `NO_MAINTENANCE`, `QUOTE_PATH_OFF`, `SYMLINKS_OFF` - and eight one-off environments each wrote one.""" - found = {} - for path in sorted(Path(__file__).resolve().parent.glob("*.py")): - if path.name == "conftest.py": - continue - lines = counts_set_to_a_number(path.read_bytes()) - if len(lines) != OWN_COUNTS.get(path.name, 0): - found[path.name] = lines + `appended_to_command_scope` (`conftest.py`, which is read too, and + works its count out). Before #758 `PASTE_ENV`, `ALLOW_FILE_PROTOCOL`, + `NO_MAINTENANCE`, `QUOTE_PATH_OFF`, `SYMLINKS_OFF` and eight one-off + environments each wrote one.""" + found = files_off_their_count(Path(__file__).resolve().parent) assert not found, ( "a `GIT_CONFIG_COUNT` written as a number, or `appended_to_command_" "scope` handed a dict written out, replaces the entries the "