From 24a003bddfa45f198d5f9257418778eda587f217 Mon Sep 17 00:00:00 2001 From: nharmata Date: Thu, 25 Apr 2019 10:39:24 -0700 Subject: [PATCH] In the SingleTarget case of TargetPattern#parse, store the parsed Label. Then, use this Label during TargetPattern.SingleTarget#eval. This is a slight simplification that lets us delete some code. RELNOTES: None PiperOrigin-RevId: 245263195 --- .../build/lib/cmdline/TargetPattern.java | 40 ++++++++----------- .../lib/skyframe/TargetPatternValue.java | 10 +---- 2 files changed, 17 insertions(+), 33 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/cmdline/TargetPattern.java b/src/main/java/com/google/devtools/build/lib/cmdline/TargetPattern.java index 86476d33c5c59a..17afd706175e82 100644 --- a/src/main/java/com/google/devtools/build/lib/cmdline/TargetPattern.java +++ b/src/main/java/com/google/devtools/build/lib/cmdline/TargetPattern.java @@ -24,8 +24,6 @@ import com.google.common.util.concurrent.Futures; import com.google.common.util.concurrent.ListenableFuture; import com.google.common.util.concurrent.ListeningExecutorService; -import com.google.devtools.build.lib.cmdline.LabelValidator.BadLabelException; -import com.google.devtools.build.lib.cmdline.LabelValidator.PackageAndTarget; import com.google.devtools.build.lib.util.BatchCallback; import com.google.devtools.build.lib.util.StringUtilities; import com.google.devtools.build.lib.util.ThreadSafeBatchCallback; @@ -283,8 +281,8 @@ public String getPathForPathAsTarget() { throw new IllegalStateException(); } - /** For patterns of type {@link Type#SINGLE_TARGET}, returns the target path. */ - public String getSingleTargetPath() { + /** For patterns of type {@link Type#SINGLE_TARGET}, returns the {@link Label} of the target. */ + public Label getSingleTargetLabel() { throw new IllegalStateException(); } @@ -306,14 +304,11 @@ public PackageIdentifier getDirectoryForTargetOrTargetsInPackage() { private static final class SingleTarget extends TargetPattern { - private final String targetName; - private final PackageIdentifier directory; + private final Label label; - private SingleTarget( - String targetName, PackageIdentifier directory, String originalPattern, String offset) { + private SingleTarget(Label label, String originalPattern, String offset) { super(Type.SINGLE_TARGET, originalPattern, offset); - this.targetName = Preconditions.checkNotNull(targetName); - this.directory = Preconditions.checkNotNull(directory); + this.label = Preconditions.checkNotNull(label); } @Override @@ -323,7 +318,7 @@ public void eval( ImmutableSet excludedSubdirectories, BatchCallback callback, Class exceptionClass) throws TargetParsingException, E, InterruptedException { - callback.process(resolver.getExplicitTarget(label(targetName)).getTargets()); + callback.process(resolver.getExplicitTarget(label).getTargets()); } @Override @@ -333,7 +328,7 @@ public boolean containsAllTransitiveSubdirectoriesForTBD(PackageIdentifier direc @Override public PackageIdentifier getDirectoryForTargetOrTargetsInPackage() { - return directory; + return label.getPackageIdentifier(); } @Override @@ -342,8 +337,8 @@ public boolean getRulesOnly() { } @Override - public String getSingleTargetPath() { - return targetName; + public Label getSingleTargetLabel() { + return label; } @Override @@ -355,12 +350,12 @@ public boolean equals(Object o) { return false; } SingleTarget that = (SingleTarget) o; - return targetName.equals(that.targetName) && directory.equals(that.directory); + return label.equals(that.label); } @Override public int hashCode() { - return Objects.hash(getType(), targetName, directory); + return Objects.hash(getType(), label); } } @@ -818,18 +813,15 @@ public TargetPattern parse(String pattern) throws TargetParsingException { } if (includesRepo || wasOriginallyAbsolute || pattern.contains(":")) { - PackageIdentifier packageIdentifier; - String fullLabel = repository.getName() + "//" + pattern; + String fullLabelString = repository.getName() + "//" + pattern; + Label label; try { - PackageAndTarget packageAndTarget = LabelValidator.validateAbsoluteLabel(fullLabel); - packageIdentifier = PackageIdentifier.create(repository, - PathFragment.create(packageAndTarget.getPackageName())); - } catch (BadLabelException e) { + label = Label.parseAbsolute(fullLabelString, ImmutableMap.of()); + } catch (LabelSyntaxException e) { String error = "invalid target format '" + originalPattern + "': " + e.getMessage(); throw new TargetParsingException(error); } - return new SingleTarget( - fullLabel, packageIdentifier, originalPattern, relativeDirectory); + return new SingleTarget(label, originalPattern, relativeDirectory); } // This is a stripped-down version of interpretPathAsTarget that does no I/O. We have a basic diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/TargetPatternValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/TargetPatternValue.java index 1aea6118f6cf3b..1daa9674b00c02 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/TargetPatternValue.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/TargetPatternValue.java @@ -247,15 +247,7 @@ private static TargetPatternKeyWithExclusionsResult computeTargetPatternKeyWithE } } } else if (excludeSingleTargets && laterParsedPattern.getType() == Type.SINGLE_TARGET) { - try { - Label label = - Label.parseAbsolute( - laterParsedPattern.getSingleTargetPath(), - /*repositoryMapping=*/ ImmutableMap.of()); - excludedSingleTargetsBuilder.add(label); - } catch (LabelSyntaxException e) { - indicesOfNegativePatternsThatNeedToBeIncludedBuilder.add(j); - } + excludedSingleTargetsBuilder.add(laterParsedPattern.getSingleTargetLabel()); } else { indicesOfNegativePatternsThatNeedToBeIncludedBuilder.add(j); }