Fix treatment of symlinks directories that exist above output files. This condition never arises in a clean build, but may arise on incremental builds in which output artifacts from the first build become part of the directory hierarchy of an output file in the second builds.
The existing checks were insufficient for two primary reasons:
1. The fast path that creates output directories is not sensitive to symlinks. If a/b/c exists, where b is a symlink, the check happily reports that "a/b/c" is a directory.
2. The recovery path previously did the walk upwards from the output directory, stopping when it found a directory. Again, this fails to handle upward symlinks.
A cache is introduced to mitigate the additional system calls here.
An alternative approach would be to perform the symlink checks along with the artifact prefix conflict checks. These checks are done only when necessary (eg, when any configured targets are analyzed / re-analyzed). It would be nice to limit the cases in which we do the new symlink checks, but this is unsound in the face of external edits to the output tree. Blaze is sound in all other such mutations to the output tree, so I've attempted to maintain our correctness contract here.
RELNOTES: None
PiperOrigin-RevId: 284052424
diff --git a/src/main/java/com/google/devtools/common/options/Converters.java b/src/main/java/com/google/devtools/common/options/Converters.java
index f4e5e55..e6884fd 100644
--- a/src/main/java/com/google/devtools/common/options/Converters.java
+++ b/src/main/java/com/google/devtools/common/options/Converters.java
@@ -14,6 +14,8 @@
package com.google.devtools.common.options;
import com.google.common.base.Splitter;
+import com.google.common.base.Strings;
+import com.google.common.cache.CacheBuilderSpec;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Maps;
@@ -612,4 +614,24 @@
super(0, 100);
}
}
+
+ /**
+ * A {@link Converter} for {@link CacheBuilderSpec}. The spec may be empty, in which case this
+ * converter returns null.
+ */
+ public static class CacheBuilderSpecConverter implements Converter<CacheBuilderSpec> {
+ @Override
+ public CacheBuilderSpec convert(String spec) throws OptionsParsingException {
+ try {
+ return Strings.isNullOrEmpty(spec) ? null : CacheBuilderSpec.parse(spec);
+ } catch (IllegalArgumentException e) {
+ throw new OptionsParsingException("Failed to parse CacheBuilderSpec: " + e.getMessage(), e);
+ }
+ }
+
+ @Override
+ public String getTypeDescription() {
+ return "Converts to a CacheBuilderSpec, or null if the input is empty";
+ }
+ }
}
