diff options
Diffstat (limited to 'src')
3 files changed, 57 insertions, 22 deletions
diff --git a/src/main/java/com/google/devtools/build/skyframe/InMemoryMemoizingEvaluator.java b/src/main/java/com/google/devtools/build/skyframe/InMemoryMemoizingEvaluator.java index 1750263b43..e8b1f634ce 100644 --- a/src/main/java/com/google/devtools/build/skyframe/InMemoryMemoizingEvaluator.java +++ b/src/main/java/com/google/devtools/build/skyframe/InMemoryMemoizingEvaluator.java @@ -205,11 +205,19 @@ public final class InMemoryMemoizingEvaluator implements MemoizingEvaluator { if (prevEntry != null && prevEntry.isDone()) { try { Iterable<SkyKey> directDeps = prevEntry.getDirectDeps(); - Preconditions.checkState( - Iterables.isEmpty(directDeps), "existing entry for %s has deps: %s", key, directDeps); - if (newValue.equals(prevEntry.getValue()) - && !valuesToDirty.contains(key) - && !valuesToDelete.contains(key)) { + if (Iterables.isEmpty(directDeps)) { + if (newValue.equals(prevEntry.getValue()) + && !valuesToDirty.contains(key) + && !valuesToDelete.contains(key)) { + it.remove(); + } + } else { + // Rare situation of an injected dep that depends on another node. Usually the dep is + // the error transience node. When working with external repositories, it can also be an + // external workspace file. Don't bother injecting it, just invalidate it. + // We'll wastefully evaluate the node freshly during evaluation, but this happens very + // rarely. + valuesToDirty.add(key); it.remove(); } } catch (InterruptedException e) { diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/FileFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/FileFunctionTest.java index 50b98a6967..4c349f831f 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/FileFunctionTest.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/FileFunctionTest.java @@ -44,19 +44,21 @@ import com.google.devtools.build.lib.testutil.ManualClock; import com.google.devtools.build.lib.testutil.TestConstants; import com.google.devtools.build.lib.testutil.TestRuleClassProvider; import com.google.devtools.build.lib.testutil.TestUtils; +import com.google.devtools.build.lib.util.BlazeClock; import com.google.devtools.build.lib.util.Pair; import com.google.devtools.build.lib.util.Preconditions; import com.google.devtools.build.lib.util.io.TimestampGranularityMonitor; import com.google.devtools.build.lib.vfs.FileStatus; import com.google.devtools.build.lib.vfs.FileSystem; -import com.google.devtools.build.lib.vfs.FileSystem.HashFunction; 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.lib.vfs.inmemoryfs.InMemoryFileSystem; import com.google.devtools.build.lib.vfs.util.FileSystems; +import com.google.devtools.build.skyframe.BuildDriver; import com.google.devtools.build.skyframe.ErrorInfo; +import com.google.devtools.build.skyframe.ErrorInfoSubject; import com.google.devtools.build.skyframe.EvaluationResult; import com.google.devtools.build.skyframe.InMemoryMemoizingEvaluator; import com.google.devtools.build.skyframe.MemoizingEvaluator; @@ -1272,6 +1274,41 @@ public class FileFunctionTest { assertThat(valueForPath(child).exists()).isFalse(); } + @Test + public void testInjectionOverIOException() throws Exception { + Path foo = file("foo"); + SkyKey fooKey = skyKey("foo"); + fs.stubStatError(foo, new IOException("bork")); + BuildDriver driver = makeDriver(); + EvaluationResult<FileValue> result = + driver.evaluate( + ImmutableList.of(fooKey), + /*keepGoing=*/ true, + /*numThreads=*/ 1, + NullEventHandler.INSTANCE); + ErrorInfoSubject errorInfoSubject = assertThatEvaluationResult(result) + .hasErrorEntryForKeyThat(fooKey); + errorInfoSubject.isTransient(); + errorInfoSubject + .hasExceptionThat() + .hasMessageThat() + .isEqualTo("bork"); + fs.stubbedStatErrors.remove(foo); + differencer.inject( + fileStateSkyKey("foo"), + FileStateValue.create( + RootedPath.toRootedPath(pkgRoot, foo), + new TimestampGranularityMonitor(BlazeClock.instance()))); + result = + driver.evaluate( + ImmutableList.of(fooKey), + /*keepGoing=*/ true, + /*numThreads=*/ 1, + NullEventHandler.INSTANCE); + assertThatEvaluationResult(result).hasNoError(); + assertThat(result.get(fooKey).exists()).isTrue(); + } + private void checkRealPath(String pathString) throws Exception { Path realPath = pkgRoot.getRelative(pathString).resolveSymbolicLinks(); assertRealPath(pathString, realPath.relativeTo(pkgRoot).toString()); diff --git a/src/test/java/com/google/devtools/build/skyframe/MemoizingEvaluatorTest.java b/src/test/java/com/google/devtools/build/skyframe/MemoizingEvaluatorTest.java index 7e21d9ee18..df97dc6b06 100644 --- a/src/test/java/com/google/devtools/build/skyframe/MemoizingEvaluatorTest.java +++ b/src/test/java/com/google/devtools/build/skyframe/MemoizingEvaluatorTest.java @@ -3773,14 +3773,11 @@ public class MemoizingEvaluatorTest { tester.getOrCreate(key).addDependency("other").setComputedValue(COPY); assertThat(tester.evalAndGet("value")).isEqualTo(prevVal); tester.differencer.inject(ImmutableMap.of(key, val)); - try { - tester.evalAndGet("value"); - fail("injection over value with deps should have failed"); - } catch (IllegalStateException e) { - assertThat(e).hasMessage( - "existing entry for " + NODE_TYPE.getName() + ":value has deps: " - + "[" + NODE_TYPE.getName() + ":other]"); - } + StringValue depVal = new StringValue("newfoo"); + tester.getOrCreate("other").setConstantValue(depVal); + tester.differencer.invalidate(ImmutableList.of(GraphTester.toSkyKey("other"))); + // Injected value is ignored for value with deps. + assertThat(tester.evalAndGet("value")).isEqualTo(depVal); } @Test @@ -3792,14 +3789,7 @@ public class MemoizingEvaluatorTest { tester.getOrCreate(key).addDependency("other").setComputedValue(COPY); assertThat(tester.evalAndGet("value")).isEqualTo(val); tester.differencer.inject(ImmutableMap.of(key, val)); - try { - tester.evalAndGet("value"); - fail("injection over value with deps should have failed"); - } catch (IllegalStateException e) { - assertThat(e).hasMessage( - "existing entry for " + NODE_TYPE.getName() + ":value has deps: " - + "[" + NODE_TYPE.getName() + ":other]"); - } + assertThat(tester.evalAndGet("value")).isEqualTo(val); } @Test |