Skip to content

Commit

Permalink
Handle the case when the BUILD file prelude is not set.
Browse files Browse the repository at this point in the history 
RELNOTES: None.
PiperOrigin-RevId: 272196422
  • Loading branch information
@lberki @copybara-github
lberki authored and copybara-github committed Oct 1, 2019
1 parent 5142172 commit db2a6e6
Show file tree
Hide file tree
Showing 3 changed files with 42 additions and 37 deletions.
52 changes: 31 additions & 21 deletions src/main/java/com/google/devtools/build/lib/skyframe/PackageFunction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<Statement> 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.<Statement>of();
}
// The prelude file doesn't have to exist. If not, we substitute an empty statement list.
List<Statement> preludeStatements =
astLookupValue.lookupSuccessful()
? astLookupValue.getAST().getStatements() : ImmutableList.<Statement>of();
LoadedPackageCacheEntry packageCacheEntry = packageFunctionCache.getIfPresent(packageId);
if (packageCacheEntry == null) {
packageCacheEntry =
Expand Down
20 changes: 11 additions & 9 deletions src/test/java/com/google/devtools/build/lib/skyframe/ASTFileLookupFunctionTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -45,21 +46,18 @@
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);
}
}

private MockFileSystem mockFS;
String preludeLabelRelativePath =
getRuleClassProvider().getPreludeLabel().toPathFragment().toString();

@Override
protected FileSystem createFileSystem() {
Expand All @@ -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"));
Expand All @@ -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"));
Expand All @@ -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',",
Expand All @@ -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',",
Expand Down
7 changes: 0 additions & 7 deletions src/test/java/com/google/devtools/build/lib/skyframe/SkylarkImportLookupFunctionTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand All @@ -69,8 +67,6 @@ public final void preparePackageLoading() throws Exception {
ImmutableMap.<String, String>of(),
new TimestampGranularityMonitor(BlazeClock.instance()));
skyframeExecutor.setActionEnv(ImmutableMap.<String, String>of());
this.preludeLabelRelativePath =
getRuleClassProvider().getPreludeLabel().toPathFragment().toString();
}

@Test
Expand Down Expand Up @@ -105,7 +101,6 @@ public void testSkylarkImportLabelsMultipleBuildFiles() throws Exception {

@Test
public void testLoadFromSkylarkFileInRemoteRepo() throws Exception {
scratch.deleteFile(preludeLabelRelativePath);
scratch.overwriteFile(
"WORKSPACE",
"local_repository(",
Expand Down Expand Up @@ -235,7 +230,6 @@ public void testSkylarkImportFilenameWithControlChars() throws Exception {

@Test
public void testLoadFromExternalRepoInWorkspaceFileAllowed() throws Exception {
scratch.deleteFile(preludeLabelRelativePath);
Path p =
scratch.overwriteFile(
"WORKSPACE",
Expand Down Expand Up @@ -369,7 +363,6 @@ public void testWithNonExistentRepository_And_DisallowLoadUsingLabelThatCrossesB

@Test
public void testLoadBzlFileFromWorkspaceWithRemapping() throws Exception {
scratch.deleteFile(preludeLabelRelativePath);
Path p =
scratch.overwriteFile(
"WORKSPACE",
Expand Down

0 comments on commit db2a6e6

Please sign in to comment.