diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index eac56645..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 @@ -223,8 +224,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..61291e26 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,21 @@ 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 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 # reported failure because `gh` was not logged in would be an install nobody @@ -319,7 +335,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 +405,29 @@ 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, 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 + 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..fe2d6759 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,114 @@ 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, 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, 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 -- " + 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, 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`. 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": + 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}" + 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 + + +@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, 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") + 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": typed}) + assert result.returncode == 2, result.stdout + result.stderr + said = " ".join(result.stderr.split()) + 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 + 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 +663,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 +719,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 +789,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 +850,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 +1261,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