Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sandbox can mishandle symlink outputs #4987

Closed
benjaminp opened this issue Apr 9, 2018 · 0 comments
Closed

sandbox can mishandle symlink outputs #4987

benjaminp opened this issue Apr 9, 2018 · 0 comments

Comments

@benjaminp
Copy link
Collaborator

benjaminp commented Apr 9, 2018

Consider these genrules that emit symlinks:

$ cat BUILD
genrule(
    name = 'test1',
    outs = ['file1', 'link1'],
    cmd = 'touch $(location file1); ln -s file1 $(location link1)',
)

genrule(
    name = 'test2',
    outs = ['link2', 'file2'],
    cmd = 'touch $(location file2); ln -s file2 $(location link2)',
)
$ bazel build //:test1 //:test2
$ ls -lh bazel-genfiles/
total 0
-r-xr-xr-x 1 benjamin benjamin 0 Apr  9 13:24 file1
-r-xr-xr-x 1 benjamin benjamin 0 Apr  9 13:24 file2
lrwxrwxrwx 1 benjamin benjamin 5 Apr  9 13:24 link1 -> file1
lrwxrwxrwx 1 benjamin benjamin 5 Apr  9 13:24 link2 -> file2

So far, so good. Unfortunately, things work less well when the real execroot is on a different filesystem than the sandbox execroot:

$ bazel clean
$ bazel build --experimental_sandbox_base /run/shm/sandbox-fs  --keep_going //:test1 //:test2
INFO: Analysed 2 targets (7 packages loaded).
INFO: Found 2 targets...
ERROR: BUILD:1:1: Couldn't build file file1: Executing genrule //:test1 failed: I/O exception during sandboxed execution: Could not move output artifacts from sandboxed execution
INFO: Elapsed time: 1.901s, Critical Path: 0.09s
INFO: 0 processes.
FAILED: Build did NOT complete successfully
$ ls -lh bazel-genfiles/
total 0
-rw-rw-r-- 1 benjamin benjamin 0 Apr  9 13:26 file1
-r-xr-xr-x 1 benjamin benjamin 0 Apr  9 13:26 file2
-r-xr-xr-x 1 benjamin benjamin 0 Apr  9 13:26 link2

Not only does the building one of the genrules fail, but the sandbox has silently replaced the symbolic link link2 with a regular file!

benjaminp added a commit to benjaminp/bazel that referenced this issue Apr 10, 2018
…SandboxedSpawn.copyOutputs.

Previously, if moveFile fell back to copying from a true rename (e.g., if the
move is across file systems), it would fully dereference the source and produce
a regular file at the output location. This CL fixes moveFile to properly copy
symlinks. Note we don't preserve metadata in the symlink case mostly because the
Bazel VFS has no API to write metadata without dereferencing symlinks. (But,
also, it's not important for the current use cases.)

This breaks the backward-compatibility of FileSystemUtils.moveFile and
FileSystemUtils.moveTreeBelow. This seems okay because the new behavior makes
more sense, and sandbox is the only consumer of these APIs.

Switching SandboxedSpawn.copyOutputs to use FileSystemUtils.moveFile rather than
Guava's Files.move fixes bazelbuild#4987.

Change-Id: I0283e8aa11fadff5b0afd7bdfd0490aca12a1f6b
benjaminp added a commit to benjaminp/bazel that referenced this issue Apr 10, 2018
…SandboxedSpawn.copyOutputs.

Previously, if moveFile fell back to copying from a true rename (e.g., if the
move is across file systems), it would fully dereference the source and produce
a regular file at the output location. This CL fixes moveFile to properly copy
symlinks. Note we don't preserve metadata in the symlink case mostly because the
Bazel VFS has no API to write metadata without dereferencing symlinks. (But,
also, it's not important for the current use cases.)

This breaks the backward-compatibility of FileSystemUtils.moveFile and
FileSystemUtils.moveTreeBelow. This seems okay because the new behavior makes
more sense, and sandbox is the only consumer of these APIs.

Switching SandboxedSpawn.copyOutputs to use FileSystemUtils.moveFile rather than
Guava's Files.move fixes bazelbuild#4987.

Change-Id: I0283e8aa11fadff5b0afd7bdfd0490aca12a1f6b
luca-digrazia pushed a commit to luca-digrazia/DatasetCommitsDiffSearch that referenced this issue Sep 4, 2022
… SandboxedSpawn.copyOutputs.

    Previously, if moveFile fell back to copying from a true rename (e.g., if the
    move is across file systems), it would fully dereference the source and produce
    a regular file at the output location. This CL fixes moveFile to properly copy
    symlinks. Note we don't preserve metadata in the symlink case mostly because the
    Bazel VFS has no API to write metadata without dereferencing symlinks. (But,
    also, it's not important for the current use cases.)

    This breaks the backward-compatibility of FileSystemUtils.moveFile and
    FileSystemUtils.moveTreeBelow. This seems okay because the new behavior makes
    more sense, and sandbox is the only consumer of these APIs.

    Switching SandboxedSpawn.copyOutputs to use FileSystemUtils.moveFile rather than
    Guava's Files.move fixes bazelbuild/bazel#4987.

    Closes #4989.

    Change-Id: I0283e8aa11fadff5b0afd7bdfd0490aca12a1f6b
    PiperOrigin-RevId: 192611227
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants