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

Revert "Enabling Bazel to generate input symlinks as defined by RE AP… #7216

Closed
wants to merge 1 commit into from

Conversation

buchgr
Copy link
Contributor

@buchgr buchgr commented Jan 22, 2019

…I V2."

This reverts commit baa1786.

The symlink resolution in this change is broken, as it does not take into
account parent symlinks. Consider the following structure on the filesystem:

a/d/file
a/b/c/symlink -> ../../d/file

And action inputs as follows:

a/d/file
(f -> a/b/c)/symlink -> ../../d/file

with (f -> a/b/c) denoting that f is a symlink to directory c.

This change then builds the following merkle tree:

a
  d
    file
f
  symlink -> ../../d/file

My guesstimate is that there are a number of additional problems with
this change and we should think hard about them when attempting to
roll foward this change in the future. See for example Bazel's symlink
resolution in FileFunction [1].

A real world example of this error is: #7212
Fixes #7212

[1] https://source.bazel.build/bazel/+/master:src/main/java/com/google/devtools/build/lib/skyframe/FileFunction.java;l=114?q=FileFunction

…I V2."

This reverts commit baa1786.

The symlink resolution in this change is broken, as it does not take into
account parent symlinks. Consider the following structure on the filesystem:

a/d/file
a/b/c/symlink -> ../../d/file

And action inputs as follows:

a/d/file
(f -> a/b/c)/symlink -> ../../d/file

with (f -> a/b/c) denoting that f is a symlink to directory c.

This change then builds the following merkle tree:

a
  d
    file
f
  symlink -> ../../d/file

My guesstimate is that there are a number of additional problems with
this change and we should think hard about them when attempting to
roll foward this change in the future. See for example Bazel's symlink
resolution in FileFunction [1].

A real world example of this error is: bazelbuild#7212
Fixes bazelbuild#7212

[1] https://source.bazel.build/bazel/+/master:src/main/java/com/google/devtools/build/lib/skyframe/FileFunction.java;l=114?q=FileFunction
@buchgr buchgr force-pushed the revert-input-symlinks branch from 67b4065 to bbd456f Compare January 22, 2019 18:58
@buchgr
Copy link
Contributor Author

buchgr commented Jan 22, 2019

@agoulti @bergsieker

@philwo
Copy link
Member

philwo commented Jan 22, 2019

Thanks!

@bazel-io bazel-io closed this in 12ab12e Jan 23, 2019
aehlig pushed a commit that referenced this pull request Jan 23, 2019
?I V2."

This reverts commit baa1786.

The symlink resolution in this change is broken, as it does not take into
account parent symlinks. Consider the following structure on the filesystem:

```
a/d/file
a/b/c/symlink -> ../../d/file
```
And action inputs as follows:
```
a/d/file
(f -> a/b/c)/symlink -> ../../d/file
```
with (f -> a/b/c) denoting that f is a symlink to directory c.

This change then builds the following merkle tree:
```
a
  d
    file
f
  symlink -> ../../d/file
```
My guesstimate is that there are a number of additional problems with
this change and we should think hard about them when attempting to
roll foward this change in the future. See for example Bazel's symlink
resolution in FileFunction [1].

A real world example of this error is: #7212
Fixes #7212

[1] https://source.bazel.build/bazel/+/master:src/main/java/com/google/devtools/build/lib/skyframe/FileFunction.java;l=114?q=FileFunction

Closes #7216.

PiperOrigin-RevId: 230527829
aehlig pushed a commit that referenced this pull request Jan 23, 2019
?I V2."

This reverts commit baa1786.

The symlink resolution in this change is broken, as it does not take into
account parent symlinks. Consider the following structure on the filesystem:

```
a/d/file
a/b/c/symlink -> ../../d/file
```
And action inputs as follows:
```
a/d/file
(f -> a/b/c)/symlink -> ../../d/file
```
with (f -> a/b/c) denoting that f is a symlink to directory c.

This change then builds the following merkle tree:
```
a
  d
    file
f
  symlink -> ../../d/file
```
My guesstimate is that there are a number of additional problems with
this change and we should think hard about them when attempting to
roll foward this change in the future. See for example Bazel's symlink
resolution in FileFunction [1].

A real world example of this error is: #7212
Fixes #7212

[1] https://source.bazel.build/bazel/+/master:src/main/java/com/google/devtools/build/lib/skyframe/FileFunction.java;l=114?q=FileFunction

Closes #7216.

PiperOrigin-RevId: 230527829
weixiao-huang pushed a commit to weixiao-huang/bazel that referenced this pull request Jan 31, 2019
?I V2."

This reverts commit baa1786.

The symlink resolution in this change is broken, as it does not take into
account parent symlinks. Consider the following structure on the filesystem:

```
a/d/file
a/b/c/symlink -> ../../d/file
```
And action inputs as follows:
```
a/d/file
(f -> a/b/c)/symlink -> ../../d/file
```
with (f -> a/b/c) denoting that f is a symlink to directory c.

This change then builds the following merkle tree:
```
a
  d
    file
f
  symlink -> ../../d/file
```
My guesstimate is that there are a number of additional problems with
this change and we should think hard about them when attempting to
roll foward this change in the future. See for example Bazel's symlink
resolution in FileFunction [1].

A real world example of this error is: bazelbuild#7212
Fixes bazelbuild#7212

[1] https://source.bazel.build/bazel/+/master:src/main/java/com/google/devtools/build/lib/skyframe/FileFunction.java;l=114?q=FileFunction

Closes bazelbuild#7216.

PiperOrigin-RevId: 230527829
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants