From db2a6e695e87eb42d5148b464458a4ee08c37de8 Mon Sep 17 00:00:00 2001 From: lberki Date: Tue, 1 Oct 2019 06:34:03 -0700 Subject: [PATCH] Handle the case when the BUILD file prelude is not set. RELNOTES: None. PiperOrigin-RevId: 272196422 --- .../build/lib/skyframe/PackageFunction.java | 52 +++++++++++-------- .../skyframe/ASTFileLookupFunctionTest.java | 20 +++---- .../SkylarkImportLookupFunctionTest.java | 7 --- 3 files changed, 42 insertions(+), 37 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/PackageFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/PackageFunction.java index 6b8ede878a97dd..6a2593e43da52c 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/PackageFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/PackageFunction.java @@ -414,28 +414,38 @@ public SkyValue compute(SkyKey key, Environment env) throws PackageFunctionExcep // Load the prelude from the same repository as the package being loaded. Can't use // Label.resolveRepositoryRelative because preludeLabel is in the main repository, not the // default one, so it is resolved to itself. - Label pkgPreludeLabel = - Label.createUnvalidated( - PackageIdentifier.create(packageId.getRepository(), preludeLabel.getPackageFragment()), - preludeLabel.getName()); - SkyKey astLookupKey = ASTFileLookupValue.key(pkgPreludeLabel); - ASTFileLookupValue astLookupValue = null; - try { - astLookupValue = (ASTFileLookupValue) env.getValueOrThrow(astLookupKey, - ErrorReadingSkylarkExtensionException.class, InconsistentFilesystemException.class); - } catch (ErrorReadingSkylarkExtensionException | InconsistentFilesystemException e) { - throw new PackageFunctionException( - new NoSuchPackageException( - packageId, "Error encountered while reading the prelude file: " + e.getMessage()), - Transience.PERSISTENT); - } - if (astLookupValue == null) { - return null; + List preludeStatements = ImmutableList.of(); + if (preludeLabel != null) { + Label pkgPreludeLabel = + Label.createUnvalidated( + PackageIdentifier.create( + packageId.getRepository(), preludeLabel.getPackageFragment()), + preludeLabel.getName()); + SkyKey astLookupKey = ASTFileLookupValue.key(pkgPreludeLabel); + ASTFileLookupValue astLookupValue = null; + try { + astLookupValue = + (ASTFileLookupValue) + env.getValueOrThrow( + astLookupKey, + ErrorReadingSkylarkExtensionException.class, + InconsistentFilesystemException.class); + } catch (ErrorReadingSkylarkExtensionException | InconsistentFilesystemException e) { + throw new PackageFunctionException( + new NoSuchPackageException( + packageId, "Error encountered while reading the prelude file: " + e.getMessage()), + Transience.PERSISTENT); + } + if (astLookupValue == null) { + return null; + } + + // The prelude file doesn't have to exist. If not, we substitute an empty statement list. + preludeStatements = + astLookupValue.lookupSuccessful() + ? astLookupValue.getAST().getStatements() + : ImmutableList.of(); } - // The prelude file doesn't have to exist. If not, we substitute an empty statement list. - List preludeStatements = - astLookupValue.lookupSuccessful() - ? astLookupValue.getAST().getStatements() : ImmutableList.of(); LoadedPackageCacheEntry packageCacheEntry = packageFunctionCache.getIfPresent(packageId); if (packageCacheEntry == null) { packageCacheEntry = diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/ASTFileLookupFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/ASTFileLookupFunctionTest.java index 3ef2e1830ffc59..82e71362995bd4 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/ASTFileLookupFunctionTest.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/ASTFileLookupFunctionTest.java @@ -28,6 +28,7 @@ import com.google.devtools.build.lib.vfs.FileStatus; import com.google.devtools.build.lib.vfs.FileSystem; import com.google.devtools.build.lib.vfs.Path; +import com.google.devtools.build.lib.vfs.PathFragment; import com.google.devtools.build.lib.vfs.inmemoryfs.InMemoryFileSystem; import com.google.devtools.build.skyframe.ErrorInfo; import com.google.devtools.build.skyframe.EvaluationResult; @@ -45,12 +46,11 @@ public class ASTFileLookupFunctionTest extends BuildViewTestCase { private class MockFileSystem extends InMemoryFileSystem { - boolean statThrowsIoException; + PathFragment throwIOExceptionFor = null; @Override public FileStatus statIfFound(Path path, boolean followSymlinks) throws IOException { - if (statThrowsIoException - && path.asFragment().getPathString().equals("/workspace/" + preludeLabelRelativePath)) { + if (throwIOExceptionFor != null && path.asFragment().equals(throwIOExceptionFor)) { throw new IOException("bork"); } return super.statIfFound(path, followSymlinks); @@ -58,8 +58,6 @@ public FileStatus statIfFound(Path path, boolean followSymlinks) throws IOExcept } private MockFileSystem mockFS; - String preludeLabelRelativePath = - getRuleClassProvider().getPreludeLabel().toPathFragment().toString(); @Override protected FileSystem createFileSystem() { @@ -69,10 +67,16 @@ protected FileSystem createFileSystem() { @Test public void testPreludeASTFileIsNotMandatory() throws Exception { + Label preludeLabel = getRuleClassProvider().getPreludeLabel(); + if (preludeLabel == null) { + // No prelude, no need to test + return; + } + reporter.removeHandler(failFastHandler); scratch.file( "foo/BUILD", "genrule(name = 'foo',", " outs = ['out.txt'],", " cmd = 'echo hello >@')"); - scratch.deleteFile(preludeLabelRelativePath); + scratch.deleteFile(preludeLabel.toPathFragment().getPathString()); invalidatePackages(); SkyKey skyKey = PackageValue.key(PackageIdentifier.parse("@//foo")); @@ -89,7 +93,7 @@ public void testIOExceptionOccursDuringReading() throws Exception { scratch.file("/workspace/tools/build_rules/BUILD"); scratch.file( "foo/BUILD", "genrule(name = 'foo',", " outs = ['out.txt'],", " cmd = 'echo hello >@')"); - mockFS.statThrowsIoException = true; + mockFS.throwIOExceptionFor = PathFragment.create("/workspace/foo/BUILD"); invalidatePackages(/*alsoConfigs=*/false); // We don't want to fail early on config creation. SkyKey skyKey = PackageValue.key(PackageIdentifier.parse("@//foo")); @@ -106,7 +110,6 @@ public void testIOExceptionOccursDuringReading() throws Exception { @Test public void testLoadFromBuildFileInRemoteRepo() throws Exception { - scratch.deleteFile(preludeLabelRelativePath); scratch.overwriteFile("WORKSPACE", "local_repository(", " name = 'a_remote_repo',", @@ -130,7 +133,6 @@ public void testLoadFromBuildFileInRemoteRepo() throws Exception { @Test public void testLoadFromSkylarkFileInRemoteRepo() throws Exception { - scratch.deleteFile(preludeLabelRelativePath); scratch.overwriteFile("WORKSPACE", "local_repository(", " name = 'a_remote_repo',", diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/SkylarkImportLookupFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/SkylarkImportLookupFunctionTest.java index afc018d4d7d40c..4465e6f9033826 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/SkylarkImportLookupFunctionTest.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/SkylarkImportLookupFunctionTest.java @@ -48,8 +48,6 @@ @RunWith(JUnit4.class) public class SkylarkImportLookupFunctionTest extends BuildViewTestCase { - String preludeLabelRelativePath; - @Before public final void preparePackageLoading() throws Exception { Path alternativeRoot = scratch.dir("/root_2"); @@ -69,8 +67,6 @@ public final void preparePackageLoading() throws Exception { ImmutableMap.of(), new TimestampGranularityMonitor(BlazeClock.instance())); skyframeExecutor.setActionEnv(ImmutableMap.of()); - this.preludeLabelRelativePath = - getRuleClassProvider().getPreludeLabel().toPathFragment().toString(); } @Test @@ -105,7 +101,6 @@ public void testSkylarkImportLabelsMultipleBuildFiles() throws Exception { @Test public void testLoadFromSkylarkFileInRemoteRepo() throws Exception { - scratch.deleteFile(preludeLabelRelativePath); scratch.overwriteFile( "WORKSPACE", "local_repository(", @@ -235,7 +230,6 @@ public void testSkylarkImportFilenameWithControlChars() throws Exception { @Test public void testLoadFromExternalRepoInWorkspaceFileAllowed() throws Exception { - scratch.deleteFile(preludeLabelRelativePath); Path p = scratch.overwriteFile( "WORKSPACE", @@ -369,7 +363,6 @@ public void testWithNonExistentRepository_And_DisallowLoadUsingLabelThatCrossesB @Test public void testLoadBzlFileFromWorkspaceWithRemapping() throws Exception { - scratch.deleteFile(preludeLabelRelativePath); Path p = scratch.overwriteFile( "WORKSPACE",