Skip to content

Commit

Permalink
Preserve file permissions when extracting runfiles zip
Browse files Browse the repository at this point in the history
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
  • Loading branch information
brandjon authored and copybara-github committed May 1, 2019
1 parent da6a7f2 commit 1b324c4
Show file tree
Hide file tree
Showing 2 changed files with 70 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -99,13 +99,38 @@ def FindModuleSpace():

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):
Expand Down
41 changes: 41 additions & 0 deletions src/test/shell/bazel/python_version_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,47 @@ EOF
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

Expand Down

0 comments on commit 1b324c4

Please sign in to comment.