From 847a41816c08b846e09c62834f28fbbcb08d5d6d Mon Sep 17 00:00:00 2001 From: Damien Martin-Guillerez Date: Wed, 10 Feb 2016 12:44:25 +0000 Subject: Make ExternalFilesHelper depends on the repository rule and not on the parsed Workspace. Issue #824 Step 4. -- MOS_MIGRATED_REVID=114314036 --- .../build/lib/skyframe/ExternalFilesHelper.java | 33 ++++++++++++++++------ 1 file changed, 25 insertions(+), 8 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ExternalFilesHelper.java b/src/main/java/com/google/devtools/build/lib/skyframe/ExternalFilesHelper.java index fabe2b5fc0..4bf976ab06 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/ExternalFilesHelper.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/ExternalFilesHelper.java @@ -15,8 +15,9 @@ package com.google.devtools.build.lib.skyframe; import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.pkgcache.PathPackageLocator; -import com.google.devtools.build.lib.util.Preconditions; +import com.google.devtools.build.lib.rules.repository.RepositoryFunction; 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; @@ -84,12 +85,13 @@ public class ExternalFilesHelper { // The outputBase may be null if we're not actually running a build. Path outputBase = pkgLocator.get().getOutputBase(); - if (outputBase != null && !rootedPath.asPath().startsWith( - pkgLocator.get().getOutputBase().getRelative(Label.EXTERNAL_PATH_PREFIX))) { + Path relativeExternal = pkgLocator + .get().getOutputBase().getRelative(Label.EXTERNAL_PATH_PREFIX); + if (outputBase != null && !rootedPath.asPath().startsWith(relativeExternal)) { return; } - // For files that are under $OUTPUT_BASE/external, add a dependency on the //external package + // For files that are under $OUTPUT_BASE/external, add a dependency on the corresponding rule // so that if the WORKSPACE file changes, the File/DirectoryStateValue will be re-evaluated. // // Note that: @@ -100,11 +102,26 @@ public class ExternalFilesHelper { // skyframe graph in the first place is through symlinks outside the package roots, which we // neither want to encourage nor optimize for since it is not common. So the set of external // files is small. - PackageValue pkgValue = (PackageValue) env.getValue(PackageValue.key( - Label.EXTERNAL_PACKAGE_IDENTIFIER)); - if (pkgValue == null) { + + PathFragment repositoryPath = rootedPath.asPath().relativeTo(relativeExternal); + if (repositoryPath.segmentCount() == 0) { + // We are the top of the repository path (/external), not in an actual external + // repository path. return; } - Preconditions.checkState(!pkgValue.getPackage().containsErrors()); + String repositoryName = repositoryPath.getSegment(0); + + try { + RepositoryFunction.getRule(repositoryName, env); + } catch (RepositoryFunction.RepositoryNotFoundException ex) { + // The repository we are looking for does not exist so we should depend on the whole + // WORKSPACE file. In that case, the call to RepositoryFunction#getRule(String, Environment) + // already requested all repository functions from the WORKSPACE file from Skyframe as part of + // the resolution. Therefore we are safe to ignore that Exception. + } catch (RepositoryFunction.RepositoryFunctionException ex) { + // This should never happen. + throw new IllegalStateException( + "Repository " + repositoryName + " cannot be resolved for path " + rootedPath, ex); + } } } -- cgit v1.2.3