From dc7af5392e60b6b98ebaea7ead11d47218a8df03 Mon Sep 17 00:00:00 2001 From: Ulf Adams Date: Fri, 25 Sep 2015 15:14:27 +0000 Subject: Split the PackageManager type hierarchy; no longer inherit LoadedPackageProv. This limits the exposure of LoadedPackageProvider, such that there will be no regressions in the use of getLoadedTarget. Unfortunately, fully removing LoadedPackageProvider is more work than I'm willing to take on right now, and this is the cleanest intermediate solution I could come up with. This unblocks my other work (removing SkyframeExecutor.errorEventHandler). Someone else will have to shave this yak. -- MOS_MIGRATED_REVID=103943375 --- .../devtools/build/lib/analysis/BuildView.java | 2 +- .../analysis/config/ConfigurationEnvironment.java | 24 ++--------- .../devtools/build/lib/buildtool/BuildTool.java | 5 ++- .../build/lib/pkgcache/LoadedPackageProvider.java | 46 ++++++++++++++++++++++ .../build/lib/pkgcache/LoadingPhaseRunner.java | 13 +++--- .../build/lib/pkgcache/PackageManager.java | 3 +- .../lib/skyframe/AbstractLabelCycleReporter.java | 5 ++- .../lib/skyframe/ActionArtifactCycleReporter.java | 3 +- .../skyframe/ConfiguredTargetCycleReporter.java | 3 +- .../build/lib/skyframe/SkyframeExecutor.java | 35 ++++++---------- .../build/lib/skyframe/SkyframeLabelVisitor.java | 4 +- .../build/lib/skyframe/SkyframePackageManager.java | 9 ----- .../skyframe/TransitiveTargetCycleReporter.java | 3 +- 13 files changed, 85 insertions(+), 70 deletions(-) (limited to 'src/main') diff --git a/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java b/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java index 7fd00a0b35..d6c3d413a0 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java @@ -261,7 +261,7 @@ public class BuildView { SkyframeExecutor skyframeExecutor, BinTools binTools, CoverageReportActionFactory coverageReportActionFactory) { this.directories = directories; - this.packageManager = skyframeExecutor.getPackageManager(); + this.packageManager = skyframeExecutor.getLoadedPackageProvider(); this.binTools = binTools; this.coverageReportActionFactory = coverageReportActionFactory; this.ruleClassProvider = ruleClassProvider; diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/ConfigurationEnvironment.java b/src/main/java/com/google/devtools/build/lib/analysis/config/ConfigurationEnvironment.java index 44ef2d89ac..441e84a1d2 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/config/ConfigurationEnvironment.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/config/ConfigurationEnvironment.java @@ -17,18 +17,16 @@ package com.google.devtools.build.lib.analysis.config; import com.google.devtools.build.lib.analysis.BlazeDirectories; import com.google.devtools.build.lib.analysis.config.BuildConfiguration.Fragment; import com.google.devtools.build.lib.cmdline.Label; -import com.google.devtools.build.lib.concurrent.Uninterruptibles; import com.google.devtools.build.lib.events.EventHandler; import com.google.devtools.build.lib.packages.NoSuchPackageException; import com.google.devtools.build.lib.packages.NoSuchTargetException; import com.google.devtools.build.lib.packages.Package; import com.google.devtools.build.lib.packages.Target; +import com.google.devtools.build.lib.pkgcache.LoadedPackageProvider; import com.google.devtools.build.lib.pkgcache.PackageProvider; import com.google.devtools.build.lib.pkgcache.TargetProvider; import com.google.devtools.build.lib.vfs.Path; -import java.util.concurrent.Callable; - import javax.annotation.Nullable; /** @@ -61,14 +59,12 @@ public interface ConfigurationEnvironment { * An implementation backed by a {@link PackageProvider} instance. */ public static final class TargetProviderEnvironment implements ConfigurationEnvironment { - private final PackageProvider packageProvider; - private final EventHandler eventHandler; + private final LoadedPackageProvider.Bridge packageProvider; private final BlazeDirectories blazeDirectories; public TargetProviderEnvironment(PackageProvider packageProvider, EventHandler eventHandler, BlazeDirectories blazeDirectories) { - this.packageProvider = packageProvider; - this.eventHandler = eventHandler; + this.packageProvider = new LoadedPackageProvider.Bridge(packageProvider, eventHandler); this.blazeDirectories = blazeDirectories; } @@ -80,19 +76,7 @@ public interface ConfigurationEnvironment { @Override public Target getTarget(final Label label) throws NoSuchPackageException, NoSuchTargetException { - try { - return Uninterruptibles.callUninterruptibly(new Callable() { - @Override - public Target call() - throws NoSuchPackageException, NoSuchTargetException, InterruptedException { - return packageProvider.getTarget(eventHandler, label); - } - }); - } catch (NoSuchPackageException | NoSuchTargetException e) { - throw e; - } catch (Exception e) { - throw new IllegalStateException(e); - } + return packageProvider.getLoadedTarget(label); } @Override diff --git a/src/main/java/com/google/devtools/build/lib/buildtool/BuildTool.java b/src/main/java/com/google/devtools/build/lib/buildtool/BuildTool.java index ebb5efecaa..b7a74c695e 100644 --- a/src/main/java/com/google/devtools/build/lib/buildtool/BuildTool.java +++ b/src/main/java/com/google/devtools/build/lib/buildtool/BuildTool.java @@ -197,8 +197,9 @@ public final class BuildTool { result.setActualTargets(analysisResult.getTargetsToBuild()); result.setTestTargets(analysisResult.getTargetsToTest()); - checkTargetEnvironmentRestrictions(analysisResult.getTargetsToBuild(), - runtime.getPackageManager()); + LoadedPackageProvider.Bridge bridge = + new LoadedPackageProvider.Bridge(env.getPackageManager(), env.getReporter()); + checkTargetEnvironmentRestrictions(analysisResult.getTargetsToBuild(), bridge); reportTargets(analysisResult); // Execution phase. diff --git a/src/main/java/com/google/devtools/build/lib/pkgcache/LoadedPackageProvider.java b/src/main/java/com/google/devtools/build/lib/pkgcache/LoadedPackageProvider.java index 552a3cc886..9194466efa 100644 --- a/src/main/java/com/google/devtools/build/lib/pkgcache/LoadedPackageProvider.java +++ b/src/main/java/com/google/devtools/build/lib/pkgcache/LoadedPackageProvider.java @@ -14,10 +14,14 @@ package com.google.devtools.build.lib.pkgcache; import com.google.devtools.build.lib.cmdline.Label; +import com.google.devtools.build.lib.concurrent.Uninterruptibles; +import com.google.devtools.build.lib.events.EventHandler; import com.google.devtools.build.lib.packages.NoSuchPackageException; import com.google.devtools.build.lib.packages.NoSuchTargetException; import com.google.devtools.build.lib.packages.Target; +import java.util.concurrent.Callable; + /** * Read-only API for retrieving packages, i.e., calling this API should not result in packages being * loaded. @@ -34,4 +38,46 @@ public interface LoadedPackageProvider { * target. */ Target getLoadedTarget(Label label) throws NoSuchPackageException, NoSuchTargetException; + + /** + * A bridge class that implements the legacy semantics of {@link #getLoadedTarget} using a + * normal {@link PackageProvider} instance. + */ + public static final class Bridge implements LoadedPackageProvider { + private final PackageProvider packageProvider; + private final EventHandler eventHandler; + + public Bridge(PackageProvider packageProvider, EventHandler eventHandler) { + this.packageProvider = packageProvider; + this.eventHandler = eventHandler; + } + + @Override + public Target getLoadedTarget(final Label label) + throws NoSuchPackageException, NoSuchTargetException { + return getLoadedTarget(packageProvider, eventHandler, label); + } + + /** + * Uninterruptible method to convert a label into a target using a given package provider and + * event handler. + */ + public static Target getLoadedTarget( + final PackageProvider packageProvider, final EventHandler eventHandler, final Label label) + throws NoSuchPackageException, NoSuchTargetException { + try { + return Uninterruptibles.callUninterruptibly(new Callable() { + @Override + public Target call() + throws NoSuchPackageException, NoSuchTargetException, InterruptedException { + return packageProvider.getTarget(eventHandler, label); + } + }); + } catch (NoSuchPackageException | NoSuchTargetException e) { + throw e; + } catch (Exception e) { + throw new IllegalStateException(e); + } + } + } } diff --git a/src/main/java/com/google/devtools/build/lib/pkgcache/LoadingPhaseRunner.java b/src/main/java/com/google/devtools/build/lib/pkgcache/LoadingPhaseRunner.java index ac6d86b200..b05a52336c 100644 --- a/src/main/java/com/google/devtools/build/lib/pkgcache/LoadingPhaseRunner.java +++ b/src/main/java/com/google/devtools/build/lib/pkgcache/LoadingPhaseRunner.java @@ -485,7 +485,7 @@ public class LoadingPhaseRunner { targetsToAnalyze = targetsToLoad; } else if (keepGoing) { // Keep going: filter out the error-free targets and only continue with those. - targetsToAnalyze = filterErrorFreeTargets(eventBus, targetsToLoad, + targetsToAnalyze = filterErrorFreeTargets(eventHandler, eventBus, targetsToLoad, labelsToLoadUnconditionally); reportAboutPartiallySuccesfulLoading(targetsToLoad, targetsToAnalyze, eventHandler); } else { @@ -537,11 +537,12 @@ public class LoadingPhaseRunner { } } - private Set getTargetsForLabels(Collection