Skip to content

Commit

Permalink
Make Bazel not raise an error when a symlink points to its own ancestor.
Browse files Browse the repository at this point in the history 
This is only an error when the symlink is to be traversed recursively. E.g. `a/a/a/b` is perfectly acceptable if `a/a` is a symlink to `.`, except if we want to recursively traverse into it during a glob or recursive target pattern expansion.

RELNOTES: Blaze now allows symbolic links that point to their own ancestor unless they are traversed recursively by e.g. a //... recursive target pattern or a recursive glob.
PiperOrigin-RevId: 334982640
  • Loading branch information
@lberki @copybara-github
lberki authored and copybara-github committed Oct 2, 2020
1 parent 7aafb4f commit 7598bc6
Show file tree
Hide file tree
Showing 10 changed files with 407 additions and 137 deletions.
246 changes: 234 additions & 12 deletions src/main/java/com/google/devtools/build/lib/actions/FileValue.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,18 @@ public boolean isDirectory() {
*/
public abstract ImmutableList<RootedPath> logicalChainDuringResolution();

/**
* If a symlink pointing back to its own ancestor was encountered during the resolution of this
* {@link FileValue}, returns the path to it. Otherwise, returns null.
*/
public abstract ImmutableList<RootedPath> pathToUnboundedAncestorSymlinkExpansionChain();

/**
* If a symlink pointing back to its own ancestor was encountered during the resolution of this
* {@link FileValue}, returns the symlinks in the cycle. Otherwise, returns null.
*/
public abstract ImmutableList<RootedPath> unboundedAncestorSymlinkExpansionChain();

/**
* Returns the real rooted path of the file, taking ancestor symlinks into account. For example,
* the rooted path ['root']/['a/b'] is really ['root']/['c/b'] if 'a' is a symlink to 'c'. Note
Expand Down Expand Up @@ -165,6 +177,8 @@ public SkyFunctionName functionName() {
*/
public static FileValue value(
ImmutableList<RootedPath> logicalChainDuringResolution,
ImmutableList<RootedPath> pathToUnboundedAncestorSymlinkExpansionChain,
ImmutableList<RootedPath> unboundedAncestorSymlinkExpansionChain,
RootedPath originalRootedPath,
FileStateValue fileStateValueFromAncestors,
RootedPath realRootedPath,
Expand Down Expand Up @@ -205,17 +219,36 @@ public static FileValue value(

if (fileStateValueFromAncestors.getType() == FileStateType.SYMLINK) {
PathFragment symlinkTarget = fileStateValueFromAncestors.getSymlinkTarget();
return shouldStoreChain
? new SymlinkFileValueWithStoredChain(
realRootedPath, realFileStateValue, logicalChainDuringResolution, symlinkTarget)
: new SymlinkFileValueWithoutStoredChain(
realRootedPath, realFileStateValue, symlinkTarget);
if (pathToUnboundedAncestorSymlinkExpansionChain != null) {
return new SymlinkFileValueWithSymlinkCycle(
realRootedPath,
realFileStateValue,
logicalChainDuringResolution,
symlinkTarget,
pathToUnboundedAncestorSymlinkExpansionChain,
unboundedAncestorSymlinkExpansionChain);
} else if (shouldStoreChain) {
return new SymlinkFileValueWithStoredChain(
realRootedPath, realFileStateValue, logicalChainDuringResolution, symlinkTarget);
} else {
return new SymlinkFileValueWithoutStoredChain(
realRootedPath, realFileStateValue, symlinkTarget);
}
} else {
if (pathToUnboundedAncestorSymlinkExpansionChain != null) {
return new DifferentRealPathFileValueWithSymlinkCycle(
realRootedPath,
realFileStateValue,
logicalChainDuringResolution,
pathToUnboundedAncestorSymlinkExpansionChain,
unboundedAncestorSymlinkExpansionChain);
} else if (shouldStoreChain) {
return new DifferentRealPathFileValueWithStoredChain(
realRootedPath, realFileStateValue, logicalChainDuringResolution);
} else {
return new DifferentRealPathFileValueWithoutStoredChain(realRootedPath, realFileStateValue);
}
}

return shouldStoreChain
? new DifferentRealPathFileValueWithStoredChain(
realRootedPath, realFileStateValue, logicalChainDuringResolution)
: new DifferentRealPathFileValueWithoutStoredChain(realRootedPath, realFileStateValue);
}

/**
Expand All @@ -240,6 +273,16 @@ public ImmutableList<RootedPath> logicalChainDuringResolution() {
return ImmutableList.of(rootedPath);
}

@Override
public ImmutableList<RootedPath> pathToUnboundedAncestorSymlinkExpansionChain() {
return null;
}

@Override
public ImmutableList<RootedPath> unboundedAncestorSymlinkExpansionChain() {
return null;
}

@Override
public RootedPath realRootedPath() {
return rootedPath;
Expand Down Expand Up @@ -273,6 +316,84 @@ public String toString() {
}
}

/**
* A {@link FileValue} whose resolution required traversing a symlink chain caused by a symlink
* pointing to its own ancestor but which eventually points to a real file.
*/
@AutoCodec.VisibleForSerialization
@AutoCodec
public static class DifferentRealPathFileValueWithSymlinkCycle
extends DifferentRealPathFileValueWithStoredChain {
// We can't store an exception here because this needs to be serialized, AutoCodec chokes on
// object cycles and FilesystemInfiniteSymlinkCycleException somehow sets its cause to itself
protected final ImmutableList<RootedPath> pathToUnboundedAncestorSymlinkExpansionChain;
protected final ImmutableList<RootedPath> unboundedAncestorSymlinkExpansionChain;

public DifferentRealPathFileValueWithSymlinkCycle(
RootedPath realRootedPath,
FileStateValue realFileStateValue,
ImmutableList<RootedPath> logicalChainDuringResolution,
ImmutableList<RootedPath> pathToUnboundedAncestorSymlinkExpansionChain,
ImmutableList<RootedPath> unboundedAncestorSymlinkExpansionChain) {
super(realRootedPath, realFileStateValue, logicalChainDuringResolution);
this.pathToUnboundedAncestorSymlinkExpansionChain =
pathToUnboundedAncestorSymlinkExpansionChain;
this.unboundedAncestorSymlinkExpansionChain = unboundedAncestorSymlinkExpansionChain;
}

@Override
public ImmutableList<RootedPath> pathToUnboundedAncestorSymlinkExpansionChain() {
return pathToUnboundedAncestorSymlinkExpansionChain;
}

@Override
public ImmutableList<RootedPath> unboundedAncestorSymlinkExpansionChain() {
return unboundedAncestorSymlinkExpansionChain;
}

@Override
public int hashCode() {
return Objects.hash(
realRootedPath,
realFileStateValue,
logicalChainDuringResolution,
pathToUnboundedAncestorSymlinkExpansionChain,
unboundedAncestorSymlinkExpansionChain);
}

@Override
public boolean equals(Object obj) {
if (obj == null) {
return false;
}

if (obj.getClass() != DifferentRealPathFileValueWithSymlinkCycle.class) {
return false;
}

DifferentRealPathFileValueWithSymlinkCycle other =
(DifferentRealPathFileValueWithSymlinkCycle) obj;
return realRootedPath.equals(other.realRootedPath)
&& realFileStateValue.equals(other.realFileStateValue)
&& logicalChainDuringResolution.equals(other.logicalChainDuringResolution)
&& pathToUnboundedAncestorSymlinkExpansionChain.equals(
other.pathToUnboundedAncestorSymlinkExpansionChain)
&& unboundedAncestorSymlinkExpansionChain.equals(
other.unboundedAncestorSymlinkExpansionChain);
}

@Override
public String toString() {
return String.format(
"symlink ancestor (real_path=%s, real_state=%s, chain=%s, path=%s, cycle=%s)",
realRootedPath,
realFileStateValue,
logicalChainDuringResolution,
pathToUnboundedAncestorSymlinkExpansionChain,
unboundedAncestorSymlinkExpansionChain);
}
}

/**
* Implementation of {@link FileValue} for paths whose fully resolved path is different than the
* requested path, but the path itself is not a symlink. For example, this is the case for the
Expand Down Expand Up @@ -309,6 +430,16 @@ public ImmutableList<RootedPath> logicalChainDuringResolution() {
return logicalChainDuringResolution;
}

@Override
public ImmutableList<RootedPath> pathToUnboundedAncestorSymlinkExpansionChain() {
return null;
}

@Override
public ImmutableList<RootedPath> unboundedAncestorSymlinkExpansionChain() {
return null;
}

@Override
public boolean equals(Object obj) {
if (obj == null) {
Expand Down Expand Up @@ -369,6 +500,16 @@ public ImmutableList<RootedPath> logicalChainDuringResolution() {
throw new IllegalStateException(this.toString());
}

@Override
public ImmutableList<RootedPath> pathToUnboundedAncestorSymlinkExpansionChain() {
return null;
}

@Override
public ImmutableList<RootedPath> unboundedAncestorSymlinkExpansionChain() {
return null;
}

@Override
public boolean equals(Object obj) {
if (obj == null) {
Expand Down Expand Up @@ -396,12 +537,93 @@ public String toString() {
}
}

/**
* A {@link FileValue} whose resolution required traversing a symlink chain caused by a symlink
* pointing to its own ancestor and which eventually points to a symlink.
*/
@AutoCodec.VisibleForSerialization
@AutoCodec
public static final class SymlinkFileValueWithSymlinkCycle
extends SymlinkFileValueWithStoredChain {
// We can't store an exception here because this needs to be serialized, AutoCodec chokes on
// object cycles and FilesystemInfiniteSymlinkCycleException somehow sets its cause to itself
private final ImmutableList<RootedPath> pathToUnboundedAncestorSymlinkExpansionChain;
private final ImmutableList<RootedPath> unboundedAncestorSymlinkExpansionChain;

public SymlinkFileValueWithSymlinkCycle(
RootedPath realRootedPath,
FileStateValue realFileStateValue,
ImmutableList<RootedPath> logicalChainDuringResolution,
PathFragment linkTarget,
ImmutableList<RootedPath> pathToUnboundedAncestorSymlinkExpansionChain,
ImmutableList<RootedPath> unboundedAncestorSymlinkExpansionChain) {
super(realRootedPath, realFileStateValue, logicalChainDuringResolution, linkTarget);
this.pathToUnboundedAncestorSymlinkExpansionChain =
pathToUnboundedAncestorSymlinkExpansionChain;
this.unboundedAncestorSymlinkExpansionChain = unboundedAncestorSymlinkExpansionChain;
}

@Override
public ImmutableList<RootedPath> pathToUnboundedAncestorSymlinkExpansionChain() {
return pathToUnboundedAncestorSymlinkExpansionChain;
}

@Override
public ImmutableList<RootedPath> unboundedAncestorSymlinkExpansionChain() {
return unboundedAncestorSymlinkExpansionChain;
}

@Override
public int hashCode() {
return Objects.hash(
realRootedPath,
realFileStateValue,
logicalChainDuringResolution,
linkTarget,
pathToUnboundedAncestorSymlinkExpansionChain,
unboundedAncestorSymlinkExpansionChain);
}

@Override
public boolean equals(Object obj) {
if (obj == null) {
return false;
}

if (obj.getClass() != SymlinkFileValueWithSymlinkCycle.class) {
return false;
}

SymlinkFileValueWithSymlinkCycle other = (SymlinkFileValueWithSymlinkCycle) obj;
return realRootedPath.equals(other.realRootedPath)
&& realFileStateValue.equals(other.realFileStateValue)
&& logicalChainDuringResolution.equals(other.logicalChainDuringResolution)
&& linkTarget.equals(other.linkTarget)
&& pathToUnboundedAncestorSymlinkExpansionChain.equals(
other.pathToUnboundedAncestorSymlinkExpansionChain)
&& unboundedAncestorSymlinkExpansionChain.equals(
other.unboundedAncestorSymlinkExpansionChain);
}

@Override
public String toString() {
return String.format(
"symlink ancestor (real_path=%s, real_state=%s, target=%s, chain=%s, path=%s, cycle=%s)",
realRootedPath,
realFileStateValue,
linkTarget,
logicalChainDuringResolution,
pathToUnboundedAncestorSymlinkExpansionChain,
unboundedAncestorSymlinkExpansionChain);
}
}

/** Implementation of {@link FileValue} for paths that are themselves symlinks. */
@AutoCodec.VisibleForSerialization
@AutoCodec
public static final class SymlinkFileValueWithStoredChain
public static class SymlinkFileValueWithStoredChain
extends DifferentRealPathFileValueWithStoredChain {
private final PathFragment linkTarget;
protected final PathFragment linkTarget;

@VisibleForTesting
public SymlinkFileValueWithStoredChain(
Expand Down
7 changes: 7 additions & 0 deletions src/main/java/com/google/devtools/build/lib/skyframe/BUILD
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1057,6 +1057,7 @@ java_library(
name = "collect_packages_under_directory_function",
srcs = ["CollectPackagesUnderDirectoryFunction.java"],
deps = [
"process_package_directory",
":collect_packages_under_directory_value",
":recursive_directory_traversal_function",
":recursive_pkg_key",
Expand Down Expand Up @@ -1524,6 +1525,8 @@ java_library(
srcs = ["GlobFunction.java"],
deps = [
":directory_listing_value",
":file_symlink_infinite_expansion_exception",
":file_symlink_infinite_expansion_uniqueness_function",
":glob_descriptor",
":glob_value",
":ignored_package_prefixes_value",
Expand Down Expand Up @@ -2020,6 +2023,8 @@ java_library(
":directory_listing_value",
":dirents",
":file_symlink_exception",
":file_symlink_infinite_expansion_exception",
":file_symlink_infinite_expansion_uniqueness_function",
":package_lookup_value",
":process_package_directory_result",
"//src/main/java/com/google/devtools/build/lib/actions:file_metadata",
Expand Down Expand Up @@ -2095,6 +2100,8 @@ java_library(
deps = [
":action_execution_value",
":directory_listing_value",
":file_symlink_infinite_expansion_exception",
":file_symlink_infinite_expansion_uniqueness_function",
":package_lookup_value",
":sky_functions",
":tree_artifact_value",
Expand Down
Loading

0 comments on commit 7598bc6

Please sign in to comment.