diff --git a/test/pgc_fingerprint.py b/test/pgc_fingerprint.py index d170ca86..a54ea012 100755 --- a/test/pgc_fingerprint.py +++ b/test/pgc_fingerprint.py @@ -83,7 +83,20 @@ def build_dirs(root): Makefile reaches a separately built module such as objstore/. """ root = pathlib.Path(root) - out = [root / "src"] + # src is a candidate like any other, and gets the SAME symlink test. It used + # to be added unconditionally, which made it the one directory exempt from + # the rule stated four lines below: `find -P` does not descend a symlinked + # directory argument, so the shell this replaced hashed nothing there while + # this module walked it. Measured on a tree whose src/ was a symlink -- + # old 9861a3f1fbd1, module 9eff36abd48e -- so a stamp written before the + # port read `stale` against a clean tree. + out = [] + try: + src = root / "src" + if src.is_dir() and not src.is_symlink(): + out.append(src) + except OSError: + pass try: entries = sorted(root.iterdir(), key=lambda p: os.fsencode(str(p))) except OSError: diff --git a/test/pytest/TESTS.md b/test/pytest/TESTS.md index 24e628cb..f759f5b3 100644 --- a/test/pytest/TESTS.md +++ b/test/pytest/TESTS.md @@ -654,6 +654,14 @@ recognises a write at `$PGC_SRCDIR`, that it recognises one through `$_bd_root`, and that a write into a COPY is *not* flagged — because a pattern that matches nothing would otherwise pass this arm silently. +`test_a_symlinked_src_is_skipped_like_any_other_symlinked_build_dir` closes the +one directory that was exempt from the module's own rule. `build_dirs()` added +`root/"src"` unconditionally and applied the symlink test to every other +candidate, so a tree whose `src/` is a symlink hashed differently across the +port — `find -P` does not descend a symlinked directory argument, so the shell +hashed nothing there while the module walked it. It carries a control, because +"skip src entirely" would satisfy the arm without it. + **What is still not guarded**, named here rather than left for someone to find: a TRUNCATED manifest — `find` returning fewer files rather than none — would produce a plausible wrong hash that neither the per-file sentinel nor the empty-manifest diff --git a/test/pytest/test_build_refusal.py b/test/pytest/test_build_refusal.py index ddd16a4e..42523ecf 100644 --- a/test/pytest/test_build_refusal.py +++ b/test/pytest/test_build_refusal.py @@ -915,3 +915,51 @@ def test_one_tree_hashes_one_way_however_the_locale_is_set(expect): f"one tree, one fingerprint, across {len(wanted)} locales: {seen}") finally: shutil.rmtree(root, ignore_errors=True) + + +def test_a_symlinked_src_is_skipped_like_any_other_symlinked_build_dir(tmp_path, expect): + """`src` was the one directory exempt from the module's own symlink rule. + + `build_dirs()` added `root/"src"` unconditionally and then applied + `if not d.is_dir() or d.is_symlink(): continue` to every other candidate. + `find -P` does not descend a symlinked directory argument, so the shell this + module replaced hashed nothing there while the module walked it. Measured on + a tree whose src/ was a symlink: + + find -P on the symlinked src/ printed nothing + the module's manifest src/a.c, src/h.h + old shell fingerprint 9861a3f1fbd1 + module fingerprint 9eff36abd48e + + A stamp written before the port then read `stale` on a clean tree, which is + the false FATAL the controller exists to prevent. + """ + real = tmp_path / "real" + real.mkdir() + (real / "a.c").write_text("int a;\n") + (real / "h.h").write_text("void h(void);\n") + t = tmp_path / "tree" + t.mkdir() + (t / "src").symlink_to(real) + (t / "Makefile").write_text("all:\n\ttrue\n") + (t / "pgcolumnar.control").write_text("x\n") + + expect.text(str((t / "src").is_symlink()), "True", "premise: src really is a symlink") + expect.num(len(list(real.iterdir())), 2, + "premise: and the target holds sources find would otherwise hash") + + out, _ = _sh_fp(f'pgc_source_manifest "{t}"') + lines = [l for l in out.splitlines() if l.strip()] + expect.num(len([l for l in lines if l.startswith("src/")]), 0, + "a symlinked src contributes nothing, as find -P contributes nothing") + expect.num(len(lines), 2, "so the tree fingerprints from its root files alone") + + # CONTROL: without this, "skip src entirely" would satisfy the arm above. + real_tree = tmp_path / "realsrc" + (real_tree / "src").mkdir(parents=True) + (real_tree / "src" / "a.c").write_text("int a;\n") + (real_tree / "Makefile").write_text("all:\n\ttrue\n") + (real_tree / "pgcolumnar.control").write_text("x\n") + out2, _ = _sh_fp(f'pgc_source_manifest "{real_tree}"') + expect.num(len([l for l in out2.splitlines() if l.startswith("src/a.c ")]), 1, + "control: a real src directory is still hashed") diff --git a/test/selftest/340-the-binary-must-be-built-from.sh b/test/selftest/340-the-binary-must-be-built-from.sh index 18ed61c2..ad34dd2a 100644 --- a/test/selftest/340-the-binary-must-be-built-from.sh +++ b/test/selftest/340-the-binary-must-be-built-from.sh @@ -775,6 +775,56 @@ check "an unhashable tree has an empty manifest" \ unset _mf _mf_lines _mf_hollow +# --------------------------------------------------------------------------- +# A SYMLINKED src/ MUST BE SKIPPED, LIKE EVERY OTHER SYMLINKED BUILD DIRECTORY. +# +# `build_dirs()` adds `root/"src"` unconditionally and then applies +# `if not d.is_dir() or d.is_symlink(): continue` to every OTHER candidate -- +# so `src` is the one directory that bypasses its own rule. The shell this +# replaced ran `find "$d" -maxdepth 1 -type f`, and `find -P` does not descend a +# symlinked command-line argument, so it hashed nothing there. +# +# Measured on a tree whose src/ is a symlink: +# +# find -P on the symlinked src/ printed nothing +# the module's manifest src/a.c, src/h.h +# old shell fingerprint 9861a3f1fbd1 +# module fingerprint 9eff36abd48e <- diverges +# +# A stamp written before the port then reads `stale` on a tree that is clean, +# which is the false FATAL the whole controller exists to prevent. The module's +# own comment states the invariant it breaks here. + +_sl="$(mktemp -d "${TMPDIR:-/tmp}/pgc-symsrc.XXXXXX")" +mkdir -p "$_sl/real" "$_sl/tree" +printf 'int a;\n' > "$_sl/real/a.c" +printf 'void h(void);\n' > "$_sl/real/h.h" +ln -s "$_sl/real" "$_sl/tree/src" +printf 'all:\n\ttrue\n' > "$_sl/tree/Makefile" +printf 'x\n' > "$_sl/tree/pgcolumnar.control" + +check "PREMISE the fixture's src really is a symlink" \ + "$([ -L "$_sl/tree/src" ] && echo yes || echo no)" "yes" +check "PREMISE and the target really holds sources find would otherwise hash" \ + "$(ls "$_sl/real" | tr '\n' ' ')" "a.c h.h " + +# find -P is the reference: it prints nothing for a symlinked directory argument. +check "a symlinked src contributes nothing, as find -P contributes nothing" \ + "$(pgc_source_manifest "$_sl/tree" | grep -c '^src/')" "0" +check "so the tree still fingerprints from its root files alone" \ + "$(pgc_source_manifest "$_sl/tree" | wc -l)" "2" + +# CONTROL: a REAL src directory must still be hashed, or the fix is "skip src". +_slr="$(mktemp -d "${TMPDIR:-/tmp}/pgc-realsrc.XXXXXX")" +mkdir -p "$_slr/src" +printf 'int a;\n' > "$_slr/src/a.c" +printf 'all:\n\ttrue\n' > "$_slr/Makefile" +printf 'x\n' > "$_slr/pgcolumnar.control" +check "control: a real src directory is still hashed" \ + "$(pgc_source_manifest "$_slr" | grep -c '^src/a.c ')" "1" + +unset _sl _slr + # The FATAL path's dump, driven rather than grepped for. A report nobody can run # is a report nobody knows is empty, and "the source says it calls it" is the kind # of claim this suite exists to refuse.