fix: correctly invalidate treeartifacts

Author: thesayyn

.github/workflows/ci.yaml

@@ -28,6 +28,40 @@ jobs:
       - uses: actions/checkout@v4
       - uses: bazel-contrib/setup-bazel@0.15.0
       - run: bazel test //...
+  # Theory 1: verify that changing a TreeArtifact's *content* (same file paths)
+  # invalidates the tar action's cache when compute_unused_inputs is enabled.
+  #
+  # The test builds //tar/tests:tar_theory1 twice with a shared --disk_cache.
+  # Between the two builds it rewrites tar/tests/whl_module.py so that the
+  # theory1_whl_install TreeArtifact has different content.  If whl_install is
+  # incorrectly pruned from the tar action's cache key the archive will be a
+  # stale cache hit; otherwise the archive must change to reflect the new content.
+  cache_invalidation_test:
+    strategy:
+      matrix:
+        os:
+          - macos-latest
+          - ubuntu-latest
+    runs-on: ${{ matrix.os }}
+    steps:
+      - uses: actions/checkout@v4
+      - uses: bazel-contrib/setup-bazel@0.15.0
+      - name: Build v1 (populate disk cache)
+        run: |
+          echo "# whl content v1" > tar/tests/whl_module.py
+          bazel build //tar/tests:tar_theory1 --disk_cache=/tmp/theory1_cache
+          cp bazel-bin/tar/tests/theory1.tar /tmp/theory1_v1.tar
+      - name: Change whl content and rebuild
+        run: |
+          echo "# whl content v2 — different from v1" > tar/tests/whl_module.py
+          bazel build //tar/tests:tar_theory1 --disk_cache=/tmp/theory1_cache
+      - name: Verify archive changed (cache correctly invalidated)
+        run: |
+          if cmp -s bazel-bin/tar/tests/theory1.tar /tmp/theory1_v1.tar; then
+            echo "FAIL: archive did not change after whl content change"
+            echo "      theory1_whl_install was incorrectly pruned from the tar cache key"
+            exit 1
+          fi
   pre-commit:
     runs-on: ubuntu-latest
     steps:
@@ -36,7 +70,7 @@ jobs:
   # For branch protection settings, this job provides a "stable" name that can be used to gate PR merges
   # on "all matrix jobs were successful".
   conclusion:
-    needs: [test, pre-commit]
+    needs: [test, cache_invalidation_test, pre-commit]
     runs-on: ubuntu-latest
     if: always()
     steps:

tar/tests/BUILD

@@ -8,6 +8,7 @@ load("@tar.bzl", "mtree_mutate", "mtree_spec", "tar")
 load(":asserts.bzl", "assert_tar_listing", "assert_unused_listing")
 load(":directory.bzl", "directory")
 load(":node_modules_tree.bzl", "node_modules_tree")
+load(":cross_linking_tree.bzl", "venv_binary", "venv_binary_independent", "whl_install", "whl_install_from_file")
 load(":runfiles_symlinks.bzl", "runfiles_symlinks")
 
 # The examples below work with both source files and generated files.
@@ -827,3 +828,133 @@ assert_tar_listing(
         "-rw-r--r--  0 0      0          21 Jan  1  2023 src_file",
     ],
 )
+
+#############
+# Example 22: compute_unused_inputs with directory-level cross-TreeArtifact symlinks.
+#
+# node_modules_tree produces TreeArtifacts where dep_symlink_b_to_a and node_modules_a
+# are directory-level symlinks to store_a.  Both the symlink directories AND store_a
+# itself are in e17_binary's runfiles, so the mtree contains content= entries for
+# store_a's files directly.  The pruner therefore correctly does not prune store_a.
+#
+# This is a regression test: it verifies that directory-level symlink TreeArtifacts
+# in runfiles are handled correctly and nothing is incorrectly pruned.
+
+mtree_spec(
+    name = "mtree22",
+    srcs = [":e17_binary"],
+)
+
+tar(
+    name = "tar22",
+    srcs = [":e17_binary"],
+    out = "22.tar",
+    compute_unused_inputs = 1,
+    mtree = ":mtree22",
+)
+
+# store_a backing files must NOT appear in unused_inputs; they are needed to
+# resolve dep_symlink_b_to_a → store_a and node_modules_a → store_a at runtime.
+assert_unused_listing(
+    name = "test22_cross_dir_symlink_treeartifact_not_pruned",
+    actual = ":tar22",
+    expected = [],
+)
+
+#############
+# Example 23: compute_unused_inputs with file-level cross-TreeArtifact symlinks.
+#
+# cross_linking_tree produces a binary whose runfiles contain:
+#   - *_whl_install: a real TreeArtifact with module.py
+#   - *_venv: a TreeArtifact containing lib/module.py as a file-level symlink into whl_install
+#
+# The mtree content= paths reference files under *_venv/ (e.g. venv/lib/module.py).
+# The *_whl_install artifact path (whl_install/module.py) never appears in any mtree
+# content=, so the join marks whl_install files as unused.  At runtime bsdtar
+# dereferences venv/lib/module.py → whl_install/module.py and fails if whl_install
+# has been pruned from the sandbox.
+#
+# This test is expected to FAIL until the pruner is fixed.
+
+whl_install(name = "e23_whl_install")
+
+venv_binary(
+    name = "e23_venv_binary",
+    whl_install = ":e23_whl_install",
+)
+
+# mtree is derived only from e23_venv_binary's runfiles.  The venv contains
+# file-level symlinks into e23_whl_install, so the mtree content= paths are
+# e23_venv_binary_venv/lib/module.py (the symlink path), never the backing
+# e23_whl_install/module.py path.  The pruner therefore marks e23_whl_install
+# files as unused even though bsdtar must dereference the symlinks at
+# archive-creation time.
+
+mtree_spec(
+    name = "mtree23",
+    srcs = [":e23_venv_binary"],
+)
+
+tar(
+    name = "tar23",
+    srcs = [
+        ":e23_venv_binary",
+        ":e23_whl_install",  # needed by bsdtar via symlink dereference, not in mtree
+    ],
+    out = "23.tar",
+    compute_unused_inputs = 1,
+    mtree = ":mtree23",
+)
+
+# e23_whl_install files must NOT appear in unused_inputs even though no mtree
+# content= directly names them: bsdtar dereferences
+#   e23_venv_binary_venv/lib/module.py → e23_whl_install/module.py
+# at archive-creation time and fails if e23_whl_install is absent from the sandbox.
+assert_unused_listing(
+    name = "test23_file_level_cross_treeartifact_not_pruned",
+    actual = ":tar23",
+    expected = [],
+)
+
+#############
+# Theory 1 targets: used by the CI cache-invalidation test.
+#
+# whl_install_from_file reads whl_module.py so the CI step can change the file's
+# content between two `bazel build` invocations (using --disk_cache) and verify
+# that the archive is rebuilt.  If whl_install is incorrectly pruned from the tar
+# action's cache key, the archive will be stale after the content change.
+
+whl_install_from_file(
+    name = "theory1_whl_install",
+    srcs = ["whl_module.py"],
+    tags = ["manual"],
+)
+
+# venv_binary_independent: the venv action does NOT depend on whl_install at
+# the Bazel level, so changing whl_module.py leaves the venv digest unchanged.
+# Only theory1_whl_install changes, which is exactly what we need: if
+# whl_install is incorrectly in unused_inputs_list, the tar's reduced cache
+# key is identical to the previous build and Bazel serves a stale archive.
+venv_binary_independent(
+    name = "theory1_venv_binary",
+    whl_name = "theory1_whl_install",
+    tags = ["manual"],
+)
+
+mtree_spec(
+    name = "mtree_theory1",
+    srcs = [":theory1_venv_binary"],
+    tags = ["manual"],
+)
+
+tar(
+    name = "tar_theory1",
+    srcs = [
+        ":theory1_venv_binary",
+        ":theory1_whl_install",  # needed by bsdtar via symlink dereference, not in mtree
+    ],
+    out = "theory1.tar",
+    compute_unused_inputs = 1,
+    mtree = ":mtree_theory1",
+    tags = ["manual"],
+)

tar/tests/cross_linking_tree.bzl

@@ -0,0 +1,135 @@
+"""
+Fixtures that model the py_console_script_binary / venv→whl_install pattern.
+
+Two rules are exported:
+
+  whl_install  — a standalone TreeArtifact containing real files.
+
+  venv_binary  — an executable whose runfiles contain a *_venv TreeArtifact with
+                 file-level RELATIVE symlinks into the whl_install TreeArtifact.
+                 Critically, whl_install is NOT placed in venv_binary's runfiles;
+                 it is only needed by the binary's action so the symlinks are valid.
+
+Usage in tests:
+
+    whl_install(name = "my_whl")
+    venv_binary(name = "my_venv_bin", whl_install = ":my_whl")
+
+    # whl_install must be a separate tar src so it is available to bsdtar
+    # (venv symlinks into it), but it is not covered by the mtree because the
+    # mtree is derived only from venv_binary's runfiles.  The pruner therefore
+    # marks whl_install files as unused even though bsdtar needs them to
+    # dereference the symlinks inside venv.
+    tar(
+        name = "my_tar",
+        srcs = [":my_venv_bin", ":my_whl"],
+        mtree = mtree_spec([":my_venv_bin"]),
+        compute_unused_inputs = 1,
+    )
+"""
+
+def _whl_install_impl(ctx):
+    tree = ctx.actions.declare_directory(ctx.label.name)
+    ctx.actions.run_shell(
+        outputs = [tree],
+        command = "echo 'module' > {d}/module.py".format(d = tree.path),
+    )
+    return [DefaultInfo(files = depset([tree]))]
+
+whl_install = rule(implementation = _whl_install_impl)
+
+def _whl_install_from_file_impl(ctx):
+    """Like whl_install but copies content from a source file.
+
+    Allows CI to change the whl content between builds (same TreeArtifact
+    structure, different file content) to verify cache-invalidation behaviour.
+    """
+    tree = ctx.actions.declare_directory(ctx.label.name)
+    src = ctx.files.srcs[0]
+    ctx.actions.run_shell(
+        inputs = [src],
+        outputs = [tree],
+        command = "cp {src} {d}/module.py".format(src = src.path, d = tree.path),
+    )
+    return [DefaultInfo(files = depset([tree]))]
+
+whl_install_from_file = rule(
+    implementation = _whl_install_from_file_impl,
+    attrs = {"srcs": attr.label_list(allow_files = True, mandatory = True)},
+)
+
+def _venv_binary_impl(ctx):
+    executable = ctx.actions.declare_file(ctx.label.name)
+    ctx.actions.write(executable, "#!/bin/bash\necho hello", is_executable = True)
+
+    whl = ctx.file.whl_install
+
+    # Build a relative symlink path from inside venv/lib/ up to the whl_install
+    # sibling directory.  The two TreeArtifacts sit side-by-side in the output
+    # tree, so ../../<whl_basename>/module.py resolves correctly.
+    whl_basename = whl.path.split("/")[-1]
+    venv = ctx.actions.declare_directory(ctx.label.name + "_venv")
+    ctx.actions.run_shell(
+        inputs = [whl],
+        outputs = [venv],
+        command = "mkdir -p {v}/lib && ln -s ../../{whl}/module.py {v}/lib/module.py".format(
+            v = venv.path,
+            whl = whl_basename,
+        ),
+    )
+
+    # Only venv goes into runfiles — whl_install is intentionally omitted so
+    # that the mtree (derived from this binary's runfiles) does not contain any
+    # content= path that names a whl_install file directly.
+    return [DefaultInfo(
+        executable = executable,
+        runfiles = ctx.runfiles(files = [venv]),
+    )]
+
+venv_binary = rule(
+    implementation = _venv_binary_impl,
+    attrs = {
+        "whl_install": attr.label(allow_single_file = True),
+    },
+    executable = True,
+)
+
+def _venv_binary_independent_impl(ctx):
+    """Like venv_binary but the venv action does NOT depend on whl_install.
+
+    The venv receives the whl_install NAME as a string attribute so the action
+    command is fully determined at analysis time without any file input.  This
+    means that when whl_install content changes, the venv action's cache key
+    (command + empty inputs) is unchanged, so the venv stays cached and its
+    digest is unchanged.  Only the tar action's whl_install input changes,
+    which is exactly the condition needed to demonstrate the stale disk-cache
+    entry: if whl_install is incorrectly in unused_inputs_list, the tar's
+    reduced cache key (excluding whl_install) matches the previous entry and
+    Bazel serves a stale archive even though the whl content changed.
+    """
+    executable = ctx.actions.declare_file(ctx.label.name)
+    ctx.actions.write(executable, "#!/bin/bash\necho hello", is_executable = True)
+
+    venv = ctx.actions.declare_directory(ctx.label.name + "_venv")
+    ctx.actions.run_shell(
+        # No inputs declared — the venv action must stay cached when whl changes.
+        inputs = [],
+        outputs = [venv],
+        command = "mkdir -p {v}/lib && ln -sf ../../{whl}/module.py {v}/lib/module.py".format(
+            v = venv.path,
+            whl = ctx.attr.whl_name,
+        ),
+    )
+
+    return [DefaultInfo(
+        executable = executable,
+        runfiles = ctx.runfiles(files = [venv]),
+    )]
+
+venv_binary_independent = rule(
+    implementation = _venv_binary_independent_impl,
+    # whl_name is just the directory basename string so the action has no file
+    # dependency on whl_install and its cache key remains stable across whl changes.
+    attrs = {"whl_name": attr.string(mandatory = True)},
+    executable = True,
+)

tar/tests/whl_module.py

@@ -0,0 +1 @@
+# whl content v1