File#short_path in Skylark is not resolvable from its root

[yugui]

The behavior of short_path method of File type in Skylark has changed in Bazel 0.3.0.
Although it looks to be related to the runfiles structure change, I doubt if this change of short_path was intentional because the new behavior looks to be different from its documentation.

short_path of a File in an external repository used to return a relative path from $(SRCDIR) in Bazel 0.2.3, but now it looks to be returning a runtime relative path in the new runfiles structure.

How to reproduce

Put the following files WORKSPACE, BUILD and example.bzl into a directory.

• WORKSPACE:

  EXAMPLE_BUILD = """
  load("@//:example.bzl", "path_record")
  path_record(
    name = "readme",
    src = "README",
  )
  """
  
  new_git_repository(
      name = "com_github_golang_glog",
      remote = "https://github.com/golang/glog.git",
      commit = "23def4e6c14b4da8ac2ed8007337bc5eb5007998",
      build_file_content = EXAMPLE_BUILD,
  )

• BUILD:

  # empty

• example.bzl:

  def _path_record_impl(ctx):
    src = ctx.file.src
    out = ctx.outputs.out
    ctx.action(
        inputs = [src],
        outputs = [out],
        command = " && ".join([
            "echo root.path = " + src.root.path + ">" + out.path,
            "echo short_path = " + src.short_path + ">>" + out.path,
            "echo path = " + src.path + ">>" + out.path,
        ]),
    )
  
  path_record = rule(
      _path_record_impl,
      attrs = {
          "src": attr.label(
              allow_files = True,
              single_file = True,
          ),
      },
      outputs = {
          "out": "%{name}.paths",
      }
  )

Then, bazel build @com_github_golang_glog//:readme

$ bazel build @com_github_golang_glog//:readme
INFO: Found 1 target...
Target @com_github_golang_glog//:readme up-to-date:
  bazel-bin/external/com_github_golang_glog/readme.paths
INFO: Elapsed time: 2.659s, Critical Path: 0.01s
$ cat bazel-bin/external/com_github_golang_glog/readme.paths
root.path =
short_path = ../com_github_golang_glog/README
path = external/com_github_golang_glog/README

Expected result

I expected the following result, which is same as the actual result in Bazel 0.2.3.

root.path =
short_path = external/com_github_golang_glog/README
path = external/com_github_golang_glog/README

Actual result

root.path =
short_path = ../com_github_golang_glog/README
path = external/com_github_golang_glog/README

For me the old behavior looks to be more appropriate because the documentation says

The path of this file relative to its root. This excludes the aforementioned root, i.e. configuration-specific fragments of the path

So I expect that short_path is equivalent to path if I resolve it from root.path.

Although it also says

This is also the path under which the file is mapped if it's in the runfiles of a binary.

But the example above shows that these two definitions do not match to each other.

Proposal

At the same time, I understand that relative path in runfiles is also important for rule developers.
So I propose distinguishing short_path from runfiles mapping as the followings.

1. Rollback the behavior of short_path.
2. Remove the last sentence from the documentation of short_path
3. Define a new Skylark-visible method, runtime_path, which returns a relative path to the file in the runfiles structure.

References

This behavior change is the root cause of an issue of rules_go, bazel-contrib/rules_go#35 (comment).

[aehlig]

I can confirm that this is a difference between the documentation and the implementation. Assigning to @kchodorow to determine whether the bug is in the implementation or documentation.

[kchodorow]

Well, the goal is to have .path also return ../com_github_golang_glog. bdfd58a did this (which was rolled back, but I'm hoping to send a code review to roll it forward today). So, it's kind of going to go in the other direction: path is going to be updated to match short_path.

For now, can the go rule just use path, if they aren't trying to reference runfiles?

[yugui]

@kchodorow

So, it's kind of going to go in the other direction: path is going to be updated to match short_path.

I've added a short-term workaround in rules_go. But IIUC, your change makes this workaround also broken because it still depends on that .path gives a path to the file in execRoot as described in the document.
Although it might be still possible to use ctx.expand_location.

What is the recommended way to get paths to files in execRoot when I build command line arguments in Skylark actions?

[kchodorow]

.path should still work, it'll just be a different path than it is now.

[yugui]

@kchodorow Ah, I understand. I should have read the commit message of bdfd58a.

[excavador]

So, problem is actual to me.

I am writing repository with rules for python (with virtualenv support)
I am writing the tool which generate BUILD-files (like gazelle).

In my rules I am generating the executable shell script, which make

• source virtualenv (dependency)
• execute python script.

If I generate script using "virtualenv.short_path" and "python_file.short_path" every rule from repository which uses my rules_python works fine.
If I am trying to define some rule INSIDE my rules_python (for gazelle like tool) and use it, I meet this problem (short_path expanded as ../<rules_python_repo_name> and that's incorrect)

If I generate script using "virtualenv.path" and "python.path" rule from my rules_python works fine, while rules from repository which uses my rules_python broken.

What the correct way to reference to some dependencies file in my scripts independent from location - primary repo or external?