rules_rust icon indicating copy to clipboard operation
rules_rust copied to clipboard

Breaking behavior change in the rules_rust runfiles library

Open DolceTriade opened this issue 7 months ago • 11 comments

Hello,

In v0.59.1 I could do the following things using the runfiles library (that I depended on):

  • runfiles::Runfiles::rlocation("path/to/dir") would return the path to the directory
  • runfiles::Runfiles::rlocation("") would return the path to the root runfiles directory

However, in v0.61.0, neither of the above behavior work. Worse, Bazel runtime behavior does not match test behavior (ie, the same code works with bazel test but will fail in bazel run.

I'm not really certain what the root cause of this behavior change is, but it seems the additional manifest based stuff is probably related..

If the above behaviors are not supported, is there an alternative?

I'll continue to dig to isolate the changes that broke the above behaviors.

DolceTriade avatar May 04 '25 06:05 DolceTriade

I can confirm that https://github.com/bazelbuild/rules_rust/commit/8de8f2b89f71819c1d226ff0b87a62bdb3f13c32

is the breaking change. Can this behavior be toggleable?

DolceTriade avatar May 04 '25 06:05 DolceTriade

Sorry about the breakage!

In v0.59.1 I could do the following things using the runfiles library (that I depended on):

* `runfiles::Runfiles::rlocation("path/to/dir")` would return the path to the directory

* `runfiles::Runfiles::rlocation("")` would return the path to the root runfiles directory

I'm not sure directories that aren't explicitly runfiles were intended to be locatable through the runfiles API but I'm not sure if there's a documented interface outlining the behavior of the interfaces. I reviewed the C++ implementation assuming it would have been implemented to the correct spec as it's still maintained by Google. Maybe @fmeum might know of some implementation guide?

Worse, Bazel runtime behavior does not match test behavior (ie, the same code works with bazel test but will fail in bazel run.

This sounds bad. Why is the behavior different? Are runfiles links only available in one context vs another?

UebelAndre avatar May 04 '25 17:05 UebelAndre

I guess I should write that guide, but I still want to simplify the process before documenting it.

@UebelAndre's commit is correct: If present, the manifest has to be preferred since the runfiles directory may exist but be empty without a good way to detect that state.

Rlocation doesn't support implicit directories (those that only arise as parents of runfiles) simply because with the manifest implementation, these directories might not exist on disk.

If you want something better, you could opt for a "virtual runfiles FS" such as the fs.FS implementation in the Go runfiles library. As the author of this functionality, let me warn you that it took way more work than expected.

@DolceTriade Could you elaborate on the run vs test difference?

fmeum avatar May 04 '25 18:05 fmeum

I debugged this a little more...

I probably should have included this but I'm still using WORKSPACE on Bazel 7.4.1 (behavior is the same on OSX and Linux tho).

I'm doing something like this: https://github.com/DolceTriade/blade/blob/builtin/blade/bep/proto_registry/lib.rs#L15-L34

If I build tests exercising the code and I inspect the runfiles in bazel-bin, I see a MANIFEST file. However, if I actually run the test, there is no manifest file in the runfiles, so the runfiles code falls back to the directory based approach which works...

However, if I do bazel run, the MANIFEST is present in the runfiles directory and thus my fake directory stuff doesn't work.

I could "hack" the old behavior by using sentinel files, but I think that kinda defeats the purpose of runfiles...

edit: I also verified that the behavior appears to be the same with bzlmod enabled too...

DolceTriade avatar May 04 '25 19:05 DolceTriade

This is expected. Whether a runfiles library uses the directory or the manifest as a "backend" is considered an implementation detail, which is also why "features" offered by only one of the backends are not generally supported.

Sentinel files aren't bad, they are actually a rather natural way to specify which directory with a given runfiles path you want (as mentioned previously, there can be multiple): the one that contains the given file.

fmeum avatar May 05 '25 10:05 fmeum

I guess the "fix" here then is to make sure the manifest and directory based approaches are consistent by making sure the directory based approach cannot return a reference to a directory?

The lack of MANIFEST in tests seems like a bazel bug, so I should probably open another issue there?

DolceTriade avatar May 05 '25 17:05 DolceTriade

I guess the "fix" here then is to make sure the manifest and directory based approaches are consistent by making sure the directory based approach cannot return a reference to a directory?

This unfortunately isn't possible since you can't always distinguish between an "implicit" directory (i.e., only a parent of a runfile) and an "explicit" directory (a source directory).

I wouldn't call the lack of a runfiles manifest for test actions a bug. Remote execution is another example of a situation which never uses a manifest. Just like files may be staged as regular files, symlinks or hardlinks depending on the execution strategy, this is something Bazel reserves the right to change for optimization purposes.

fmeum avatar May 05 '25 19:05 fmeum

If bazel chooses to be inconsistent, why can't we force the runfiles library to pick the underlying implementation we prefer? Would we be open to accepting a PR to set a preferred mode?

DolceTriade avatar May 05 '25 23:05 DolceTriade

Bazel doesn't choose to, it has to (at least with the way it currently generates manifests and runfiles directories). There are situations in which the use of the runfiles directory would result in incorrect lookup results, so an API that forces a particular mode would be inherently unsafe to use.

In the short term, if you know that you are exclusively working on Unix, you could look up runfiles directly relative to RUNFILES_DIR (if set) or else $0.runfiles. Once you have the dir, most runfiles libraries allow you to set it via a constructor.

Mid- to long-term I will work on making the runfiles directory a safe default instead.

fmeum avatar May 06 '25 07:05 fmeum

That sounds good! Let me know if I can help.

DolceTriade avatar May 06 '25 17:05 DolceTriade

I encountered this issue as well. It was particularly confusing because the same rlocation! call behaved differently under bazel run and bazel test.

sfc-gh-bhannel avatar Oct 21 '25 22:10 sfc-gh-bhannel