runfiles symlink obscured

mobileink commented 3 years ago

Description of the problem / feature request:

There seems to be a hidden requirement for data files, namely that the target label must not be a prefix of the runfile. If it is, we get a warning like

WARNING: /Users/gar/mina/ppx_optcomp/test/BUILD.bazel:21:9: runfiles symlink test/import_relativity/exit.mlh -> test/import_relativity/exit.mlh obscured by test/import_relativity -> bazel-out/darwin-fastbuild/bin/test/import_relativity

and failure, at least for test rules.

If I were not familiar with test rule implementation details I think this would be indecipherable. I suggest:

document this in the data attribute documentation , not just the runfiles docs.
add a suggestion to the warning message, something like "Make sure your target name is not a prefix of a data file."
even better, eliminate the problem - do not symlink the target label name, generate something unique.

Feature requests: what underlying problem are you trying to solve with this feature?

Avoid surprises.

Bugs: what's the simplest, easiest way to reproduce this bug? Please provide a minimal example if possible.

I have a test rule in package //test with name="import_relativity", and data=["import_relativity/entry.mlh"]. In the rule implementation, I iterate over ctx.files.data, adding each to ctx.runfiles. What gets added is test/import_relativity/entry.mlh. The test fails when run with file not found for the entry.mlh file, plus the warning noted above. This is apparently because a symlink is created from the target label, test/import_relativity, which "obscures" the runfile symlink. If I change the name of the rule, e.g. to import_relativity_test, then the test succeeds.

What operating system are you running Bazel on?

MacOS Catalina

What's the output of `bazel info release`?

release 3.6.0

Have you found anything relevant by searching the web?

No.

aiuto commented 3 years ago

This is true for more than tests. A small repo is...

### print_file.py
def cat_file():
  rf = runfiles.Create()
  path = rf.Rlocation('example/print_file/bar.txt')
  print("Open", path)
  with open(path, 'r') as inp:
    print(inp.read())

if __name__ == '__main__':
  cat_file()

### BUILD
py_binary(
  name = "print_file",
  srcs = ["print_file.py"],
  data = ["print_file/bar.txt"],
  deps = ["@bazel_tools//tools/python/runfiles"],
)

### WORKSPACE
workspace(name='example')

The conflict is that we set up the execution environment in a particular way

The final binary must use the name of the target, print_file
But this example also expects print_file to name a folder.

No particular solutions are great

we could put data in a specially named folder.... but what name, and that will break everyone who thinks they know how to pick up data files.
we could rename the executable, but that breaks different expectations. For example, once you build a binary, you expect to be able to do bazel-bin/print_file.

I think the best solution here is to document that target names in any BUILD file should not clash with folder names. This is true for most build systems. When you create a binary, you expect that the binary will be named whatever you named it. If you also have a folder of that name, ... things break.

twheys commented 3 years ago

I have a very similar problem caused by targets like genrule that output a directory file. If I try to use the directory file output by a genrule in addition to other files that share the same path or are subdirectories under the genrule directory, then I get the same error.

For example:

genrule target outputs a directory /{workspace}/a/b Another target outputs file /{workspace}/a/b/foo.file

In a third rule, I create runfile symlinks for both of these files, then I get the warning saying that /{workspace}/a/b/foo.file obscures /{workspace}/a/b and the in resulting output, /{workspace}/a/b/foo.file is omitted.

Since there is not another mechanism for Bazel targets that generate a variable number of output files than to use a containing directory file, it would be nice if it would at least be possible to merge directory files and regular files.

bazelbuild / bazel