Repository: bazelbuild/bazel
Issue: Python wrapper sometimes puts manifest filename into RUNFILES_DIR
Labels: type: bug
untriaged
team-Rules-Python

### Description of the bug:


py_binary rules come with a auto-generated [wrapper script](https://github.com/bazelbuild/bazel/blob/master/tools/python/python_bootstrap_template.txt) which automatically detects runfiles and sets either `RUNFILES_DIR` or `RUNFILES_MANIFEST_FILE`, depending on what is found first.

This script has a bug: if `progname.runfiles_manifest` file is missing, but `programe.runfiles/MANIFEST` is present, the script erroneously sets `RUNFILES_DIR` to manifest file location. This causes all sorts of failures down the line (for example when using bazel's `runfiles.bash` rules)

Code in question: https://github.com/bazelbuild/bazel/blob/4427a1fb0cf537da82e1ca4841e0cecda99e1107/tools/python/python_bootstrap_template.txt#L239
(compare to previous stanza)

### What's the simplest, easiest way to reproduce this bug? Please provide a minimal example if possible.

To demonstrate it, we need any bazel repo which uses python runfiles lib. Bazel's own `rules_pkg` is one example. 
First, check out and run the test. It should work: 
```
git clone https://github.com/bazelbuild/rules_pkg.git
cd rules_pkg
bazel run //tests:stamp_test  # This works
```
Now naively package the runfiles dir and try to use it. Note we've got `stamp_test.runfiles/MANIFEST` (inside runfiles) but forgot `stamp_test.runfiles_manifest`:
```
rm -rf /tmp/work && mkdir /tmp/work && cp -r bazel-bin/tests/stamp_test bazel-bin/tests/stamp_test.runfiles /tmp/work && /tmp/work/stamp_test
...
NotADirectoryError: [Errno 20] Not a directory: '/tmp/work/stamp_test.runfiles/MANIFEST/rules_pkg/tests/stamped_zip.zip'

----------------------------------------------------------------------
Ran 2 tests in 0.002s

FAILED (errors=2)
```
observe how it treats `stamp_test.runfiles/MANIFEST` file as a directory and fails.

### Which operating system are you running Bazel on?


Various Ubuntu versions (18.04, 20.04, 22.04)

### What is the output of `bazel info release`?


This is present in HEAD and was introduced in 2018. Testcases above were tested on 6.0.0

### If `bazel info release` returns `development version` or `(@non-git)`, tell us how you built Bazel.


_No response_

### What's the output of `git remote get-url origin; git rev-parse master; git rev-parse HEAD` ?


```text
https://github.com/bazelbuild/bazel.git
4427a1fb0cf537da82e1ca4841e0cecda99e1107
4427a1fb0cf537da82e1ca4841e0cecda99e1107
```


### Have you found anything relevant by searching the web?

Nothing. I am guessing very few people use custom bazel packaging scripts that only copy one file but not both.

### Any other information, logs, or outputs that you want to share?

We have a workaround which is to copy both files, and it probably is more correct that way, too. I am reporting this bug for the unlikely case someone else encounters it.

Workarounds for my test case above:
- Include both manifest files:
```
rm -rf /tmp/work && mkdir /tmp/work && cp -r bazel-bin/tests/stamp_test{,.runfiles,.runfiles_manifest} /tmp/work && /tmp/work/stamp_test
..
OK
```

- Do not include any manifest files
```
rm -rf /tmp/work && mkdir /tmp/work && cp -r bazel-bin/tests/stamp_test{,.runfiles} /tmp/work && rm -f /tmp/work/stamp_test.runfiles/MANIFEST && /tmp/work/stamp_test
..
OK
```


Issue comments:
Hi @mafanasyev-tri, Can you provide an sample example to test the same. Thanks! 
Updated ticket description to include test case.

Pull request review comments:

Patch that resolved the issue:
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 19514526104b4c..92dd6b82fa0dae 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
@@ -215,7 +215,13 @@ def GetRepositoriesImports(module_space, import_all):
   return [os.path.join(module_space, '%workspace_name%')]
 
 def RunfilesEnvvar(module_space):
-  """Finds the runfiles manifest or the runfiles directory."""
+  """Finds the runfiles manifest or the runfiles directory.
+
+  Returns:
+    A tuple of (var_name, var_value) where var_name is either 'RUNFILES_DIR' or
+    'RUNFILES_MANIFEST_FILE' and var_value is the path to that directory or
+    file, or (None, None) if runfiles couldn't be found.
+  """
   # If this binary is the data-dependency of another one, the other sets
   # RUNFILES_MANIFEST_FILE or RUNFILES_DIR for our sake.
   runfiles = os.environ.get('RUNFILES_MANIFEST_FILE', None)
@@ -236,9 +242,12 @@ def RunfilesEnvvar(module_space):
     return ('RUNFILES_MANIFEST_FILE', runfiles)
 
   # Look for the runfiles "input" manifest, argv[0] + ".runfiles/MANIFEST"
+  # Normally .runfiles_manifest and MANIFEST are both present, but the
+  # former will be missing for zip-based builds or if someone copies the
+  # runfiles tree elsewhere.
   runfiles = os.path.join(module_space, 'MANIFEST')
   if os.path.exists(runfiles):
-    return ('RUNFILES_DIR', runfiles)
+    return ('RUNFILES_MANIFEST_FILE', runfiles)
 
   # If running in a sandbox and no environment variables are set, then
   # Look for the runfiles  next to the binary.


Test patch: