Skip to content

Commit

Permalink
Target pattern parsing cleanup
Browse files Browse the repository at this point in the history
================

* `TargetPattern#renameRepository` is merged into `TargetPattern.Parser#parse`. This is much less error-prone -- repo mapping should not be an afterthought, but something that should always be applied if possible (sometimes it's not, for example for `blaze query`).
  * This also fixes the bug where calling `native.register_toolchains("//:all")` in `@some_repo//:defs.bzl` registers `@//:all` instead of `@some_repo//:all` (see change in RegisteredExecutionPlatformsTest)
* A new class `SignedTargetPattern` is introduced, which can store whether the pattern is positive or negative (with the `sign` method).
* `TargetPatternValue#keys` is greatly simplified thanks to the changes above; the exception throwing is confined to the parsing step, and the construction of `TargetPatternKey` can happen as a separate step, obviating the whole "skykey or exception" dance.
* Following from the above, the //external package now stores registered toolchains and execution platforms as parsed target patterns, instead of simple strings.

Among other things, this would help implement toolchain registration in bzlmod.

PiperOrigin-RevId: 400720457
  • Loading branch information
Wyverald authored and copybara-github committed Oct 4, 2021
1 parent d250a23 commit 630be02
Show file tree
Hide file tree
Showing 30 changed files with 444 additions and 434 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@
// limitations under the License.
package com.google.devtools.build.lib.bazel.commands;

import static com.google.common.collect.ImmutableList.toImmutableList;

import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
Expand All @@ -24,6 +26,7 @@
import com.google.devtools.build.lib.cmdline.LabelConstants;
import com.google.devtools.build.lib.cmdline.LabelSyntaxException;
import com.google.devtools.build.lib.cmdline.RepositoryName;
import com.google.devtools.build.lib.cmdline.TargetPattern;
import com.google.devtools.build.lib.events.Event;
import com.google.devtools.build.lib.events.ExtendedEventHandler.ResolvedEvent;
import com.google.devtools.build.lib.packages.Rule;
Expand Down Expand Up @@ -166,12 +169,17 @@ public BlazeCommandResult exec(CommandEnvironment env, OptionsParsingResult opti
env.getReporter()
.post(
genericArgsCall(
"register_toolchains", fileValue.getPackage().getRegisteredToolchains()));
"register_toolchains",
fileValue.getPackage().getRegisteredToolchains().stream()
.map(TargetPattern::getOriginalPattern)
.collect(toImmutableList())));
env.getReporter()
.post(
genericArgsCall(
"register_execution_platforms",
fileValue.getPackage().getRegisteredExecutionPlatforms()));
fileValue.getPackage().getRegisteredExecutionPlatforms().stream()
.map(TargetPattern::getOriginalPattern)
.collect(toImmutableList())));
env.getReporter().post(new RepositoryOrderEvent(repositoryOrder.build()));

// take all Starlark workspace rules and get their values
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,7 @@ private static ImmutableSet<String> getExplicitTargetPatterns(
CommandEnvironment env, List<String> requestedTargetPatterns)
throws ViewCreationFailedException {
ImmutableSet.Builder<String> explicitTargetPatterns = ImmutableSet.builder();
TargetPattern.Parser parser = new TargetPattern.Parser(env.getRelativeWorkingDirectory());
TargetPattern.Parser parser = TargetPattern.mainRepoParser(env.getRelativeWorkingDirectory());

for (String requestedTargetPattern : requestedTargetPatterns) {
if (requestedTargetPattern.startsWith("-")) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -315,7 +315,7 @@ private static ImmutableSet<String> getExplicitTargetPatterns(
CommandEnvironment env, List<String> requestedTargetPatterns)
throws ViewCreationFailedException {
ImmutableSet.Builder<String> explicitTargetPatterns = ImmutableSet.builder();
TargetPattern.Parser parser = new TargetPattern.Parser(env.getRelativeWorkingDirectory());
TargetPattern.Parser parser = TargetPattern.mainRepoParser(env.getRelativeWorkingDirectory());

for (String requestedTargetPattern : requestedTargetPatterns) {
if (requestedTargetPattern.startsWith("-")) {
Expand Down
2 changes: 2 additions & 0 deletions src/main/java/com/google/devtools/build/lib/cmdline/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ java_library(
"LabelConstants.java",
"LabelSerializationProxy.java",
"ResolvedTargets.java",
"SignedTargetPattern.java",
"TargetParsingException.java",
"TargetPattern.java",
"TargetPatternResolver.java",
Expand All @@ -38,6 +39,7 @@ java_library(
"//src/main/java/net/starlark/java/annot",
"//src/main/java/net/starlark/java/eval",
"//src/main/protobuf:failure_details_java_proto",
"//third_party:auto_value",
"//third_party:error_prone_annotations",
"//third_party:guava",
"//third_party:jsr305",
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
// Copyright 2021 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.cmdline;

import com.google.auto.value.AutoValue;

/** A {@link TargetPattern} with a potential minus sign in front of it, signifying exclusion. */
@AutoValue
public abstract class SignedTargetPattern {
public abstract TargetPattern pattern();

public abstract Sign sign();

public static SignedTargetPattern create(TargetPattern pattern, Sign sign) {
return new AutoValue_SignedTargetPattern(pattern, sign);
}

/** Whether this target pattern begins with a minus sign (NEGATIVE) or not (POSITIVE). */
public enum Sign {
POSITIVE,
NEGATIVE
}

public static SignedTargetPattern parse(String pattern, TargetPattern.Parser parser)
throws TargetParsingException {
if (pattern.startsWith("-")) {
return create(parser.parse(pattern.substring(1)), SignedTargetPattern.Sign.NEGATIVE);
} else {
return create(parser.parse(pattern), SignedTargetPattern.Sign.POSITIVE);
}
}
}
Loading

0 comments on commit 630be02

Please sign in to comment.