From b4f461ecc183d9adc9482f4cad848687ed0227ee Mon Sep 17 00:00:00 2001 From: John Cater Date: Tue, 25 Oct 2016 16:16:35 +0000 Subject: Add new skyframe function to lookup the repository given a path, and use that to report invalid package references. Fixes #1592. -- MOS_MIGRATED_REVID=137164164 --- .../ErrorDeterminingRepositoryException.java | 27 +++ .../devtools/build/lib/packages/Package.java | 17 +- .../skyframe/LocalRepositoryLookupFunction.java | 246 +++++++++++++++++++++ .../lib/skyframe/LocalRepositoryLookupValue.java | 166 ++++++++++++++ .../build/lib/skyframe/PackageLookupFunction.java | 47 +++- .../build/lib/skyframe/PackageLookupValue.java | 1 - .../devtools/build/lib/skyframe/SkyFunctions.java | 2 + .../build/lib/skyframe/SkyframeExecutor.java | 1 + .../lib/skyframe/SkylarkImportLookupValue.java | 6 +- .../build/lib/skyframe/WorkspaceFileFunction.java | 12 +- 10 files changed, 515 insertions(+), 10 deletions(-) create mode 100644 src/main/java/com/google/devtools/build/lib/packages/ErrorDeterminingRepositoryException.java create mode 100644 src/main/java/com/google/devtools/build/lib/skyframe/LocalRepositoryLookupFunction.java create mode 100644 src/main/java/com/google/devtools/build/lib/skyframe/LocalRepositoryLookupValue.java (limited to 'src/main/java/com/google/devtools/build/lib') diff --git a/src/main/java/com/google/devtools/build/lib/packages/ErrorDeterminingRepositoryException.java b/src/main/java/com/google/devtools/build/lib/packages/ErrorDeterminingRepositoryException.java new file mode 100644 index 0000000000..f8659aee4b --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/packages/ErrorDeterminingRepositoryException.java @@ -0,0 +1,27 @@ +// Copyright 2016 The Bazel Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.devtools.build.lib.packages; + +/** Exception indicating an attempt to access a repository which is not found or does not exist. */ +public class ErrorDeterminingRepositoryException extends NoSuchThingException { + + public ErrorDeterminingRepositoryException(String message) { + super(message); + } + + public ErrorDeterminingRepositoryException(String message, Throwable cause) { + super(message, cause); + } +} diff --git a/src/main/java/com/google/devtools/build/lib/packages/Package.java b/src/main/java/com/google/devtools/build/lib/packages/Package.java index 54ccc3e022..7e1fca4518 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/Package.java +++ b/src/main/java/com/google/devtools/build/lib/packages/Package.java @@ -15,6 +15,7 @@ package com.google.devtools.build.lib.packages; import com.google.common.annotations.VisibleForTesting; +import com.google.common.base.Predicate; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; @@ -475,11 +476,23 @@ public class Package { return (Rule) targets.get(targetName); } + /** Returns all rules in the package that match the given rule class. */ + public Iterable getRulesMatchingRuleClass(final String ruleClass) { + Iterable targets = getTargets(Rule.class); + return Iterables.filter( + targets, + new Predicate() { + @Override + public boolean apply(@Nullable Rule rule) { + return rule.getRuleClass().equals(ruleClass); + } + }); + } + /** * Returns this package's workspace name. * - *

Package-private to encourage callers to get their workspace name from a rule, not a - * package.

+ *

Package-private to encourage callers to get their workspace name from a rule, not a package. */ public String getWorkspaceName() { return workspaceName; diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/LocalRepositoryLookupFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/LocalRepositoryLookupFunction.java new file mode 100644 index 0000000000..4c97875c11 --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/skyframe/LocalRepositoryLookupFunction.java @@ -0,0 +1,246 @@ +// Copyright 2016 The Bazel Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +package com.google.devtools.build.lib.skyframe; + +import com.google.common.base.Optional; +import com.google.common.base.Predicate; +import com.google.common.collect.Iterables; +import com.google.devtools.build.lib.cmdline.Label; +import com.google.devtools.build.lib.cmdline.LabelSyntaxException; +import com.google.devtools.build.lib.cmdline.RepositoryName; +import com.google.devtools.build.lib.events.Event; +import com.google.devtools.build.lib.packages.AggregatingAttributeMapper; +import com.google.devtools.build.lib.packages.BuildFileNotFoundException; +import com.google.devtools.build.lib.packages.ErrorDeterminingRepositoryException; +import com.google.devtools.build.lib.packages.Package; +import com.google.devtools.build.lib.packages.Package.NameConflictException; +import com.google.devtools.build.lib.packages.Rule; +import com.google.devtools.build.lib.rules.repository.LocalRepositoryRule; +import com.google.devtools.build.lib.skyframe.PackageFunction.PackageFunctionException; +import com.google.devtools.build.lib.syntax.Type; +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 com.google.devtools.build.skyframe.SkyFunctionException.Transience; +import com.google.devtools.build.skyframe.SkyKey; +import com.google.devtools.build.skyframe.SkyValue; +import java.io.IOException; +import javax.annotation.Nullable; + +/** SkyFunction for {@link LocalRepositoryLookupValue}s. */ +public class LocalRepositoryLookupFunction implements SkyFunction { + + @Override + @Nullable + public String extractTag(SkyKey skyKey) { + return null; + } + + // Implementation note: Although LocalRepositoryLookupValue.NOT_FOUND exists, it should never be + // returned from this method. + @Override + public SkyValue compute(SkyKey skyKey, Environment env) + throws SkyFunctionException, InterruptedException { + RootedPath directory = (RootedPath) skyKey.argument(); + + // Is this the root directory? If so, we're in the MAIN repository. This assumes that the main + // repository has a WORKSPACE in the root directory, but Bazel will have failed with an error + // before this can be called if that is incorrect. + if (directory.getRelativePath().equals(PathFragment.EMPTY_FRAGMENT)) { + return LocalRepositoryLookupValue.mainRepository(); + } + + // Does this directory contain a WORKSPACE file? + Optional maybeWorkspaceFileExists = maybeGetWorkspaceFileExistence(env, directory); + if (!maybeWorkspaceFileExists.isPresent()) { + return null; + } else if (maybeWorkspaceFileExists.get()) { + Optional maybeRepository = + maybeCheckWorkspaceForRepository(env, directory); + if (!maybeRepository.isPresent()) { + return null; + } + LocalRepositoryLookupValue repository = maybeRepository.get(); + // If the repository that was discovered doesn't exist, continue recursing. + if (repository.exists()) { + return repository; + } + } + + // If we haven't found a repository yet, check the parent directory. + RootedPath parentDirectory = + RootedPath.toRootedPath( + directory.getRoot(), directory.getRelativePath().getParentDirectory()); + return env.getValue(LocalRepositoryLookupValue.key(parentDirectory)); + } + + private Optional maybeGetWorkspaceFileExistence(Environment env, RootedPath directory) + throws InterruptedException, LocalRepositoryLookupFunctionException { + try { + RootedPath workspaceRootedFile = + RootedPath.toRootedPath( + directory.getRoot(), + directory + .getRelativePath() + .getChild(PackageLookupValue.BuildFileName.WORKSPACE.getFilename())); + FileValue workspaceFileValue = + (FileValue) + env.getValueOrThrow( + FileValue.key(workspaceRootedFile), + IOException.class, + FileSymlinkException.class, + InconsistentFilesystemException.class); + if (workspaceFileValue == null) { + return Optional.absent(); + } + return Optional.of(workspaceFileValue.exists()); + } catch (IOException e) { + throw new LocalRepositoryLookupFunctionException( + new ErrorDeterminingRepositoryException( + "IOException while checking if there is a WORKSPACE file in " + + directory.asPath().getPathString(), + e), + Transience.PERSISTENT); + } catch (FileSymlinkException e) { + throw new LocalRepositoryLookupFunctionException( + new ErrorDeterminingRepositoryException( + "FileSymlinkException while checking if there is a WORKSPACE file in " + + directory.asPath().getPathString(), + e), + Transience.PERSISTENT); + } catch (InconsistentFilesystemException e) { + throw new LocalRepositoryLookupFunctionException( + new ErrorDeterminingRepositoryException( + "InconsistentFilesystemException while checking if there is a WORKSPACE file in " + + directory.asPath().getPathString(), + e), + Transience.PERSISTENT); + } + } + + /** + * Checks whether the directory exists and is a workspace root. Returns {@link Optional#absent()} + * if Skyframe needs to re-run, {@link Optional#of(LocalRepositoryLookupValue)} otherwise. + */ + private Optional maybeCheckWorkspaceForRepository( + Environment env, RootedPath directory) + throws InterruptedException, LocalRepositoryLookupFunctionException { + // Look up the main WORKSPACE file by the external package, to find all repositories. + PackageLookupValue externalPackageLookupValue; + try { + externalPackageLookupValue = + (PackageLookupValue) + env.getValueOrThrow( + PackageLookupValue.key(Label.EXTERNAL_PACKAGE_IDENTIFIER), + BuildFileNotFoundException.class, + InconsistentFilesystemException.class); + if (externalPackageLookupValue == null) { + return Optional.absent(); + } + } catch (BuildFileNotFoundException e) { + throw new LocalRepositoryLookupFunctionException( + new ErrorDeterminingRepositoryException( + "BuildFileNotFoundException while loading the //external package", e), + Transience.PERSISTENT); + } catch (InconsistentFilesystemException e) { + throw new LocalRepositoryLookupFunctionException( + new ErrorDeterminingRepositoryException( + "InconsistentFilesystemException while loading the //external package", e), + Transience.PERSISTENT); + } + + RootedPath workspacePath = + externalPackageLookupValue.getRootedPath(Label.EXTERNAL_PACKAGE_IDENTIFIER); + + SkyKey workspaceKey = WorkspaceFileValue.key(workspacePath); + do { + WorkspaceFileValue value; + try { + value = + (WorkspaceFileValue) + env.getValueOrThrow( + workspaceKey, PackageFunctionException.class, NameConflictException.class); + if (value == null) { + return Optional.absent(); + } + } catch (PackageFunctionException e) { + // TODO(jcater): When WFF is rewritten to not throw a PFE, update this. + throw new LocalRepositoryLookupFunctionException( + new ErrorDeterminingRepositoryException( + "PackageFunctionException while loading the root WORKSPACE file", e), + Transience.PERSISTENT); + } catch (NameConflictException e) { + throw new LocalRepositoryLookupFunctionException( + new ErrorDeterminingRepositoryException( + "NameConflictException while loading the root WORKSPACE file", e), + Transience.PERSISTENT); + } + + Package externalPackage = value.getPackage(); + if (externalPackage.containsErrors()) { + Event.replayEventsOn(env.getListener(), externalPackage.getEvents()); + } + + // Find all local_repository rules in the WORKSPACE, and check if any have a "path" attribute + // the same as the requested directory. + Iterable localRepositories = + externalPackage.getRulesMatchingRuleClass(LocalRepositoryRule.NAME); + Rule rule = + Iterables.find( + localRepositories, + new Predicate() { + @Override + public boolean apply(@Nullable Rule rule) { + AggregatingAttributeMapper mapper = AggregatingAttributeMapper.of(rule); + PathFragment pathAttr = new PathFragment(mapper.get("path", Type.STRING)); + return directory.getRelativePath().equals(pathAttr); + } + }, + null); + if (rule != null) { + try { + return Optional.of( + LocalRepositoryLookupValue.success(RepositoryName.create("@" + rule.getName()))); + } catch (LabelSyntaxException e) { + // This shouldn't be possible if the rule name is valid, and it should already have been + // validated. + throw new LocalRepositoryLookupFunctionException( + new ErrorDeterminingRepositoryException( + "LabelSyntaxException while creating the repository name from the rule " + + rule.getName(), + e), + Transience.PERSISTENT); + } + } + workspaceKey = value.next(); + + // TODO(bazel-team): This loop can be quadratic in the number of load() statements, consider + // rewriting or unrolling. + } while (workspaceKey != null); + + return Optional.of(LocalRepositoryLookupValue.notFound()); + } + + /** + * Used to declare all the exception types that can be wrapped in the exception thrown by {@link + * LocalRepositoryLookupFunction#compute}. + */ + private static final class LocalRepositoryLookupFunctionException extends SkyFunctionException { + public LocalRepositoryLookupFunctionException( + ErrorDeterminingRepositoryException e, Transience transience) { + super(e, transience); + } + } +} diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/LocalRepositoryLookupValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/LocalRepositoryLookupValue.java new file mode 100644 index 0000000000..794dcc37d6 --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/skyframe/LocalRepositoryLookupValue.java @@ -0,0 +1,166 @@ +// Copyright 2016 The Bazel Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +package com.google.devtools.build.lib.skyframe; + +import com.google.devtools.build.lib.cmdline.RepositoryName; +import com.google.devtools.build.lib.vfs.RootedPath; +import com.google.devtools.build.skyframe.SkyKey; +import com.google.devtools.build.skyframe.SkyValue; + +/** + * A value that represents a local repository lookup result. + * + *

Local repository lookups will always produce a value. The {@code #getRepository} method + * returns the name of the repository that the directory resides in. + */ +public abstract class LocalRepositoryLookupValue implements SkyValue { + + static SkyKey key(RootedPath directory) { + return SkyKey.create(SkyFunctions.LOCAL_REPOSITORY_LOOKUP, directory); + } + + private static final LocalRepositoryLookupValue MAIN_REPO_VALUE = new MainRepositoryLookupValue(); + private static final LocalRepositoryLookupValue NOT_FOUND_VALUE = + new NotFoundLocalRepositoryLookupValue(); + + public static LocalRepositoryLookupValue mainRepository() { + return MAIN_REPO_VALUE; + } + + public static LocalRepositoryLookupValue success(RepositoryName repositoryName) { + return new SuccessfulLocalRepositoryLookupValue(repositoryName); + } + + public static LocalRepositoryLookupValue notFound() { + return NOT_FOUND_VALUE; + } + + /** + * Returns {@code true} if the local repository lookup succeeded and the {@link #getRepository} + * method will return a useful result. + */ + public abstract boolean exists(); + + /** + * Returns the {@link RepositoryName} of the local repository contained in the directory which was + * looked up, {@link RepositoryName#MAIN} if the directory is part of the main repository, or + * throws a {@link IllegalStateException} if there was no repository found. + */ + public abstract RepositoryName getRepository(); + + /** Represents a successful lookup of the main repository. */ + public static final class MainRepositoryLookupValue extends LocalRepositoryLookupValue { + + // This should be a singleton value. + private MainRepositoryLookupValue() {} + + @Override + public boolean exists() { + return true; + } + + @Override + public RepositoryName getRepository() { + return RepositoryName.MAIN; + } + + @Override + public String toString() { + return "MainRepositoryLookupValue"; + } + + @Override + public boolean equals(Object obj) { + // All MainRepositoryLookupValue instances are equivalent. + return obj instanceof MainRepositoryLookupValue; + } + + @Override + public int hashCode() { + return MainRepositoryLookupValue.class.getSimpleName().hashCode(); + } + } + + /** Represents a successful lookup of a local repository. */ + public static final class SuccessfulLocalRepositoryLookupValue + extends LocalRepositoryLookupValue { + private final RepositoryName repositoryName; + + public SuccessfulLocalRepositoryLookupValue(RepositoryName repositoryName) { + this.repositoryName = repositoryName; + } + + @Override + public boolean exists() { + return true; + } + + @Override + public RepositoryName getRepository() { + return repositoryName; + } + + @Override + public String toString() { + return "SuccessfulLocalRepositoryLookupValue(" + repositoryName + ")"; + } + + @Override + public boolean equals(Object obj) { + if (!(obj instanceof SuccessfulLocalRepositoryLookupValue)) { + return false; + } + SuccessfulLocalRepositoryLookupValue other = (SuccessfulLocalRepositoryLookupValue) obj; + return repositoryName.equals(other.repositoryName); + } + + @Override + public int hashCode() { + return repositoryName.hashCode(); + } + } + + /** Represents the state where no repository was found, either local or the main repository. */ + public static final class NotFoundLocalRepositoryLookupValue extends LocalRepositoryLookupValue { + + // This should be a singleton value. + private NotFoundLocalRepositoryLookupValue() {} + + @Override + public boolean exists() { + return false; + } + + @Override + public RepositoryName getRepository() { + throw new IllegalStateException("Repository was not found"); + } + + @Override + public String toString() { + return "NotFoundLocalRepositoryLookupValue"; + } + + @Override + public boolean equals(Object obj) { + // All NotFoundLocalRepositoryLookupValue instances are equivalent. + return obj instanceof NotFoundLocalRepositoryLookupValue; + } + + @Override + public int hashCode() { + return NotFoundLocalRepositoryLookupValue.class.getSimpleName().hashCode(); + } + } +} diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/PackageLookupFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/PackageLookupFunction.java index 8654501138..331a081b4e 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/PackageLookupFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/PackageLookupFunction.java @@ -19,10 +19,12 @@ import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.cmdline.LabelValidator; import com.google.devtools.build.lib.cmdline.PackageIdentifier; import com.google.devtools.build.lib.packages.BuildFileNotFoundException; +import com.google.devtools.build.lib.packages.ErrorDeterminingRepositoryException; import com.google.devtools.build.lib.packages.NoSuchPackageException; import com.google.devtools.build.lib.pkgcache.PathPackageLocator; import com.google.devtools.build.lib.skyframe.PackageLookupValue.BuildFileName; import com.google.devtools.build.lib.syntax.EvalException; +import com.google.devtools.build.lib.util.Preconditions; import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.PathFragment; import com.google.devtools.build.lib.vfs.RootedPath; @@ -61,6 +63,7 @@ public class PackageLookupFunction implements SkyFunction { public SkyValue compute(SkyKey skyKey, Environment env) throws PackageLookupFunctionException, InterruptedException { PathPackageLocator pkgLocator = PrecomputedValue.PATH_PACKAGE_LOCATOR.get(env); + PackageIdentifier packageKey = (PackageIdentifier) skyKey.argument(); if (PackageFunction.isDefaultsPackage(packageKey)) { return PackageLookupValue.success(pkgLocator.getPathEntries().get(0), BuildFileName.BUILD); @@ -149,10 +152,50 @@ public class PackageLookupFunction implements SkyFunction { RootedPath buildFileRootedPath = RootedPath.toRootedPath(packagePathEntry, buildFileFragment); - if (crossRepositoryLabelViolationStrategy != CrossRepositoryLabelViolationStrategy.IGNORE) { - // TODO(jcater): Check for cross repository package label violations. + if (crossRepositoryLabelViolationStrategy == CrossRepositoryLabelViolationStrategy.ERROR) { + // Is this path part of a local repository? + RootedPath currentPath = + RootedPath.toRootedPath(packagePathEntry, buildFileFragment.getParentDirectory()); + SkyKey repositoryLookupKey = LocalRepositoryLookupValue.key(currentPath); + + // TODO(jcater): Consider parallelizing these lookups. + LocalRepositoryLookupValue localRepository; + try { + localRepository = + (LocalRepositoryLookupValue) + env.getValueOrThrow( + repositoryLookupKey, ErrorDeterminingRepositoryException.class); + if (localRepository == null) { + return null; + } + } catch (ErrorDeterminingRepositoryException e) { + // If the directory selected isn't part of a repository, that's an error. + // TODO(katre): Improve the error message given here. + throw new PackageLookupFunctionException( + new BuildFileNotFoundException( + packageIdentifier, + "Unable to determine the local repository for directory " + + currentPath.asPath().getPathString()), + Transience.PERSISTENT); + } + + if (localRepository.exists() + && !localRepository.getRepository().equals(packageIdentifier.getRepository())) { + // There is a repository mismatch, this is an error. + // TODO(jcater): Work out the correct package name for this error message. + return PackageLookupValue.invalidPackageName( + "Package crosses into repository " + localRepository.getRepository().getName()); + } + + // There's no local repository, keep going. + } else { + // Future-proof against adding future values to CrossRepositoryLabelViolationStrategy. + Preconditions.checkState( + crossRepositoryLabelViolationStrategy == CrossRepositoryLabelViolationStrategy.IGNORE, + crossRepositoryLabelViolationStrategy); } + // Check for the existence of the build file. FileValue fileValue = getFileValue(buildFileRootedPath, env, packageIdentifier); if (fileValue == null) { return null; diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/PackageLookupValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/PackageLookupValue.java index a9861b727a..8ad5ae0c06 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/PackageLookupValue.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/PackageLookupValue.java @@ -15,7 +15,6 @@ package com.google.devtools.build.lib.skyframe; import com.google.common.base.Objects; import com.google.devtools.build.lib.cmdline.PackageIdentifier; -import com.google.devtools.build.lib.skyframe.PackageLookupValue.BuildFileName; import com.google.devtools.build.lib.util.Preconditions; import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.PathFragment; diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyFunctions.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyFunctions.java index 09dc01baa2..3e138adaff 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyFunctions.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyFunctions.java @@ -104,6 +104,8 @@ public final class SkyFunctions { public static final SkyFunctionName EXTERNAL_PACKAGE = SkyFunctionName.create("EXTERNAL_PACKAGE"); public static final SkyFunctionName ACTION_TEMPLATE_EXPANSION = SkyFunctionName.create("ACTION_TEMPLATE_EXPANSION"); + public static final SkyFunctionName LOCAL_REPOSITORY_LOOKUP = + SkyFunctionName.create("LOCAL_REPOSITORY_LOOKUP"); public static Predicate isSkyFunction(final SkyFunctionName functionName) { return new Predicate() { diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java index 146c468aaf..39cf8c8e06 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java @@ -417,6 +417,7 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory { new RecursiveFilesystemTraversalFunction()); map.put(SkyFunctions.FILESET_ENTRY, new FilesetEntryFunction()); map.put(SkyFunctions.ACTION_TEMPLATE_EXPANSION, new ActionTemplateExpansionFunction()); + map.put(SkyFunctions.LOCAL_REPOSITORY_LOOKUP, new LocalRepositoryLookupFunction()); map.putAll(extraSkyFunctions); return map.build(); } diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkImportLookupValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkImportLookupValue.java index 201e8fcab5..d97c034dfe 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkImportLookupValue.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkImportLookupValue.java @@ -19,7 +19,6 @@ import com.google.devtools.build.lib.syntax.Environment.Extension; import com.google.devtools.build.lib.util.Preconditions; import com.google.devtools.build.skyframe.SkyKey; import com.google.devtools.build.skyframe.SkyValue; - import java.io.Serializable; import java.util.Objects; @@ -74,6 +73,11 @@ public class SkylarkImportLookupValue implements SkyValue { this.inWorkspace = inWorkspace; } + @Override + public String toString() { + return importLabel + (inWorkspace ? " (in workspace)" : ""); + } + @Override public boolean equals(Object obj) { if (this == obj) { diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceFileFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceFileFunction.java index 87ec20c159..faacb3c92d 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceFileFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceFileFunction.java @@ -110,7 +110,11 @@ public class WorkspaceFileFunction implements SkyFunction { return null; } parser.execute(ast, importResult.importMap); - } catch (PackageFunctionException | NameConflictException e) { + } catch (PackageFunctionException e) { + // TODO(jcater): Unwrap the PackageFunctionException and handle the underlying error. + // PFE shouldn't be exposed to callers. + throw new WorkspaceFileFunctionException(e, Transience.PERSISTENT); + } catch (NameConflictException e) { throw new WorkspaceFileFunctionException(e, Transience.PERSISTENT); } @@ -129,12 +133,12 @@ public class WorkspaceFileFunction implements SkyFunction { } private static final class WorkspaceFileFunctionException extends SkyFunctionException { - public WorkspaceFileFunctionException(Exception e, Transience transience) { + public WorkspaceFileFunctionException(PackageFunctionException e, Transience transience) { super(e, transience); } - public WorkspaceFileFunctionException(Exception e) { - super(e, Transience.PERSISTENT); + public WorkspaceFileFunctionException(NameConflictException e, Transience transience) { + super(e, transience); } } } -- cgit v1.2.3