Runfiles: support space in file paths

[laszlocsomor]

The runfiles manifest remains the same for files
without space in their runfiles-relative path.

For files with space in their runfiles-relative
path, the link path's spaces are replaced by the
ASCII 0x01 character (U+0001). This character is
very unlikely to appear in paths (so it's a safe
bet as an escape character) and it has a
single-byte UTF-8 encoding, so when looking up
paths from the runfiles manifest (which itself is
UTF-8 encoded) tools like 'sed' can easily
transform '\x01' to space and back.

Thus the first space on a line is always the
delimiter, but runfiles libraries must replace the
requested path's spaces with '\x01' upon every
manifest-based lookup.

This design ensures that manifest consumers keep
working without having to handle '\x01', as long
as all data-dependencies are space-free. Otherwise
they break, but they would break today anyway.

RELNOTES[NEW]: Runfiles: data-dependencies (runfiles) may now contain spaces.

Fixes #4327

Change-Id: I37ce3b3ef2a9e92691e987588b0e6d67969a78c5

[laszlocsomor]

Sorry, had to move the test to another directory and now it fails on Windows. Will ping the thread again when it's ready for review.

[laszlocsomor]

Ready again :)

[jin]

Is this waiting on a review from @michajlo?

[laszlocsomor]

No, it's waiting input from me.
It's pending internal review but I didn't yet decide whether I want to import it or not.

Let's close it for now, I see PRs are getting cleaned up and I don't want to hinder that.

[laszlocsomor]

(As in: there were some concerns about it by reviewers, and I haven't decided whether the PR carries its weight.)

[marmos91]

@laszlocsomor, do you have any insight about the status of this fix? I am still facing #4327 because I need to run some Chromium-related tools on OSX, which have paths that contain spaces. Will this ever land? Is there another fix coming?

[ulfjack]

The problem is that the file is now a public interface, which means changing the format is a breaking change. It's going to be very difficult to roll anything like this out.

[laszlocsomor]

@marmos91 : I don't have any insight. But with --experimental_inprocess_symlink_creation, Bazel allows data-deps with spaces under the runfiles directory.

The runfiles manifest is still broken (still uses space to separate link name and link target), so data-deps require using symlinks. Luckily, on macOS that is enabled by default.

Demo:

foo/BUILD:

sh_test(
    name = "x",
    srcs = ["x.sh"],
    data = glob(["*"]),
)

foo/x.sh:

#!/bin/bash
echo Hello
find $TEST_SRCDIR | sed 's|^.*__main__/||'
echo Bye

ls -la foo/:

  $ ls -la foo/
total 16
drwxrwxr-x 2 laszlo laszlo 4096 Jul  6 12:26  .
drwxrwxr-x 4 laszlo laszlo 4096 Jul  6 12:26  ..
-rw-rw-r-- 1 laszlo laszlo    0 Jul  6 12:10  a
-rw-rw-r-- 1 laszlo laszlo    0 Jul  6 12:10 'b c'
-rw-rw-r-- 1 laszlo laszlo   72 Jul  6 12:21  BUILD
-rwxrwxr-x 1 laszlo laszlo   75 Jul  6 12:26  x.sh

$ bazel test //foo:x --test_output=all --experimental_inprocess_symlink_creation:

(...)
INFO: From Testing //foo:x
==================== Test output for //foo:x:
Hello
bazel-out/k8-fastbuild/bin/foo/x.runfiles
bazel-out/k8-fastbuild/bin/foo/x.runfiles/__main__
foo
foo/x.sh
foo/BUILD
foo/b c
foo/x
foo/a
Bye
================================================================================
//foo:x 
(...)