diff options
author | Ulf Adams <ulfjack@google.com> | 2016-02-10 11:39:31 +0000 |
---|---|---|
committer | Dmitry Lomov <dslomov@google.com> | 2016-02-10 16:34:40 +0000 |
commit | 4224fc020c94fb363cad0c0b5dfcc225cd8e2c1a (patch) | |
tree | d40727fb5c5404e0c5a432dc66822b08389e032e | |
parent | caf1477087748b636b7be2bb112ea35f6f2140f4 (diff) |
SkyframeLoadingPhaseRunner: implement --compile_one_dependency.
Also fix a bug in the LoadingPhaseRunner - we weren't printing an error for
failed targets, duh!
--
MOS_MIGRATED_REVID=114310591
3 files changed, 69 insertions, 10 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/pkgcache/CompileOneDependencyTransformer.java b/src/main/java/com/google/devtools/build/lib/pkgcache/CompileOneDependencyTransformer.java index 85766dac93..7caddf6329 100644 --- a/src/main/java/com/google/devtools/build/lib/pkgcache/CompileOneDependencyTransformer.java +++ b/src/main/java/com/google/devtools/build/lib/pkgcache/CompileOneDependencyTransformer.java @@ -17,6 +17,7 @@ import com.google.common.collect.Lists; import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.cmdline.ResolvedTargets; import com.google.devtools.build.lib.cmdline.TargetParsingException; +import com.google.devtools.build.lib.events.Event; import com.google.devtools.build.lib.events.EventHandler; import com.google.devtools.build.lib.packages.Attribute; import com.google.devtools.build.lib.packages.BuildType; @@ -37,12 +38,11 @@ import java.util.List; /** * Implementation of --compile_one_dependency. */ -final class CompileOneDependencyTransformer { +public final class CompileOneDependencyTransformer { + private final TargetProvider targetProvider; - private final PackageManager pkgManager; - - public CompileOneDependencyTransformer(PackageManager pkgManager) { - this.pkgManager = pkgManager; + public CompileOneDependencyTransformer(TargetProvider targetProvider) { + this.targetProvider = targetProvider; } /** @@ -100,7 +100,7 @@ final class CompileOneDependencyTransformer { try { // The call to getSrcTargets here can be removed in favor of the // rule.getLabels() call below once we update "srcs" for all rules. - if (SrcTargetUtil.getSrcTargets(eventHandler, rule, pkgManager).contains(target)) { + if (SrcTargetUtil.getSrcTargets(eventHandler, rule, targetProvider).contains(target)) { if (rule.getRuleClassObject().isPreferredDependency(target.getName())) { return rule; } else if (fallbackRule == null) { @@ -148,10 +148,11 @@ final class CompileOneDependencyTransformer { try { // If the rule has source targets, return it. - if (!SrcTargetUtil.getSrcTargets(eventHandler, result, pkgManager).isEmpty()) { + if (!SrcTargetUtil.getSrcTargets(eventHandler, result, targetProvider).isEmpty()) { return result; } } catch (NoSuchThingException e) { + eventHandler.handle(Event.error(e.getMessage())); throw new TargetParsingException( "Couldn't find dependency on target '" + target.getLabel() + "'"); } catch (InterruptedException e) { diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/TargetPatternPhaseFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/TargetPatternPhaseFunction.java index 0f6476f577..ea30c0584c 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/TargetPatternPhaseFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/TargetPatternPhaseFunction.java @@ -22,10 +22,13 @@ import com.google.devtools.build.lib.cmdline.TargetParsingException; import com.google.devtools.build.lib.events.Event; import com.google.devtools.build.lib.packages.Target; import com.google.devtools.build.lib.packages.TargetUtils; +import com.google.devtools.build.lib.pkgcache.CompileOneDependencyTransformer; import com.google.devtools.build.lib.pkgcache.FilteringPolicies; import com.google.devtools.build.lib.pkgcache.LoadingOptions; import com.google.devtools.build.lib.pkgcache.LoadingPhaseRunner; +import com.google.devtools.build.lib.pkgcache.TargetProvider; import com.google.devtools.build.lib.pkgcache.TestFilter; +import com.google.devtools.build.lib.skyframe.EnvironmentBackedRecursivePackageProvider.MissingDepException; import com.google.devtools.build.lib.skyframe.TargetPatternPhaseValue.TargetPatternList; import com.google.devtools.build.lib.skyframe.TargetPatternValue.TargetPatternKey; import com.google.devtools.build.lib.skyframe.TargetPatternValue.TargetPatternSkyKeyOrException; @@ -209,11 +212,20 @@ final class TargetPatternPhaseFunction implements SkyFunction { } } + ResolvedTargets<Target> result = builder.build(); if (compileOneDependency) { - // TODO(ulfjack): Add support for compile_one_dependency before hooking this up. - throw new UnsupportedOperationException(); + TargetProvider targetProvider = new EnvironmentBackedRecursivePackageProvider(env); + try { + return new CompileOneDependencyTransformer(targetProvider) + .transformCompileOneDependency(env.getListener(), result); + } catch (MissingDepException e) { + return null; + } catch (TargetParsingException e) { + env.getListener().handle(Event.error(e.getMessage())); + return ResolvedTargets.failed(); + } } - return builder.build(); + return result; } /** diff --git a/src/test/java/com/google/devtools/build/lib/pkgcache/LoadingPhaseRunnerTest.java b/src/test/java/com/google/devtools/build/lib/pkgcache/LoadingPhaseRunnerTest.java index e56117e454..1e0b234d89 100644 --- a/src/test/java/com/google/devtools/build/lib/pkgcache/LoadingPhaseRunnerTest.java +++ b/src/test/java/com/google/devtools/build/lib/pkgcache/LoadingPhaseRunnerTest.java @@ -550,6 +550,52 @@ public class LoadingPhaseRunnerTest { tester.assertContainsEventWithFrequency("invalid value in 'bash_version' attribute", 1); } + @Test + public void testCompileOneDependency() throws Exception { + tester.addFile("base/BUILD", + "cc_library(name = 'hello', srcs = ['hello.cc'])"); + tester.useLoadingOptions("--compile_one_dependency"); + LoadingResult loadingResult = assertNoErrors(tester.load("base/hello.cc")); + assertThat(loadingResult.getTargets()).containsExactlyElementsIn(getTargets("//base:hello")); + } + + @Test + public void testCompileOneDependencyNonExistentSource() throws Exception { + tester.addFile("base/BUILD", + "cc_library(name = 'hello', srcs = ['hello.cc', '//bad:bad.cc'])"); + tester.useLoadingOptions("--compile_one_dependency"); + try { + tester.load("base/hello.cc"); + fail(); + } catch (TargetParsingException expected) { + tester.assertContainsError("no such package 'bad'"); + } + } + + @Test + public void testCompileOneDependencyNonExistentSourceKeepGoing() throws Exception { + tester.addFile("base/BUILD", + "cc_library(name = 'hello', srcs = ['hello.cc', '//bad:bad.cc'])"); + tester.useLoadingOptions("--compile_one_dependency"); + if (runsLoadingPhase()) { + // The LegacyLoadingPhaseRunner throws an exception if it can't load any of the sources in the + // same rule as the source we're looking for even with --keep_going. + // In general, we probably want --compile_one_dependency to be compatible with --keep_going + // for consistency, but it's unclear if this is actually a problem for anyone. The most common + // use case for compile_one_dependency is to iterate quickly on a single file, without + // --keep_going. + try { + tester.load("base/hello.cc"); + fail(); + } catch (TargetParsingException expected) { + tester.assertContainsError("no such package 'bad'"); + } + } else { + LoadingResult loadingResult = tester.loadKeepGoing("base/hello.cc"); + assertThat(loadingResult.hasTargetPatternError()).isTrue(); + } + } + private void assertCircularSymlinksDuringTargetParsing(String targetPattern) throws Exception { try { tester.load(targetPattern); |