diff --git a/templates/assembly-root/scripts/validate-pins.py b/templates/assembly-root/scripts/validate-pins.py index 8b7c1818..20ba7f35 100755 --- a/templates/assembly-root/scripts/validate-pins.py +++ b/templates/assembly-root/scripts/validate-pins.py @@ -470,7 +470,8 @@ def _refuse_an_unreadable_git_dir(root: Path, dotgit: Path, (`_gitfile_opens`): only where it is a regular file, and an open that finds nothing is a git directory with no `HEAD`. One this user cannot open is named, `.git/modules/spec/HEAD` or `spec/.git/HEAD`, with the - same remedy.""" + same remedy. So is one git cannot read as a `HEAD`, a FIFO among them + (#1290): see `_a_head_git_cannot_read`.""" named = dotgit try: gitdir = _leg_git_dir(dotgit) @@ -480,10 +481,52 @@ def _refuse_an_unreadable_git_dir(root: Path, dotgit: Path, if _is_there(head): named = named / "HEAD" _gitfile_opens(head) + _a_head_git_cannot_read(head) except OSError as exc: raise _unreadable_file(code, _named(root, named), exc, reads) from exc +# A GIT DIRECTORY'S `HEAD` THAT GIT CANNOT READ AS ONE (#1290, RV1239 on +# #1239, W1256 on #1263). The guard above opens a `HEAD` only where it is a +# regular file, since the open of a FIFO waits for a writer; but git's own +# open of it, in the pinned commit's `git rev-parse`, waited, and `git_out` +# has no timeout, so the gate gave no answer at all. A `HEAD` that `stat`, +# which follows a link, finds neither a file nor a directory (a FIFO, a +# socket or a device, or a symlink to one) is refused by name before git +# is asked. A socket, or a symlink to a FIFO or a device, which git does +# not wait on, ended the run in `pin-commit-unresolvable`, which says the +# pinned commit is missing; git reads a symlinked `HEAD` only where it +# names `refs/`. A directory, a symlink to a file or a directory, and a +# symlink that leads nowhere are left to git, as they were. The root's own +# `.git/HEAD`, asked here too (`_refuse_an_unreadable_root`, #1146), is +# `git-repo-unreadable` the same way. +# +# HERE, AND NOT AS A TIMEOUT IN `git_out` (#1290's choice (b)). `git_out` is +# `scripts/repo_shape.py`'s, a shape copy every root pins on its own and +# may hold older than this file (#370, #610), so a timeout there reaches no +# root whose copy predates it; it is shared by every tool, `tree_digest`'s +# `ls-tree -r` over a whole leg among them, with no flag to say how long is +# too long; and a call that timed out raises `subprocess.TimeoutExpired`, +# which no caller here catches (a traceback, exit 1, a finding's code), or, +# made a `git-failed` refusal, reads as `pin-commit-unresolvable` again. +# What else in a git directory git waits on, its `config` a FIFO among +# them (measured: no answer), is not asked here. + +def _a_head_git_cannot_read(head: Path) -> None: + """Raise an `OSError` whose reason says what stands there, as + `_what_stands` names it, for a git directory's `HEAD` that is neither a + file nor a directory (#1290), so `_refuse_an_unreadable_git_dir` names + it as it names a `HEAD` this user cannot open: `: cannot be read + (a FIFO, not a file)`, or `(a symlink to a device, not a file)`, with + the remedy for a cause that is no permission, which says to put a + regular file back in its place. `family-root`'s `validate-family.py` + raises the same words for a member's.""" + what = _what_stands( + head, lambda mode: stat.S_ISREG(mode) or stat.S_ISDIR(mode)) + if what is not None: + raise OSError(None, f"{what}, not a file" if what else "not a file") + + #: The refusal of a root whose own repository this user cannot read (#1146), #: and what that repository is read for, as `_unreadable_file`'s remedy #: names it. `validate-manifest.py` refuses its `.git` by the same code. diff --git a/templates/family-root/scripts/validate-family.py b/templates/family-root/scripts/validate-family.py index 72b59b2b..67de858f 100755 --- a/templates/family-root/scripts/validate-family.py +++ b/templates/family-root/scripts/validate-family.py @@ -727,7 +727,8 @@ def _refuse_an_unreadable_git_dir(checkout: Path, path: str) -> None: a FIFO waits for a writer, and an open that finds nothing is a git directory with no `HEAD`. One this user cannot open is named, `.git/modules//HEAD` or `/.git/HEAD`, with the same - remedy.""" + remedy. So is one git cannot read as a `HEAD`, a FIFO among them + (#1290): see `_a_head_git_cannot_read`.""" dotgit = checkout / ".git" named = f"{path}/.git" try: @@ -746,10 +747,60 @@ def _refuse_an_unreadable_git_dir(checkout: Path, path: str) -> None: if _is_a(head, _any_kind): named = named_head _gitfile_opens(head) + _a_head_git_cannot_read(head) except OSError as exc: raise _unreadable(MEMBER_UNREADABLE, named, exc) from exc +# A GIT DIRECTORY'S `HEAD` THAT GIT CANNOT READ AS ONE (#1290, RV1239 on +# #1239, W1256 on #1263). The guard above opens a `HEAD` only where it is a +# regular file, since the open of a FIFO waits for a writer; but git's own +# open of it, in the pinned commit's `git rev-parse`, waited, and `git_out` +# has no timeout, so the gate gave no answer at all. A `HEAD` that `stat`, +# which follows a link, finds neither a file nor a directory (a FIFO, a +# socket or a device, or a symlink to one) is refused by name before git +# is asked. A socket, or a symlink to a FIFO or a device, which git does +# not wait on, ended the run in `member-commit-unresolvable`, which says the +# pinned commit is missing; git reads a symlinked `HEAD` only where it +# names `refs/`. A directory, a symlink to a file or a directory, and a +# symlink that leads nowhere are left to git, as they were. The holder's +# own `HEAD` is not asked here (#1172's). +# +# HERE, AND NOT AS A TIMEOUT IN `git_out` (#1290's choice (b)). `git_out` is +# `scripts/repo_shape.py`'s, a shape copy every root pins on its own and +# may hold older than this file (#370, #610), so a timeout there reaches no +# root whose copy predates it; it is shared by every tool, `tree_digest`'s +# `ls-tree -r` over a whole leg among them, with no flag to say how long is +# too long; and a call that timed out raises `subprocess.TimeoutExpired`, +# which no caller here catches (a traceback, exit 1, a finding's code), or, +# made a `git-failed` refusal, reads as `member-commit-unresolvable` again. +# What else in a git directory git waits on, its `config` a FIFO among +# them (measured: no answer), is not asked here. + +#: What a `stat` mode that is neither a file nor a directory names, as +#: `validate-pins.py`'s `_KINDS` names it (#1290). A device is either kind. +_HEAD_KINDS = ( + (stat.S_ISFIFO, "a FIFO"), (stat.S_ISSOCK, "a socket"), + (stat.S_ISCHR, "a device"), (stat.S_ISBLK, "a device")) + + +def _a_head_git_cannot_read(head: Path) -> None: + """Raise an `OSError` whose reason says what stands there, in + `validate-pins.py`'s words for a leg's, for a member git directory's + `HEAD` that is neither a file nor a directory (#1290), so + `_refuse_an_unreadable_git_dir` names it as it names a `HEAD` this user + cannot open: `: cannot be read (a FIFO, not a file)`, or `(a + symlink to a device, not a file)`, with the remedy for a cause that is + no permission, which says to put a regular file back in its place. Its + `stat`, then its own `lstat`, as that gate's `_what_stands` asks.""" + mode = head.stat().st_mode + if stat.S_ISREG(mode) or stat.S_ISDIR(mode): + return + link = "a symlink to " if stat.S_ISLNK(head.lstat().st_mode) else "" + kind = next((name for test, name in _HEAD_KINDS if test(mode)), None) + raise OSError(None, f"{link}{kind}, not a file" if kind else "not a file") + + def _checkout_gone(root: Path, path: str) -> bool: """`_gone` of the `.git` at the gitlink path `path`, or `member-unreadable` naming it where its `stat` was refused for any diff --git a/tests/test_validate_family_unreadable.py b/tests/test_validate_family_unreadable.py index c22d021f..f321c88e 100644 --- a/tests/test_validate_family_unreadable.py +++ b/tests/test_validate_family_unreadable.py @@ -53,6 +53,14 @@ regular file, and refused `member-unreadable`, naming that `HEAD`; a parity row holds the name to the gate's. +A MEMBER GIT DIRECTORY'S `HEAD` THAT GIT CANNOT READ AS ONE (#1290), as +the gate's leg's: a FIFO is not opened by #1256's guard, but git's open of +it waits for a writer, and the gate gave no answer in 60 s. A `HEAD` that +is neither a file nor a directory (a FIFO, a socket or a device, or a +symlink to one) is `member-unreadable`, naming it and what stands there, +before git is asked; each run over a FIFO is bounded, and a parity row +holds the line to the gate's. + WHAT IS ON DISK IS ASKED ONLY WHERE IT IS SAID (round 2, r956's SHOULD 1 on #956): `_forced` and `_staged_over` ask git first, and where HEAD records a gitlink at the index's own commit, the disk under it is never @@ -98,6 +106,9 @@ from test_validate_family_unnamed_mounts import ( codes, commands, findings, spare_commits, stage_gitlink, ) +from test_validate_pins_spelled_and_named import ( + FIFO_HEADS, NOT_HEADS, bounded, +) OTHER_ROW = f"members/{OTHER}" HAND = f"members/{SPARE}" @@ -1027,6 +1038,26 @@ def test_a_member_git_directory_head_whose_open_is_refused_is_refused( assert "member-commit-unresolvable" not in err +@pytest.mark.parametrize("make, reason", FIFO_HEADS) +@pytest.mark.parametrize("made,head", HEADS) +def test_a_member_fifo_head_is_refused_by_name_within_a_bound( + holder, request, made, head, make, reason): + """#1290, W1256 on #1263: the holder's own gate, the member's git + directory's `HEAD` a FIFO, or a symlink to one. #1256's guard does not + open it, but `git rev-parse` did, and waited for a writer: the gate gave + no answer in 60 s. It is `member-unreadable`, exit 2, naming that `HEAD` + and what stands there, with the advice for a cause that is no + permission, before git is asked.""" + extra = made(holder, request) + (holder / head).rename(holder / (head + "-aside")) + make(holder / head) + result = bounded(holder / "scripts" / "validate-family.py", holder, + *extra) + assert_refused(result.returncode, result.stderr, MEMBER_UNREADABLE, head, + reason) + assert "member-commit-unresolvable" not in result.stderr + + def test_the_member_tree_asked_alone_names_a_head_whole_outside( holder, request, monkeypatch): """`_check_member_tree` asked alone, the open of the `HEAD` refused: it @@ -1183,22 +1214,16 @@ def test_a_member_head_is_named_as_the_gate_names_a_leg_s( @pytest.mark.parametrize("own_directory", OWN_DIRECTORY) -@pytest.mark.parametrize("make", [ - pytest.param(lambda head: head.mkdir(), id="directory"), - pytest.param(lambda head: os.mkfifo(head), id="fifo", - marks=pytest.mark.skipif(not hasattr(os, "mkfifo"), - reason="no FIFO on this platform")), -]) -def test_a_member_head_that_is_no_regular_file_is_not_opened( - sources, tmp_path, monkeypatch, own_directory, make): +def test_a_member_head_that_is_a_directory_is_left_to_git( + sources, tmp_path, monkeypatch, own_directory): """Only a `HEAD` that is a regular file is opened (#1256), in the family - gate as in the leg gate (#1222): one that is anything else is git's to - answer for, and the open of a FIFO would wait for a writer. An open of - either fails the row rather than hanging it.""" + gate as in the leg gate (#1222), and one that is a directory is git's to + answer for, as it always was: the control for #1290's rows. An open + fails the row.""" gitdir = member_git_directory(tmp_path, own_directory) head = gitdir / "HEAD" head.rename(gitdir / "HEAD-aside") - make(head) + head.mkdir() failing(monkeypatch, "open", "HEAD", OSError(errno.EIO, "opened")) assert sources.family._refuse_an_unreadable_git_dir(tmp_path / ROW, ROW) is None @@ -1206,6 +1231,34 @@ def test_a_member_head_that_is_no_regular_file_is_not_opened( tmp_path, tmp_path / ROW / ".git") is None +@pytest.mark.parametrize("own_directory", OWN_DIRECTORY) +@pytest.mark.parametrize("make, reason", NOT_HEADS) +def test_a_member_head_git_cannot_read_is_named_as_the_gate_names_a_leg_s( + sources, tmp_path, monkeypatch, own_directory, make, reason): + """PARITY (#1290): a `HEAD` that `stat`, which follows a link, finds + neither a file nor a directory, a FIFO, a socket, or a symlink to a FIFO + or a device, is refused unopened, and the family gate's line is the leg + gate's, naming the same `HEAD` and what stands there. The guards are + asked alone, so that where one lets it through, the row fails and git + is never started on a FIFO.""" + gitdir = member_git_directory(tmp_path, own_directory) + head = gitdir / "HEAD" + head.rename(gitdir / "HEAD-aside") + make(head) + failing(monkeypatch, "open", "HEAD", OSError(errno.EIO, "opened")) + family_refusal = refusal_of( + sources.family, sources.family._refuse_an_unreadable_git_dir, + tmp_path / ROW, ROW) + gate_refusal = refusal_of( + sources.gate, sources.gate._refuse_an_unreadable_git_dir, tmp_path, + tmp_path / ROW / ".git") + named = head.relative_to(tmp_path).as_posix() + assert family_refusal.code == MEMBER_UNREADABLE + assert family_refusal.detail == f"{named}: cannot be read ({reason})" + assert gate_refusal.code == "pin-submodule-unreadable" + assert gate_refusal.detail == family_refusal.detail + + def test_a_member_head_no_system_call_can_name_is_left_to_git(sources, tmp_path): """A `gitdir: ` line that holds a NUL names a git directory no system diff --git a/tests/test_validate_pins_spelled_and_named.py b/tests/test_validate_pins_spelled_and_named.py index da69389d..24f97420 100644 --- a/tests/test_validate_pins_spelled_and_named.py +++ b/tests/test_validate_pins_spelled_and_named.py @@ -51,6 +51,16 @@ under THE LOCKSTEP RULE. It is opened as a gitfile is, and refused `pin-submodule-unreadable`, naming that `HEAD`. +What #1222 left, #1290, from RV1239's review of #1239: + + * A `HEAD` that is a FIFO is not opened by #1222's guard, but git's own + open of it waits for a writer, and `git_out` has no timeout, so the + gate gave no answer in 45 s. A `HEAD` that is neither a file nor a + directory (a FIFO, a socket or a device, or a symlink to one) is + refused `pin-submodule-unreadable` by name, before git is asked. Each + run of the gate over a FIFO is bounded (`bounded`), so a hang fails + its row and not the run. + THE HARNESS is `tests/test_validate_pins_named_and_spelled.py`'s: the gate runs as the copy a real scaffold wrote (`validate`), and in-process as that copy loaded as a module (`validator`). A stub `gh` is a `#!` script, so its @@ -68,6 +78,7 @@ import hashlib import json import os +import signal import socket import stat import subprocess @@ -76,7 +87,9 @@ import pytest -from conftest import REPO, WINDOWS_SKIP, run_script +from conftest import ( + REPO, WINDOWS_SKIP, blank_unnamed_pin_sources, run_script, +) from test_validate_pins_named_and_spelled import ( AGAIN, CHECKOUT_READS, COMMIT, OTHER, PERMISSION, READS, SHAPE_PIN, UNPRINTABLE, declare_openxdox, failing, os_error, refusal_of_the_run, @@ -977,22 +990,143 @@ def test_a_head_gone_when_it_is_opened_is_left_to_git(validator, project, assert report.findings == [] -@pytest.mark.parametrize("make", [ - pytest.param(made_directory, id="directory"), - pytest.param(made_fifo, id="fifo", marks=POSIX_NODES), -]) -def test_a_head_that_is_no_regular_file_is_not_opened(validator, tmp_path, - monkeypatch, make): - """Only a `HEAD` that is a regular file is opened: one that is anything - else is git's to answer for, and the open of a FIFO would wait for a - writer. An open of either fails the row rather than hanging it.""" +def test_a_head_that_is_a_directory_is_left_to_git(validator, tmp_path, + monkeypatch): + """Only a `HEAD` that is a regular file is opened, and one that is a + directory is git's to answer for, as it always was (#1222, #1290): the + control for #1290's rows below. An open fails the row.""" dotgit = tmp_path / "leg" / ".git" dotgit.mkdir(parents=True) - make(dotgit / "HEAD") + made_directory(dotgit / "HEAD") failing(monkeypatch, "open", {"HEAD": AssertionError("opened")}) assert validator._refuse_an_unreadable_git_dir(tmp_path, dotgit) is None +# --- #1290: a leg git directory's `HEAD` that git cannot read as one --------- + +#: How long one run of the gate may take before its row calls it hung +#: (#1290). The gate answers this project in a second or two; over a FIFO +#: `HEAD` it gave no answer in 45 s (RV1239) or in 60 s (W1256). +BOUND = 30 + + +def bounded(script: Path, root: Path, + *args: str) -> subprocess.CompletedProcess: + """`script` run in `root` as `run_script` runs a tool, but in a session + of its own and for `BOUND` seconds at most (#1290). A run that has not + ended by then is killed with every process in its session, the git it + waits in among them, so no git is left waiting on a FIFO, and the row + fails: a hang fails its row, never the whole run. POSIX only, as a + FIFO is.""" + env = blank_unnamed_pin_sources({**os.environ, "LANES_LANE": ""}, {}) + with subprocess.Popen( + [sys.executable, str(script), *args], cwd=str(root), + stdin=subprocess.DEVNULL, stdout=subprocess.PIPE, + stderr=subprocess.PIPE, env=env, start_new_session=True) as proc: + try: + out, err = proc.communicate(timeout=BOUND) + except subprocess.TimeoutExpired: + os.killpg(proc.pid, signal.SIGKILL) + proc.communicate() + pytest.fail(f"{script.name} gave no answer in {BOUND} s") + return subprocess.CompletedProcess( + proc.args, proc.returncode, out.decode("utf-8", "replace"), + err.decode("utf-8", "replace")) + + +def made_link_to_a_fifo(path: Path) -> None: + os.mkfifo(path.with_name(path.name + "-fifo")) + os.symlink(path.name + "-fifo", path) + + +def made_link_to_a_device(path: Path) -> None: + os.symlink(os.devnull, path) + + +#: What can stand where a git directory's `HEAD` should be that git cannot +#: read as one, and the reason the gate names it by (#1290). A device is +#: made only by root, so its row is a symlink to one. +NOT_HEADS = [ + pytest.param(made_fifo, "a FIFO, not a file", id="fifo", + marks=POSIX_NODES), + pytest.param(made_socket, "a socket, not a file", id="socket", + marks=POSIX_NODES), + pytest.param(made_link_to_a_fifo, "a symlink to a FIFO, not a file", + id="link-to-fifo", marks=POSIX_NODES), + pytest.param(made_link_to_a_device, "a symlink to a device, not a file", + id="link-to-device", marks=POSIX_NODES), +] + +#: The two of them git waits on. +FIFO_HEADS = NOT_HEADS[0::2] + + +def in_head_s_place(project: Path, own_directory: bool, head: str, + make) -> None: + """`make` where the leg's git directory's `HEAD` is, the `HEAD` itself + taken out of the way: the leg's own `.git` directory's, or the one + under `.git/modules/` its gitfile names.""" + if own_directory: + leg_with_its_own_git_directory(project) + (project / head).rename(project / (head + "-aside")) + make(project / head) + + +@pytest.mark.parametrize("own_directory, head", HEADS) +@pytest.mark.parametrize("make, reason", FIFO_HEADS) +def test_a_fifo_head_is_refused_by_name_within_a_bound( + project, own_directory, head, make, reason): + """#1290, RV1239 on #1239: the project's own gate, the leg's git + directory's `HEAD` a FIFO, or a symlink to one. #1222's guard does not + open it, but `git rev-parse` did, and waited for a writer: the gate gave + no answer. It is `pin-submodule-unreadable`, naming that `HEAD` and what + stands there, with the advice for a cause that is no permission, exit 2, + before git is asked, and nothing is written.""" + in_head_s_place(project, own_directory, head, make) + before = tree_bytes(project) + result = bounded(project / "scripts" / "validate-pins.py", project) + assert result.stderr == ( + f"REFUSED pin-submodule-unreadable: {head}: cannot be read " + f"({reason})\n{CHECKOUT_READS}{OTHER}{AGAIN}\n"), result.stderr + assert result.returncode == 2, result.stdout + result.stderr + assert tree_bytes(project) == before, "a validator writes nothing" + + +@pytest.mark.parametrize("own_directory, head", HEADS) +@pytest.mark.parametrize("make, reason", NOT_HEADS) +def test_a_head_git_cannot_read_is_refused_by_name_unopened( + validator, project, monkeypatch, own_directory, head, make, reason): + """#1290 in-process, each kind: a FIFO, a socket, and a symlink to a + FIFO or a device, `stat`, which follows a link, finds no file and no + directory. Each is refused by name, as a `HEAD` this user cannot open + is, and none is opened. The guard is asked alone, so that where it lets + one through, the row fails and git is never started on a FIFO.""" + in_head_s_place(project, own_directory, head, make) + failing(monkeypatch, "open", {"HEAD": AssertionError("opened")}) + with pytest.raises(validator.Refusal) as caught: + validator._refuse_an_unreadable_git_dir(project, + project / "spec" / ".git") + assert caught.value.code == "pin-submodule-unreadable" + assert caught.value.detail == f"{head}: cannot be read ({reason})" + assert caught.value.remediation == CHECKOUT_READS + OTHER + AGAIN + + +@POSIX_NODES +def test_the_root_s_own_fifo_head_is_refused_by_name_within_a_bound(project): + """#1290: the root's own `.git/HEAD` a FIFO. `_refuse_an_unreadable_root` + asks the root's `.git` what it asks a leg's (#1146), so it is + `git-repo-unreadable`, naming `.git/HEAD`, where git waited on it and + the gate gave no answer.""" + head = project / ".git" / "HEAD" + head.rename(project / ".git" / "HEAD-aside") + made_fifo(head) + result = bounded(project / "scripts" / "validate-pins.py", project) + assert result.stderr.splitlines()[0] == ( + "REFUSED git-repo-unreadable: .git/HEAD: cannot be read " + "(a FIFO, not a file)"), result.stderr + assert result.returncode == 2, result.stdout + result.stderr + + def test_a_head_no_system_call_can_name_is_left_to_git(validator, tmp_path): """A `gitdir: ` line that holds a NUL names a git directory no system call can take, so its `HEAD` is nothing there, as `_is_there` answers a