From 543ffbb08312175236dfe16a1539e7f667d7dfd2 Mon Sep 17 00:00:00 2001 From: Steven Dee <262429907+stevendee-recon@users.noreply.github.com> Date: Thu, 8 Oct 2026 15:45:02 -0700 Subject: [PATCH] fix(toolchains): exclude pycache symlinks from runtime files The runtime's `lib/**` glob matches the `__pycache__` symlinks added in #3661, so they end up in runfiles. With `RULES_PYTHON_PYCACHE_DIR=/dev/null` they are symlinks to a character device, and packaging a binary into a tar layer fails (#4198). The glob now excludes `__pycache__` and everything under it. The pyc cache is now keyed on the runtime archive's sha256 rather than a hash of the workspace root. The sources in an archive all share one mtime, so Python's mtime-and-size check can't tell a patch release's sources from the previous one's. The workspace hash didn't keep workspaces apart anyway: the repository contents cache shares one fetched runtime, symlinks included, across workspaces. The default root stops trying locations that are likely to be shared and instead tries a more reliably user-owned fallback: if `XDG_CACHE_HOME` is unset, then on non-Windows, `$HOME/.cache` is used (per [XDG][0]). On Windows, `%LOCALAPPDATA%` is used. (NB. [`platformdirs`][1] would choose `rules_python\Cache` rather than `rules_python_pycache`; the current change opts for one directory name everywhere.) [0]: https://specifications.freedesktop.org/basedir/latest/#variables [1]: https://platformdirs.readthedocs.io/en/latest/ --- docs/environment-variables.md | 13 +++++-- news/4198.changed.md | 3 ++ news/4198.fixed.md | 4 ++ python/private/python_repository.bzl | 46 ++++++++--------------- tests/integration/custom_commands_test.py | 11 ++++++ 5 files changed, 43 insertions(+), 34 deletions(-) create mode 100644 news/4198.changed.md create mode 100644 news/4198.fixed.md diff --git a/docs/environment-variables.md b/docs/environment-variables.md index 3eb62221c0..cb26840c2a 100644 --- a/docs/environment-variables.md +++ b/docs/environment-variables.md @@ -98,7 +98,7 @@ be stored in. This directory may be reused between invocations, depending on the sandboxing configuration. Setting it to `/dev/null` will, in effect, disable runtime pyc caching. By setting e.g. -`--sandbox_add_mount_pair=/tmp/rules_python_pycache`, it's possible for pyc +`--sandbox_add_mount_pair=$HOME/.cache/rules_python_pycache`, it's possible for pyc caching to persist across invocations. **Behavior specific to downloaded runtimes:** @@ -109,13 +109,18 @@ Otherwise, the following environment variables are checked in the following order. Their values will have `rules_python_pycache` appended to them to form the root pycache directory: 1. `XDG_CACHE_HOME`. -2. `TMP` (non-Windows) or `TEMP` (Windows). -3. The common platform-specific temporary directory (`/tmp` (non-Windows) or - `C:\Temp` (Windows)). +2. `$HOME/.cache` (non-Windows), the XDG default when `XDG_CACHE_HOME` is + unset, or `LOCALAPPDATA` (Windows). If such a diretory cannot be found, or created, then `/dev/null` will be used, which will effectively disable pyc caching. +:::{versionchanged} VERSION_NEXT_PATCH +Temporary directories (`TMP`, `TEMP`, `/tmp` and `C:\Temp`) are no longer used. +They get cleaned out while the fetched runtime persists, which left the +`__pycache__` symlinks dangling. +::: + ::: :::{envvar} RULES_PYTHON_PYPI_HUB_RESERVED diff --git a/news/4198.changed.md b/news/4198.changed.md new file mode 100644 index 0000000000..b91cabe42d --- /dev/null +++ b/news/4198.changed.md @@ -0,0 +1,3 @@ +(toolchains) Downloaded runtimes cache pyc files under `$HOME/.cache` +(`LOCALAPPDATA` on Windows) instead of a temporary directory when +`XDG_CACHE_HOME` is unset. diff --git a/news/4198.fixed.md b/news/4198.fixed.md new file mode 100644 index 0000000000..bed52870bf --- /dev/null +++ b/news/4198.fixed.md @@ -0,0 +1,4 @@ +(toolchains) `__pycache__` symlinks in downloaded runtimes no longer end up in +runfiles, and a runtime no longer loads pyc files compiled from another +version's sources +([#4198](https://github.com/bazel-contrib/rules_python/issues/4198)). diff --git a/python/private/python_repository.bzl b/python/private/python_repository.bzl index 3fd0928a01..56b4e06536 100644 --- a/python/private/python_repository.bzl +++ b/python/private/python_repository.bzl @@ -63,38 +63,24 @@ def _get_pycache_root(rctx): os_name = repo_utils.get_platforms_os_name(rctx) is_windows = os_name == "windows" - # 1. RULES_PYTHON_PYCACHE_DIR + # Python checks a pyc file against its source's mtime and size, and the + # sources in a runtime's archive all share one mtime. Keying on the archive + # keeps a runtime from loading pyc files compiled from another's sources. + suffix = "{}/{}".format(rctx.name, rctx.attr.sha256[:16] or rctx.attr.python_version) + res = rctx.getenv("RULES_PYTHON_PYCACHE_DIR") if res: - res = res + "/" + rctx.name + res = res + "/" + suffix return repo_utils.mkdir(rctx, res) - # Suffix for cases 2-4 - # The first level directory is static and documented so that it is easy to - # use with e.g. --sandbox_add_mount_pair=/tmp/rules_python_pycache - suffix = "rules_python_pycache/{}/{}".format(hash(str(rctx.workspace_root)), rctx.name) - - # 2. XDG_CACHE_HOME res = rctx.getenv("XDG_CACHE_HOME") - if res: - path = repo_utils.mkdir(rctx, rctx.path(res).get_child(suffix)) - if path: - return path - - # 3. TMP or TEMP - res = rctx.getenv("TMP") or rctx.getenv("TEMP") - if res: - path = repo_utils.mkdir(rctx, rctx.path(res).get_child(suffix)) - if path: - return path - - # 4. /tmp or Windows equivalent - if is_windows: - path = rctx.path("C:/Temp").get_child(suffix) - else: - path = rctx.path("/tmp").get_child(suffix) - - return repo_utils.mkdir(rctx, path) + if not res: + res = rctx.getenv("LOCALAPPDATA" if is_windows else "HOME") + if res and not is_windows: + res += "/.cache" + if not res: + return None + return repo_utils.mkdir(rctx, rctx.path(res).get_child("rules_python_pycache").get_child(suffix)) def _create_pycache_symlinks(rctx, logger): """Finds all directories with a .py file and creates __pycache__ symlinks. @@ -252,9 +238,9 @@ def _python_repository_impl(rctx): # Exclude them from the glob because otherwise between the first time and second time a python toolchain is used," # the definition of this filegroup will change, and depending rules will get invalidated." # See https://github.com/bazel-contrib/rules_python/issues/1008 for unconditionally adding these to toolchains so we can stop ignoring them." - # pyc* is ignored because pyc creation creates temporary .pyc.NNNN files - "**/__pycache__/*.pyc*", - "**/__pycache__/*.pyo*", + # The __pycache__ directories themselves are symlinks out of the repository. + "**/__pycache__", + "**/__pycache__/**", ] if "windows" in platform: diff --git a/tests/integration/custom_commands_test.py b/tests/integration/custom_commands_test.py index 336937ece0..2b705d9228 100644 --- a/tests/integration/custom_commands_test.py +++ b/tests/integration/custom_commands_test.py @@ -28,6 +28,17 @@ def test_run_build_python_zip_false(self): ) self.assert_result_matches(result, "bazel-out") + # Regression test for https://github.com/bazel-contrib/rules_python/issues/4198 + def test_runtime_files_exclude_pycache_symlinks(self): + result = self.run_bazel( + "cquery", + "--repo_env=RULES_PYTHON_PYCACHE_DIR=/dev/null", + "--output=files", + "deps(//:bin)", + ) + pycache = [f for f in result.stdout.splitlines() if "__pycache__" in f] + self.assertEqual(pycache, []) + if __name__ == "__main__": # Enabling this makes the runner log subprocesses as the test goes along.