Preserve file permissions when extracting runfiles zip
In Python rules, the Python stub script is responsible for extracting the runfiles from the zip file when --build_python_zip is used. However, there's a bug in the standard Python library (https://bugs.python.org/issue15795) that causes file permissions to be lost when extracting zips. This causes runfiles to not be marked executable, which is a problem when the py_runtime is an in-build runtime (issue #5104).
This change updates the stub script to workaround the bug by directly chmodding extracted files with their intended mode. It also adds a regression test combining a custom py_runtime with --build_python_zip.
Fixes #5104, unblocks #7375.
RELNOTES: Fixed an issue where some `py_runtime`s were incompatible with using `--build_python_zip` (#5104).
PiperOrigin-RevId: 246195931
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/python/python_stub_template.txt b/src/main/java/com/google/devtools/build/lib/bazel/rules/python/python_stub_template.txt
index dac21c9..a6c5110 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/rules/python/python_stub_template.txt
+++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/python/python_stub_template.txt
@@ -99,13 +99,38 @@
raise AssertionError('Cannot find .runfiles directory for %s' % sys.argv[0])
+def ExtractZip(zip_path, dest_dir):
+ """Extracts the contents of a zip file, preserving the unix file mode bits.
+
+ These include the permission bits, and in particular, the executable bit.
+
+ Ideally the zipfile module should set these bits, but it doesn't. See:
+ https://bugs.python.org/issue15795.
+
+ Args:
+ zip_path: The path to the zip file to extract
+ dest_dir: The path to the destination directory
+ """
+ zip_path = GetWindowsPathWithUNCPrefix(zip_path)
+ dest_dir = GetWindowsPathWithUNCPrefix(dest_dir)
+ with zipfile.ZipFile(zip_path) as zf:
+ for info in zf.infolist():
+ zf.extract(info, dest_dir)
+ # UNC-prefixed paths must be absolute/normalized. See
+ # https://docs.microsoft.com/en-us/windows/desktop/fileio/naming-a-file#maximum-path-length-limitation
+ file_path = os.path.abspath(os.path.join(dest_dir, info.filename))
+ # The Unix st_mode bits (see "man 7 inode") are stored in the upper 16
+ # bits of external_attr. Of those, we set the lower 12 bits, which are the
+ # file mode bits (since the file type bits can't be set by chmod anyway).
+ attrs = info.external_attr >> 16
+ if attrs != 0: # Rumor has it these can be 0 for zips created on Windows.
+ os.chmod(file_path, attrs & 0o7777)
+
# Create the runfiles tree by extracting the zip file
def CreateModuleSpace():
- ZIP_RUNFILES_DIRECTORY_NAME = "runfiles"
temp_dir = tempfile.mkdtemp("", "Bazel.runfiles_")
- zf = zipfile.ZipFile(GetWindowsPathWithUNCPrefix(os.path.dirname(__file__)))
- zf.extractall(GetWindowsPathWithUNCPrefix(temp_dir))
- return os.path.join(temp_dir, ZIP_RUNFILES_DIRECTORY_NAME)
+ ExtractZip(os.path.dirname(__file__), temp_dir)
+ return os.path.join(temp_dir, "runfiles")
# Returns repository roots to add to the import path.
def GetRepositoriesImports(module_space, import_all):
diff --git a/src/test/shell/bazel/python_version_test.sh b/src/test/shell/bazel/python_version_test.sh
index 22c2e22..e463f4d 100755
--- a/src/test/shell/bazel/python_version_test.sh
+++ b/src/test/shell/bazel/python_version_test.sh
@@ -188,6 +188,47 @@
expect_log "File contents: abcdefg"
}
+# Regression test for #5104. This test ensures that it's possible to use
+# --build_python_zip in combination with a py_runtime (as opposed to not using
+# a py_runtime, i.e., the legacy --python_path mechanism).
+#
+# Note that with --incompatible_use_python_toolchains flipped, we're always
+# using a py_runtime, so in that case this amounts to a test that
+# --build_python_zip works at all.
+#
+# The specific issue #5104 was caused by file permissions being lost when
+# unzipping runfiles, which led to an unexecutable runtime.
+function test_build_python_zip_works_with_py_runtime() {
+ mkdir -p test
+
+ cat > test/BUILD << EOF
+py_binary(
+ name = "pybin",
+ srcs = ["pybin.py"],
+)
+
+py_runtime(
+ name = "mock_runtime",
+ interpreter = ":mockpy.sh",
+ python_version = "PY3",
+)
+EOF
+ cat > test/pybin.py << EOF
+# This doesn't actually run because we use a mock Python runtime that never
+# executes the Python code.
+print("I am pybin!")
+EOF
+ cat > test/mockpy.sh <<EOF
+#!/bin/bash
+echo "I am mockpy!"
+EOF
+ chmod u+x test/mockpy.sh
+
+ bazel run //test:pybin --python_top=//test:mock_runtime --build_python_zip \
+ &> $TEST_log || fail "bazel run failed"
+ expect_log "I am mockpy!"
+}
+
function test_pybin_can_have_different_version_pybin_as_data_dep() {
mkdir -p test
