bazelbuild / rules_python

Bazel Python Rules
https://rules-python.readthedocs.io
Apache License 2.0
524 stars 539 forks source link

py_binary/py_test runfiles inconsistent and broken on Windows #1870

Open sputt opened 5 months ago

sputt commented 5 months ago

🐞 bug report

Affected Rule

py_binary, py_test

Is this a regression?

Not sure

Description

With an empty .bazelrc in Bazel 7.1.1, runfiles behavior on Windows is not behaving as expected. I can't enable symlinks on Windows, so all Python binaries are built into an exe and zip of runfiles.

Consider a small binary which prints information about a runfile. When bundled as a py_binary, the python runfiles library locates the runfiles manifest and provides an absolute path to the original file. If you dump this to console, it looks like this:

Working dir: c:\users\sputt\_bazel~1\2jd6fjpz\execroot\_main\bazel-~1\x64_wi~1\bin\helloe~2.run      
Dumping path to runfile "rules_req_compile/LICENSE.txt":
C:/users/sputt/req-compile/LICENSE.txt

Problem 1 - why doesn't this use the runfiles we just unzipped? The manifest directs us to the original source file on disk, which can unfortunately confuse tools that make assumptions about what files are present. An example tool might be the pytest library, searching for conftest.py files. I can also see how this would make shipping a binary more difficult, whereas if the runfile in the zip was used the exe/zip combo would be portable to a different machine.

OK, so modifying my program, I'll use the short_path to the runfile and print the same information:

Short path
c:\users\sputt\_bazel~1\2jd6fjpz\execroot\_main\bazel-~1\x64_wi~1\bin\helloe~2.run\LICENSE.txt 

Problem 2, why is the current directory set to the runfiles root? This is contrary to the expectation of the current directory is set to your workspace directory, like it would be on Linux (or presumably with symlinks enabled on Windows). So this file does not exist and the program fails.

Let's repeat the experiment under py_test.

Using the runfiles library:

Working dir: C:\Users\sputt\AppData\Local\Temp\Bazel.runfiles_1klez0rv\runfiles\_main
Dumping path to runfile "rules_req_compile/LICENSE.txt":
C:/users/sputt/req-compile/LICENSE.txt

Once again, the test binary uses the runfiles manifest and locates the runfile.

Now using short_path in the test:

Short path
C:\Users\sputt\AppData\Local\Temp\Bazel.runfiles_1klez0rv\runfiles\_main\LICENSE.txt

This file exists, and the test can read the runfile. This runfile comes from the zip archive, NOT from the source directory.

Problem 3 seems best described as "why do py_test and py_binary run differently?" It seems like we zip runfiles in order to be able to short_path our way to files, but this doesn't work in the bazel run case. And using the runfiles library could be viewed as a runfiles escape, trivially allowing tests to inadvertently access the original source tree.

πŸ”¬ Minimal Reproduction

import os
import sys
from pathlib import Path

from python.runfiles import runfiles

RUNFILES = runfiles.Create()

if __name__ == '__main__':
    print("Working dir: {}".format(os.getcwd()))

    print("Runfile Manifest")
    print(RUNFILES.Rlocation("rules_req_compile/LICENSE.txt"))
    print(Path(RUNFILES.Rlocation("rules_req_compile/LICENSE.txt")).stat().st_size)

    print("Short path")
    print(Path("LICENSE.txt").absolute())
    print(Path("LICENSE.txt").stat().st_size)
   # Make the test fail
    sys.exit(1)
load("@rules_python//python:defs.bzl", "py_binary", "py_test")

py_binary(
    name = "hello",
    srcs = ["hello.py"],
    data = ["LICENSE.txt"],
    deps = ["@rules_python//python/runfiles"],
)

py_test(
    name = "hello_test",
    main = "hello.py",
    srcs = ["hello.py"],
    data = ["LICENSE.txt"],
    deps = ["@rules_python//python/runfiles"],
)

🌍 Your Environment

Operating System:

  
Windows 10
  

Output of bazel version:

  
(venv3) PS C:\Users\clzdg\req-compile> bazel version
Bazelisk version: v1.19.0
Build label: 7.1.1
Build target: @@//src/main/java/com/google/devtools/build/lib/bazel:BazelServer
Build time: Thu Mar 21 18:11:32 2024 (1711044692)
Build timestamp: 1711044692
Build timestamp as int: 1711044692
  

Rules_python version:

  
bazel_dep(name = "rules_python", version = "0.31.0")
  

Anything else relevant?

aignas commented 4 months ago

None of the maintainers are developing on Windows and this area may definitely have inconsistency. Proposals, PRs are welcome.