From d91b31440025ad533243e9ed4e08b162bdfba366 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Fri, 16 Oct 2026 11:55:01 +0000 Subject: [PATCH 1/2] openRepoShape and tests.yml: the `machine:` row finds `python` where there is no `python3`, a drive-letter `$OPENREPOSHAPE_REPO` is a path, a Windows-spelled `$OPENREPOSHAPE_BIN_DIR` is respelled with `cygpath -u` or refused, and `tests-git-bash` runs #598's installer cases, so Git Bash no longer says `python3 MISSING` beside a passing preflight, clones `https://github.com/C:/...` or prints an `export PATH` bash cannot use (#649) #596's Git Bash follow-ups, five lines, from #585's writer. - The `machine:` block `--install` ends with probed `python3` alone. Where Python came from python.org, Git Bash has `python` and no `python3`, so the row said `python3 MISSING` and pointed at `--preflight`, which then passed with `python`. It now asks `command -v` for `python3`, then `python`, in `find_python`'s order, and says `present, as python` when only `python` answers. The row keeps its name, so README's and the handbook's description of the block (`python3`, each `present` or `MISSING`) stays true. Still a probe and never a run: the Microsoft Store's `python3` stub reads as present, as macOS's Command Line Tools stubs already do, and `--preflight` runs the version check that tells a stub from a Python. Running that check here would break the block's promise to execute nothing but `gh auth status`, and the Store stub opens the Store when run bare. - `shape_clone_url` read `C:/src/openRepoShape` and `C:\src\openRepoShape` as an owner/name, so setup.sh and `--doctor` were handed `https://github.com/C:/src/openRepoShape.git`. A drive letter followed by `/` or `\` now goes through as a path, as `/...` and `....` do, and as the package's `clone_url` already reads it. No owner/name holds a colon. - `--install` named a Windows-spelled `$OPENREPOSHAPE_BIN_DIR` as typed, so its PATH note printed `export PATH="D:\tools\bin:$PATH"`, which the drive's colon splits, and never found the directory on a `$PATH` that holds `/d/tools/bin`. It is respelled with `cygpath -u` first, which Git Bash has, so every line names the directory as bash does. Where no cygpath answers, the value is refused before anything is placed, with the variable to set instead: no line about `$PATH` could work there. - `tests-git-bash` now runs `tests/test_install_sh_filter_repo.py` too: the uv install, the two readinesses and the pasted next steps, which README says Git Bash runs "tested in CI", ran on Linux only. actionlint 1.7.12 passes the workflow. - `test_a_ref_the_doctor_would_check_out_is_validated`'s docstring said the refusal arrives before any clone. The command clones first, then checks the ref before `git checkout` is handed it; the docstring now says so, and the order is unchanged. Tests: `test_the_machine_report_finds_a_python_by_either_name` (3 cases), `test_a_windows_spelled_bin_dir_is_spelled_the_way_bash_spells_it` (2), `test_a_windows_spelled_bin_dir_is_refused_where_no_cygpath_spells_it` (2) and `test_a_clone_is_handed_a_path_or_url_as_it_is` (7 spellings through setup.sh and through `--doctor`, 14). Eleven fail on the base, a5245427. Off Windows a fake `cygpath` stands in for Git Bash's; on Windows the respelling test uses Git Bash's own on a real drive path. `install_with_machine` now names its directory with `bash_path`, because its `$PATH` holds no `cygpath`, and `run_cmd` takes a `cwd`. Closes #649 Lane: openreposhape-5 (openRepoShape-5) Co-Authored-By: Claude Opus 5.5 --- .github/workflows/tests.yml | 7 +- openRepoShape | 46 ++++++- tests/test_openreposhape_command.py | 189 ++++++++++++++++++++++++++-- 3 files changed, 230 insertions(+), 12 deletions(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index eac56645..72acc9ea 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -223,8 +223,13 @@ jobs: # guarantees allows: `runner.temp` is `D:\a\_temp` or `C:\a\_temp`, # and the default profile path is ~50 of MAX_PATH's 260 characters # before the suites' own nesting starts. + # + # The installer's suite is TWO FILES (#649): `tests/test_install_sh_ + # filter_repo.py` holds the git-filter-repo step and the two + # readinesses step 3's one line ends with (#595), on + # `tests/test_install_sh.py`'s fakes and set-up. - name: the launcher's and the installer's suites, under Git Bash - run: python -m pytest -q -rA tests/test_openreposhape_command.py tests/test_install_sh.py --basetemp=${{ runner.temp }}\t + run: python -m pytest -q -rA tests/test_openreposhape_command.py tests/test_install_sh.py tests/test_install_sh_filter_repo.py --basetemp=${{ runner.temp }}\t # THE openreposhape PACKAGE, BUILT AND INSTALLED AS A PERSON INSTALLS IT # (#563). A JOB OF ITS OWN, AND NO PYTEST IN IT: the build backend diff --git a/openRepoShape b/openRepoShape index 4c95873c..ffbbf584 100755 --- a/openRepoShape +++ b/openRepoShape @@ -241,9 +241,15 @@ fetch_or_die() { # handed the clone URL of the very repository these bytes came from: name a # fork or mirror once and it is used for the fetch AND the clone. A URL or a # path on disk (what the tests point it at) goes through untouched. +# +# A WINDOWS PATH IS A PATH TOO (#649): `C:/src/openRepoShape` and +# `C:\src\openRepoShape`, which Git Bash and git itself both take, were read +# as an owner/name and became `https://github.com/C:/src/openRepoShape.git`. +# No owner/name holds a colon, and the package's `clone_url` reads a drive +# letter the same way. shape_clone_url() { case "$REPO" in - *://* | *@*:* | /* | .*) printf '%s' "$REPO" ;; + *://* | *@*:* | /* | .* | [A-Za-z]:/* | [A-Za-z]:\\*) printf '%s' "$REPO" ;; *) printf 'https://github.com/%s.git' "$REPO" ;; esac } @@ -306,11 +312,19 @@ collect_commands() { # `python3`, and `git-filter-repo` -- which only `adopt-project.py` needs and # `--preflight` therefore does not ask about. # -# IT OFFERS NOTHING AND RUNS NO INSTALLER. Four `command -v` probes, which +# IT OFFERS NOTHING AND RUNS NO INSTALLER. `command -v` probes, which # execute nothing at all, and one `gh auth status`, which is the only way to # learn a login state. An offer belongs behind a typed yes, in `--preflight`, # and this line points there rather than growing a second consent. # +# A PYTHON BY EITHER NAME, `python3` THEN `python`, in `find_python`'s order +# (#649). Where Python came from python.org, Git Bash has `python` and no +# `python3`, and this row said `python3 MISSING` while `--preflight` passed. +# The row keeps its name and says `present, as python` there. Still a probe +# and never a run, so the Microsoft Store's `python3` stub, like macOS's +# Command Line Tools stubs, reads as present here: `--preflight` runs the +# version check that tells a stub from a Python, and this report runs none. +# # IT NEVER CHANGES THE EXIT CODE. A machine missing a prerequisite did not # make the copy that was just placed any less placed, and an install that # reported failure because `gh` was not logged in would be an install nobody @@ -319,7 +333,10 @@ machine_report() { local name state short=0 say " machine:" for name in git gh python3 git-filter-repo; do - if command -v "$name" >/dev/null 2>&1; then + if [ "$name" = python3 ] && ! command -v python3 >/dev/null 2>&1 && + command -v python >/dev/null 2>&1; then + state="present, as python" + elif command -v "$name" >/dev/null 2>&1; then state="present" if [ "$name" = gh ]; then if gh auth status >/dev/null 2>&1; then @@ -386,7 +403,28 @@ record_ref() { install_commands() { local dir="${OPENREPOSHAPE_BIN_DIR:-$HOME/.local/bin}" - local name target source verb placed=0 recording=0 record + local name target source verb placed=0 recording=0 record spelled="" + # A WINDOWS SPELLING IS SPELLED AGAIN THE WAY BASH SPELLS IT (#649), with + # `cygpath -u`, which Git Bash has, before anything is checked or placed: + # `D:\tools\bin` becomes `/d/tools/bin`. Kept as typed, every line below + # named it, and the PATH note printed `export PATH="D:\tools\bin:$PATH"`, + # which the drive's colon splits in two, and never found it on a `$PATH` + # that holds `/d/tools/bin`. Where no cygpath answers, the bash running + # this does not know the drive letter as a drive, and the value is + # refused: no line it could print about `$PATH` would work. + case "$dir" in + [A-Za-z]:/* | [A-Za-z]:\\*) + if command -v cygpath >/dev/null 2>&1; then + spelled="$(cygpath -u -- "$dir" 2>/dev/null)" || spelled="" + fi + case "$spelled" in + "" | [A-Za-z]:*) + die "--install would place this command in '$dir', which is spelled for Windows, and no cygpath here spells it the way bash does, so the directory could not go on \$PATH: the drive's colon splits it. Nothing was placed. Set \$OPENREPOSHAPE_BIN_DIR to the directory as this shell spells it, then run this again." + ;; + esac + dir="$spelled" + ;; + esac if [ "$REF_FROM_RECORD" -eq 1 ] || { [ "$REF_NAMED" -eq 1 ] && [ "${OPENREPOSHAPE_RECORD_REF:-}" = 1 ]; }; then recording=1 diff --git a/tests/test_openreposhape_command.py b/tests/test_openreposhape_command.py index 4f6f2ea6..9daf8efb 100644 --- a/tests/test_openreposhape_command.py +++ b/tests/test_openreposhape_command.py @@ -117,8 +117,8 @@ def command_env(home: Path | None = None, env: dict | None = None, def run_cmd(*args: str, home: Path | None = None, env: dict | None = None, - setup_sh: Path | None = SETUP, - command: Path = COMMAND) -> subprocess.CompletedProcess: + setup_sh: Path | None = SETUP, command: Path = COMMAND, + cwd: Path | None = None) -> subprocess.CompletedProcess: """The command, run from its file, with that environment. `input=""` means stdin is a pipe rather than a terminal, which is what the @@ -129,11 +129,13 @@ def run_cmd(*args: str, home: Path | None = None, env: dict | None = None, `command` is this checkout's file unless a test names an INSTALLED copy, which is how the record `--install` leaves beside one is read (#564). + `cwd`, where a relative path the command is handed must land. """ return subprocess.run([BASH, bash_path(command), *args], capture_output=True, text=True, check=False, input="", env=command_env(home, env, setup_sh), - start_new_session=True) + start_new_session=True, + cwd=None if cwd is None else str(cwd)) def write_script(path: Path, text: str) -> None: @@ -346,6 +348,85 @@ def test_install_says_how_to_put_it_on_path(tmp_path): in result.stdout +#: `$OPENREPOSHAPE_BIN_DIR` spelled for Windows, as a Git Bash user may type +#: it, where a test needs a value that names no directory of its own (#649). +WINDOWS_BIN_DIR = "D:\\tools\\bin" + + +def fake_cygpath(directory: Path, answer: str | None) -> None: + """A `cygpath` in `directory` that spells `WINDOWS_BIN_DIR` as `answer`, + as Git Bash's own `cygpath -u` spells a drive path, or fails when + `answer` is None. Anything else it is asked fails.""" + directory.mkdir(exist_ok=True) + answers = "" if answer is None else ( + f'[ "$*" = {shlex.quote("-u -- " + WINDOWS_BIN_DIR)} ] && ' + f"printf '%s\\n' {shlex.quote(answer)} && exit 0\n") + write_script(directory / "cygpath", f"#!/bin/sh\n{answers}exit 1\n") + (directory / "cygpath").chmod(0o755) + + +@pytest.mark.parametrize("on_path", [False, True], + ids=["not-on-path", "on-path"]) +def test_a_windows_spelled_bin_dir_is_spelled_the_way_bash_spells_it( + tmp_path, on_path): + """`OPENREPOSHAPE_BIN_DIR=D:\\tools\\bin` in Git Bash (#649). + + `--install` named the directory as typed, so its PATH note printed + `export PATH="D:\\tools\\bin:$PATH"`, which the drive's colon splits in + two, and never found it on a `$PATH` holding `/d/tools/bin`. It is + spelled with `cygpath -u` first, so every line names the directory as + bash does, and the note is printed only when that directory is not on + `$PATH`. On Windows it is a real drive path and Git Bash's own + `cygpath`; elsewhere a fake answers as that one would. + """ + bin_dir = tmp_path / "bin" + spelled = bash_path(bin_dir) + path = os.environ["PATH"] + if os.name == "nt": + typed = str(bin_dir) + else: + typed = WINDOWS_BIN_DIR + fake_cygpath(tmp_path / "fake-path", spelled) + path = f"{tmp_path / 'fake-path'}{os.pathsep}{path}" + if on_path: + path = f"{bin_dir}{os.pathsep}{path}" + result = run_cmd("--install", home=tmp_path, cwd=tmp_path, + env={"PATH": path, "OPENREPOSHAPE_BIN_DIR": typed}) + assert result.returncode == 0, result.stdout + result.stderr + lines = result.stdout.splitlines() + assert f"openRepoShape: installed at {spelled}/openRepoShape" in lines, \ + result.stdout + assert (bin_dir / "openRepoShape").read_bytes() == COMMAND.read_bytes() + assert (f' export PATH="{spelled}:$PATH"' in lines) is not on_path, \ + result.stdout + assert typed not in result.stdout + + +@pytest.mark.parametrize("failing_cygpath", [False, True], + ids=["no-cygpath", "cygpath-fails"]) +def test_a_windows_spelled_bin_dir_is_refused_where_no_cygpath_spells_it( + tmp_path, failing_cygpath): + """With no `cygpath` to spell it, or one that cannot, the drive's colon + would still split the directory on `$PATH`, so `--install` refuses + before it places anything, and says what to set instead (#649). The + `$PATH` is the machine report's own, which holds no `cygpath` but the + failing fake.""" + env = machine_path(tmp_path, ()) + env.pop("_log") + if failing_cygpath: + fake_cygpath(tmp_path / "machine-path", None) + result = run_cmd("--install", home=tmp_path, cwd=tmp_path, + env={**env, "OPENREPOSHAPE_BIN_DIR": WINDOWS_BIN_DIR}) + assert result.returncode == 2, result.stdout + result.stderr + said = " ".join(result.stderr.split()) + assert f"--install would place this command in '{WINDOWS_BIN_DIR}', " \ + "which is spelled for Windows" in said, said + assert "Nothing was placed. Set $OPENREPOSHAPE_BIN_DIR to the directory " \ + "as this shell spells it, then run this again." in said, said + assert result.stdout == "" + assert [path.name for path in tmp_path.iterdir()] == ["machine-path"] + + def test_install_from_a_file_never_calls_gh(tmp_path): """`--install` run from a file copies THOSE bytes. A machine with no `gh` — or no network — must still be able to install the command, so a `gh` on @@ -553,6 +634,9 @@ def test_help_still_exits_zero(): #: restricted `$PATH` the machine-report tests build. Everything else it uses #: is a bash builtin. The list is short because the installer is: make a #: temporary directory, copy a file into place, compare, chmod, tidy up. +#: `cygpath` is left off on purpose: `--install` asks it only to respell a +#: directory written for Windows (#649), and a test that hands this `$PATH` +#: one is a test of the refusal where no `cygpath` answers. #: `tests/test_setup_sh.py::test_the_shim_refuses_before_cloning_when_git_is_ #: missing` is the precedent for the whole shape of this — an absolute bash #: and a `$PATH` the test owns entirely, because there is no other way to @@ -606,12 +690,15 @@ def install_with_machine(tmp_path, present, logged_in=True): log = Path(env.pop("_log")) bin_dir = tmp_path / "bin" # The bash by ABSOLUTE path: `subprocess` resolves a bare argv[0] against - # the CHILD's PATH, and this one holds no bash on purpose. + # the CHILD's PATH, and this one holds no bash on purpose. The directory + # as bash spells it, because this PATH holds no `cygpath` to respell a + # Windows one (#649). result = subprocess.run( [BASH, bash_path(COMMAND), "--install"], capture_output=True, text=True, check=False, input="", env=command_env(home=tmp_path, setup_sh=None, - env={**env, "OPENREPOSHAPE_BIN_DIR": str(bin_dir)})) + env={**env, + "OPENREPOSHAPE_BIN_DIR": bash_path(bin_dir)})) return result, log, bin_dir @@ -673,6 +760,33 @@ def test_the_machine_report_runs_no_installer(tmp_path): assert "was EXECUTED by a read-only report" not in result.stderr +@pytest.mark.parametrize("pythons, row", [ + pytest.param(("python",), "present, as python", id="python-alone"), + pytest.param(("python3", "python"), "present", id="both"), + pytest.param((), "MISSING", id="neither"), +]) +def test_the_machine_report_finds_a_python_by_either_name(tmp_path, pythons, + row): + """`python3`, THEN `python`, the order `--preflight` finds one in (#649). + + Where Python came from python.org, Git Bash has `python` and no + `python3`, and the row said `python3 MISSING`, with the line pointing at + `--preflight`, which then passed. It keeps its name and says which name + answered, and only a machine with neither is short of one. Still by + `command -v` alone: every fake here logs a run, and only `gh auth + status` may be in the log. + """ + result, log, _bin = install_with_machine( + tmp_path, ("git", "gh", *pythons, "git-filter-repo")) + assert result.returncode == 0, result.stdout + result.stderr + assert re.search(rf"^ python3 +{re.escape(row)}$", result.stdout, + re.M), result.stdout + assert ("--preflight" in result.stdout) is (row == "MISSING"), \ + result.stdout + assert log.read_text(encoding="utf-8").split("\n")[:-1] == \ + ["gh auth status"], log.read_text(encoding="utf-8") + + # --- the fix round on PR #96 ------------------------------------------------ @pytest.mark.parametrize("args", [ @@ -707,8 +821,9 @@ def test_a_ref_the_doctor_would_check_out_is_validated(ref, tmp_path): `setup.sh` validates `--shape-ref` in three steps and this value lands in the same place, which the first cut of `--doctor` missed (Copilot, PR #96): a leading dash is an option to git, not a ref. Run from STDIN - so the clone path is the one taken, and the refusal has to arrive before - any clone does. + so the clone path is the one taken: the command clones this checkout + first, then checks the ref, and refuses before `git checkout` is ever + handed it. """ result = subprocess.run( ["bash", "-s", "--", "--doctor", str(tmp_path)], executable=BASH, @@ -1117,6 +1232,66 @@ def test_install_from_stdin_fetches_itself_into_a_live_workdir(offline_github, assert f"{name}: installed at" in result.stdout +# --- what `git clone` is handed for `$OPENREPOSHAPE_REPO` (#649) ------------- + +#: `$OPENREPOSHAPE_REPO` as a person sets it, and what a clone is handed for +#: it, `None` meaning the value itself. An owner/name becomes its github.com +#: URL; a URL, an scp address and a path go through as they are, and a +#: Windows path with a drive letter is a path in either slash (#649). +CLONE_URLS = [ + pytest.param("acme/openRepoShape", + "https://github.com/acme/openRepoShape.git", id="owner-name"), + pytest.param("https://example.invalid/acme/openRepoShape.git", None, + id="url"), + pytest.param("git@example.invalid:acme/openRepoShape.git", None, + id="scp"), + pytest.param("/srv/git/openRepoShape.git", None, id="posix-path"), + pytest.param("C:/src/openRepoShape", None, id="drive-slash"), + pytest.param("C:\\src\\openRepoShape", None, id="drive-backslash"), + pytest.param("d:\\work\\openRepoShape.git", None, id="drive-lower-case"), +] + + +def cloned_for(tmp_path, repo: str, caller: str) -> list: + """What the clone `caller` makes is handed for `$OPENREPOSHAPE_REPO`: + the value setup.sh is given to clone, through a setup.sh that prints + it, or the URL `--doctor`'s own `git clone` names, through a `git` that + prints it and fails, from a copy of the command with no checkout of the + standard beside it. Nothing is cloned and nothing is fetched.""" + if caller == "setup.sh": + stub = tmp_path / "setup.sh" + write_script(stub, '#!/usr/bin/env bash\n' + 'printf "CLONES %s\\n" "$OPENREPOSHAPE_REPO"\n') + result = run_cmd("--preflight", setup_sh=stub, + env={"OPENREPOSHAPE_REPO": repo}) + assert result.returncode == 0, result.stdout + result.stderr + else: + fake = tmp_path / "fake-path" + fake.mkdir() + write_script(fake / "git", '#!/bin/sh\n[ "$1" = clone ] && ' + 'printf "CLONES %s\\n" "$4"\nexit 1\n') + (fake / "git").chmod(0o755) + copy = tmp_path / "openRepoShape" + copy.write_bytes(COMMAND.read_bytes()) + result = run_cmd("--doctor", str(tmp_path), command=copy, env={ + "OPENREPOSHAPE_REPO": repo, "TMPDIR": str(tmp_path), + "PATH": f"{fake}{os.pathsep}{os.environ['PATH']}"}) + assert result.returncode == 2, result.stdout + result.stderr + assert "could not clone" in result.stderr + return re.findall(r"^CLONES (.*)$", result.stdout, re.M) + + +@pytest.mark.parametrize("caller", ["setup.sh", "doctor"]) +@pytest.mark.parametrize("repo, url", CLONE_URLS) +def test_a_clone_is_handed_a_path_or_url_as_it_is(tmp_path, repo, url, + caller): + """`shape_clone_url`, through both of its callers (#649). A drive + letter was read as an owner, so `C:/src/openRepoShape` became + `https://github.com/C:/src/openRepoShape.git`, a repository on + github.com that nobody named, for setup.sh's clone and the doctor's.""" + assert cloned_for(tmp_path, repo, caller) == [url or repo] + + # --- the ref `--install` records, and the installed command runs (#564) ----- #: A ref for an install to record. Nothing fetches it: every install below From e85573acc9c825cccc1c26f8f452078c99f639b1 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Fri, 16 Oct 2026 12:33:58 +0000 Subject: [PATCH 2/2] =?UTF-8?q?tests/test=5Fopenreposhape=5Fcommand.py:=20?= =?UTF-8?q?a=20Windows-spelled=20`$OPENREPOSHAPE=5FBIN=5FDIR`=20is=20teste?= =?UTF-8?q?d=20in=20both=20slashes=20and=20against=20a=20`cygpath`=20that?= =?UTF-8?q?=20answers=20a=20drive,=20so=20dropping=20the=20`D:/=E2=80=A6`?= =?UTF-8?q?=20arm=20or=20the=20drive-answer=20refusal=20no=20longer=20leav?= =?UTF-8?q?es=20the=20suite=20green;=20three=20comments=20in=20openRepoSha?= =?UTF-8?q?pe=20and=20tests.yml=20say=20what=20is=20true=20(#649)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix round 1 on PR #656, from rv649's review. - The respelling test and the refusal test now run over `D:\tools\bin` and `D:/tools/bin`. `fake_cygpath` takes the typed value, because it answers only the exact `-u -- ` it is asked. With the `D:/…` arm dropped the suite stayed green (91 passed), though `D:/tools/bin` with no `cygpath` printed `export PATH="D:/tools/bin:$PATH"`, the defect #649 names; five of the new cases fail under that mutation. - The refusal test has a third case: a `cygpath` that answers `D:/tools/bin` with exit 0, which the `[A-Za-z]:*` arm of the refusal refuses. With that arm dropped the suite was green too; two of the new cases fail under that mutation. - On Windows the refusal test's value is a drive path under `tmp_path` (`str(...)` for the backslash, `.as_posix()` for the slash), and the assert names that value. It was an absolute `D:\tools\bin`, so a refusal that regressed would have put a command on the machine's real D: drive, where the `iterdir` check could not see it. - Three comments are corrected; no code line or command changes. The `machine:` row's now names a cost it left out: a `python` that is Python 2, or older than 3.9, where there is no `python3`, reads as present and loses the `--preflight` pointer. The refusal's no longer says the bash running it does not know the drive letter as a drive: Git Bash does, and the drive's colon still splits `$PATH`. The `tests-git-bash` job's opening comment names the installer's second file. Lane: openreposhape-5 (openRepoShape-5) Co-Authored-By: Claude Sonnet 5.5 --- .github/workflows/tests.yml | 5 +- openRepoShape | 15 +++--- tests/test_openreposhape_command.py | 83 +++++++++++++++++++---------- 3 files changed, 68 insertions(+), 35 deletions(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 72acc9ea..e97dc97f 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -138,8 +138,9 @@ jobs: # THE LAUNCHER'S AND THE INSTALLER'S SUITES ON WINDOWS, UNDER GIT BASH # (#585). README says Git Bash runs the `openRepoShape` command and the # one-line install, "tested in CI but for a typed `yes`", and this job is - # what makes that true: `tests/test_openreposhape_command.py` and - # `tests/test_install_sh.py`, run here as Git Bash runs them. + # what makes that true: `tests/test_openreposhape_command.py`, + # `tests/test_install_sh.py` and `tests/test_install_sh_filter_repo.py`, + # run here as Git Bash runs them. # # TWO SUITES, NOT THE WHOLE SUITE. #597 dropped the Windows job for the # whole estate, and this job is the one exception made to it (see the diff --git a/openRepoShape b/openRepoShape index ffbbf584..61291e26 100755 --- a/openRepoShape +++ b/openRepoShape @@ -321,9 +321,11 @@ collect_commands() { # (#649). Where Python came from python.org, Git Bash has `python` and no # `python3`, and this row said `python3 MISSING` while `--preflight` passed. # The row keeps its name and says `present, as python` there. Still a probe -# and never a run, so the Microsoft Store's `python3` stub, like macOS's -# Command Line Tools stubs, reads as present here: `--preflight` runs the -# version check that tells a stub from a Python, and this report runs none. +# and never a run, so what answers is never asked its version: the Microsoft +# Store's `python3` stub and macOS's Command Line Tools stubs read as present +# here, and so does a `python` that is Python 2, or older than 3.9, where +# there is no `python3`. `--preflight` runs the version check, and this +# report runs none. # # IT NEVER CHANGES THE EXIT CODE. A machine missing a prerequisite did not # make the copy that was just placed any less placed, and an install that @@ -409,9 +411,10 @@ install_commands() { # `D:\tools\bin` becomes `/d/tools/bin`. Kept as typed, every line below # named it, and the PATH note printed `export PATH="D:\tools\bin:$PATH"`, # which the drive's colon splits in two, and never found it on a `$PATH` - # that holds `/d/tools/bin`. Where no cygpath answers, the bash running - # this does not know the drive letter as a drive, and the value is - # refused: no line it could print about `$PATH` would work. + # that holds `/d/tools/bin`. Where no cygpath answers, nothing here can + # spell it as bash's `$PATH` needs it (and off Windows the drive letter is + # no drive at all), so the value is refused: no line it could print about + # `$PATH` would work. case "$dir" in [A-Za-z]:/* | [A-Za-z]:\\*) if command -v cygpath >/dev/null 2>&1; then diff --git a/tests/test_openreposhape_command.py b/tests/test_openreposhape_command.py index 9daf8efb..fe2d6759 100644 --- a/tests/test_openreposhape_command.py +++ b/tests/test_openreposhape_command.py @@ -349,44 +349,66 @@ def test_install_says_how_to_put_it_on_path(tmp_path): #: `$OPENREPOSHAPE_BIN_DIR` spelled for Windows, as a Git Bash user may type -#: it, where a test needs a value that names no directory of its own (#649). +#: it, in either slash (#649): where a test needs a value that names no +#: directory of its own. WINDOWS_BIN_DIR = "D:\\tools\\bin" +WINDOWS_BIN_DIR_SLASH = "D:/tools/bin" + +#: The two spellings a person types, for a test to run over both. Git Bash +#: takes `D:\tools\bin` and `D:/tools/bin` alike, and `--install` has an arm +#: for each. +BIN_DIR_SPELLINGS = pytest.mark.parametrize("spelling", + ["backslash", "slash"]) + + +def typed_bin_dir(bin_dir: Path, spelling: str) -> str: + """`$OPENREPOSHAPE_BIN_DIR` as a person types it for Windows, in + `spelling`, "backslash" or "slash". Off Windows it is `D:\\tools\\bin` + or `D:/tools/bin`, which name no directory of their own. On Windows it + is `bin_dir` itself, a real drive path under the test's directory, with + `str` for the backslash and `as_posix` for the slash: a command placed + where it should not be then lands where the test can see it, and never + on the machine's own `D:` drive.""" + if os.name == "nt": + return bin_dir.as_posix() if spelling == "slash" else str(bin_dir) + return WINDOWS_BIN_DIR_SLASH if spelling == "slash" else WINDOWS_BIN_DIR -def fake_cygpath(directory: Path, answer: str | None) -> None: - """A `cygpath` in `directory` that spells `WINDOWS_BIN_DIR` as `answer`, - as Git Bash's own `cygpath -u` spells a drive path, or fails when - `answer` is None. Anything else it is asked fails.""" +def fake_cygpath(directory: Path, typed: str, answer: str | None) -> None: + """A `cygpath` in `directory` that spells `typed` as `answer`, as Git + Bash's own `cygpath -u` spells a drive path, or fails when `answer` is + None. Anything else it is asked fails.""" directory.mkdir(exist_ok=True) answers = "" if answer is None else ( - f'[ "$*" = {shlex.quote("-u -- " + WINDOWS_BIN_DIR)} ] && ' + f'[ "$*" = {shlex.quote("-u -- " + typed)} ] && ' f"printf '%s\\n' {shlex.quote(answer)} && exit 0\n") write_script(directory / "cygpath", f"#!/bin/sh\n{answers}exit 1\n") (directory / "cygpath").chmod(0o755) +@BIN_DIR_SPELLINGS @pytest.mark.parametrize("on_path", [False, True], ids=["not-on-path", "on-path"]) def test_a_windows_spelled_bin_dir_is_spelled_the_way_bash_spells_it( - tmp_path, on_path): - """`OPENREPOSHAPE_BIN_DIR=D:\\tools\\bin` in Git Bash (#649). + tmp_path, on_path, spelling): + """`OPENREPOSHAPE_BIN_DIR=D:\\tools\\bin` or `D:/tools/bin` in Git Bash + (#649). `--install` named the directory as typed, so its PATH note printed `export PATH="D:\\tools\\bin:$PATH"`, which the drive's colon splits in two, and never found it on a `$PATH` holding `/d/tools/bin`. It is spelled with `cygpath -u` first, so every line names the directory as bash does, and the note is printed only when that directory is not on - `$PATH`. On Windows it is a real drive path and Git Bash's own - `cygpath`; elsewhere a fake answers as that one would. + `$PATH`. Both slashes are drive paths. On Windows it is a real drive + path and Git Bash's own `cygpath`; elsewhere a fake answers as that one + would. """ bin_dir = tmp_path / "bin" spelled = bash_path(bin_dir) + typed = typed_bin_dir(bin_dir, spelling) path = os.environ["PATH"] - if os.name == "nt": - typed = str(bin_dir) - else: - typed = WINDOWS_BIN_DIR - fake_cygpath(tmp_path / "fake-path", spelled) + if os.name != "nt": + fake_cygpath(tmp_path / "fake-path", typed, spelled) path = f"{tmp_path / 'fake-path'}{os.pathsep}{path}" if on_path: path = f"{bin_dir}{os.pathsep}{path}" @@ -402,24 +424,31 @@ def test_a_windows_spelled_bin_dir_is_spelled_the_way_bash_spells_it( assert typed not in result.stdout -@pytest.mark.parametrize("failing_cygpath", [False, True], - ids=["no-cygpath", "cygpath-fails"]) +@BIN_DIR_SPELLINGS +@pytest.mark.parametrize("cygpath_there, answer", [ + pytest.param(False, None, id="no-cygpath"), + pytest.param(True, None, id="cygpath-fails"), + pytest.param(True, WINDOWS_BIN_DIR_SLASH, id="cygpath-answers-a-drive"), +]) def test_a_windows_spelled_bin_dir_is_refused_where_no_cygpath_spells_it( - tmp_path, failing_cygpath): - """With no `cygpath` to spell it, or one that cannot, the drive's colon - would still split the directory on `$PATH`, so `--install` refuses - before it places anything, and says what to set instead (#649). The - `$PATH` is the machine report's own, which holds no `cygpath` but the - failing fake.""" + tmp_path, cygpath_there, answer, spelling): + """With no `cygpath` to spell it, one that cannot, or one that answers + with a drive spelling again, the drive's colon would still split the + directory on `$PATH`, so `--install` refuses before it places anything, + and says what to set instead (#649). Either slash. The `$PATH` is the + machine report's own, which holds no `cygpath` but the fake. The typed + value is under `tmp_path` on Windows, so a refusal that regressed would + place the command where this test looks, and not on a real drive.""" env = machine_path(tmp_path, ()) env.pop("_log") - if failing_cygpath: - fake_cygpath(tmp_path / "machine-path", None) + typed = typed_bin_dir(tmp_path / "bin", spelling) + if cygpath_there: + fake_cygpath(tmp_path / "machine-path", typed, answer) result = run_cmd("--install", home=tmp_path, cwd=tmp_path, - env={**env, "OPENREPOSHAPE_BIN_DIR": WINDOWS_BIN_DIR}) + env={**env, "OPENREPOSHAPE_BIN_DIR": typed}) assert result.returncode == 2, result.stdout + result.stderr said = " ".join(result.stderr.split()) - assert f"--install would place this command in '{WINDOWS_BIN_DIR}', " \ + assert f"--install would place this command in '{typed}', " \ "which is spelled for Windows" in said, said assert "Nothing was placed. Set $OPENREPOSHAPE_BIN_DIR to the directory " \ "as this shell spells it, then run this again." in said, said