Store content digests in repository marker files. https://www.bazel.build/designs/2016/10/18/repository-invalidation.html
Change-Id: I6cb01397a35cd32169a0e415f8d7f944e7d840df
PiperOrigin-RevId: 166200841
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryContext.java b/src/main/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryContext.java
index 72a873c..de78271 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryContext.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryContext.java
@@ -714,13 +714,17 @@
if (fileValue == null) {
throw RepositoryFunction.restart();
}
- if (!fileValue.isFile()) {
- throw new EvalException(Location.BUILTIN,
- "Not a file: " + rootedPath.asPath().getPathString());
+ if (!fileValue.isFile() || fileValue.isSpecialFile()) {
+ throw new EvalException(
+ Location.BUILTIN, "Not a regular file: " + rootedPath.asPath().getPathString());
}
- // A label do not contains space so it safe to use as a key.
- markerData.put("FILE:" + label, Integer.toString(fileValue.realFileStateValue().hashCode()));
+ // A label does not contains space so it safe to use as a key.
+ try {
+ markerData.put("FILE:" + label, RepositoryFunction.fileValueToMarkerValue(fileValue));
+ } catch (IOException e) {
+ throw new EvalException(Location.BUILTIN, e);
+ }
return new SkylarkPath(rootedPath.asPath());
}
diff --git a/src/main/java/com/google/devtools/build/lib/rules/repository/NewRepositoryFileHandler.java b/src/main/java/com/google/devtools/build/lib/rules/repository/NewRepositoryFileHandler.java
index 094f5e5..6317a65 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/repository/NewRepositoryFileHandler.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/repository/NewRepositoryFileHandler.java
@@ -157,8 +157,11 @@
// TODO(pcloudy): Don't add absolute path into markerData once it's not supported
fileKey = fileValue.realRootedPath().asPath().getPathString();
}
- markerData.put(
- "FILE:" + fileKey, Integer.toString(fileValue.realFileStateValue().hashCode()));
+ try {
+ markerData.put("FILE:" + fileKey, RepositoryFunction.fileValueToMarkerValue(fileValue));
+ } catch (IOException e) {
+ throw new RepositoryFunctionException(e, Transience.TRANSIENT);
+ }
} else if (fileContent != null) {
RepositoryFunction.writeFile(outputDirectory, filename, fileContent);
} else {
@@ -265,6 +268,13 @@
Transience.TRANSIENT);
}
+ if (!fileValue.isFile() || fileValue.isSpecialFile()) {
+ throw new RepositoryFunctionException(
+ new EvalException(
+ rule.getLocation(), String.format("%s is not a regular file", rootedFile.asPath())),
+ Transience.PERSISTENT);
+ }
+
return fileValue;
}
diff --git a/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryFunction.java b/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryFunction.java
index 469eb61..e82ae7b 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryFunction.java
@@ -17,6 +17,7 @@
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableList;
+import com.google.common.io.BaseEncoding;
import com.google.devtools.build.lib.analysis.BlazeDirectories;
import com.google.devtools.build.lib.analysis.RuleDefinition;
import com.google.devtools.build.lib.cmdline.Label;
@@ -30,6 +31,7 @@
import com.google.devtools.build.lib.packages.Rule;
import com.google.devtools.build.lib.rules.ExternalPackageUtil;
import com.google.devtools.build.lib.skyframe.ActionEnvironmentFunction;
+import com.google.devtools.build.lib.skyframe.FileStateValue.RegularFileStateValue;
import com.google.devtools.build.lib.skyframe.FileSymlinkException;
import com.google.devtools.build.lib.skyframe.FileValue;
import com.google.devtools.build.lib.skyframe.InconsistentFilesystemException;
@@ -222,11 +224,11 @@
FileSymlinkException.class,
InconsistentFilesystemException.class);
- if (fileValue == null || !fileValue.isFile()) {
+ if (fileValue == null || !fileValue.isFile() || fileValue.isSpecialFile()) {
return false;
}
- return Objects.equals(value, Integer.toString(fileValue.realFileStateValue().hashCode()));
+ return Objects.equals(value, fileValueToMarkerValue(fileValue));
} catch (LabelSyntaxException e) {
throw new IllegalStateException(
"Key " + key + " is not a correct file key (should be in form FILE:label)", e);
@@ -239,6 +241,22 @@
}
}
+ /**
+ * Convert to a @{link com.google.devtools.build.lib.skyframe.FileValue} to a String appropriate
+ * for placing in a repository marker file.
+ *
+ * @param fileValue The value to convert. It must correspond to a regular file.
+ */
+ public static String fileValueToMarkerValue(FileValue fileValue) throws IOException {
+ Preconditions.checkArgument(fileValue.isFile() && !fileValue.isSpecialFile());
+ // Return the file content digest in hex. fileValue may or may not have the digest available.
+ byte[] digest = ((RegularFileStateValue) fileValue.realFileStateValue()).getDigest();
+ if (digest == null) {
+ digest = fileValue.realRootedPath().asPath().getDigest();
+ }
+ return BaseEncoding.base16().lowerCase().encode(digest);
+ }
+
static boolean verifyMarkerDataForFiles(
Rule rule, Map<String, String> markerData, Environment env) throws InterruptedException {
for (Map.Entry<String, String> entry : markerData.entrySet()) {
diff --git a/src/test/java/com/google/devtools/build/lib/rules/repository/RepositoryFunctionTest.java b/src/test/java/com/google/devtools/build/lib/rules/repository/RepositoryFunctionTest.java
index d550924..7220670 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/repository/RepositoryFunctionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/repository/RepositoryFunctionTest.java
@@ -17,13 +17,20 @@
import static com.google.common.truth.Truth.assertThat;
import com.google.common.annotations.VisibleForTesting;
+import com.google.common.io.BaseEncoding;
import com.google.devtools.build.lib.analysis.BlazeDirectories;
import com.google.devtools.build.lib.analysis.RuleDefinition;
import com.google.devtools.build.lib.analysis.util.BuildViewTestCase;
import com.google.devtools.build.lib.packages.Rule;
+import com.google.devtools.build.lib.skyframe.FileContentsProxy;
+import com.google.devtools.build.lib.skyframe.FileStateValue;
+import com.google.devtools.build.lib.skyframe.FileStateValue.RegularFileStateValue;
+import com.google.devtools.build.lib.skyframe.FileValue;
+import com.google.devtools.build.lib.skyframe.FileValue.RegularFileValue;
import com.google.devtools.build.lib.vfs.FileSystemUtils;
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.build.lib.vfs.PathFragment;
+import com.google.devtools.build.lib.vfs.RootedPath;
import com.google.devtools.build.skyframe.SkyFunction;
import com.google.devtools.build.skyframe.SkyFunctionException;
import java.util.Map;
@@ -112,4 +119,20 @@
assertMarkerFileEscaping("a \\\nb");
assertMarkerFileEscaping("a \nb");
}
+
+ @Test
+ public void testFileValueToMarkerValue() throws Exception {
+ RootedPath path = RootedPath.toRootedPath(rootDirectory, scratch.file("foo", "bar"));
+
+ // Digest should be returned if the FileStateValue has it.
+ FileStateValue fsv = new RegularFileStateValue(3, 100, new byte[] {1, 2, 3, 4}, null);
+ FileValue fv = new RegularFileValue(path, fsv);
+ assertThat(RepositoryFunction.fileValueToMarkerValue(fv)).isEqualTo("01020304");
+
+ // Digest should also be returned if the FileStateValue doesn't have it.
+ fsv = new RegularFileStateValue(3, 100, null, new FileContentsProxy(100, 200));
+ fv = new RegularFileValue(path, fsv);
+ String expectedDigest = BaseEncoding.base16().lowerCase().encode(path.asPath().getDigest());
+ assertThat(RepositoryFunction.fileValueToMarkerValue(fv)).isEqualTo(expectedDigest);
+ }
}
diff --git a/src/test/py/bazel/bazel_external_repository_test.py b/src/test/py/bazel/bazel_external_repository_test.py
index 098aac7..50dcb57 100644
--- a/src/test/py/bazel/bazel_external_repository_test.py
+++ b/src/test/py/bazel/bazel_external_repository_test.py
@@ -58,7 +58,23 @@
exit_code, _, stderr = self.RunBazel(['build', '@six_archive//...'])
self.assertEqual(exit_code, 0, os.linesep.join(stderr))
- # Test Repository reloading after build file changes
+ fetching_disabled_msg = 'fetching is disabled'
+
+ # Changing the mtime of the BUILD file shouldn't invalidate it.
+ os.utime(self.Path('third_party/six.BUILD'), (100, 200))
+ exit_code, _, stderr = self.RunBazel(
+ ['build', '--nofetch', '@six_archive//...'])
+ self.assertEqual(exit_code, 0, os.linesep.join(stderr))
+ self.assertNotIn(fetching_disabled_msg, os.linesep.join(stderr))
+
+ # Check that --nofetch prints a warning if the BUILD file is changed.
+ self.ScratchFile('third_party/six.BUILD', build_file + ['"a noop string"'])
+ exit_code, _, stderr = self.RunBazel(
+ ['build', '--nofetch', '@six_archive//...'])
+ self.assertEqual(exit_code, 0, os.linesep.join(stderr))
+ self.assertIn(fetching_disabled_msg, os.linesep.join(stderr))
+
+ # Test repository reloading after BUILD file changes.
self.ScratchFile('third_party/six.BUILD', build_file + ['foobar'])
exit_code, _, stderr = self.RunBazel(['build', '@six_archive//...'])
self.assertEqual(exit_code, 1, os.linesep.join(stderr))
