Skip to content

Commit

Permalink
Automated rollback of commit 771cb7a.
Browse files Browse the repository at this point in the history
*** Reason for rollback ***

Breaks bazel-skylib

See #8227 for details.

*** Original change description ***

Make target pattern parsing repository-renaming aware.

Platform and toolchain resolution rely on the target pattern parsing code to turn target pattern strings into Labels. Since most of the target pattern parsing codepaths turn target patterns that originated from the command line, they don't need to pass along the repository renaming map. But instances that affect platform and toolchain target patterns, we need to pass the map.

This allows us to turn on the --incompatible_remap_main_repo flag on by default in Bazel.

Closes #7902.
Fixes #7755, #7773, #7654.

RELNOTES: None
PiperOrigin-RevId: 246546091
  • Loading branch information
c-parsons authored and copybara-github committed May 3, 2019
1 parent f6b091b commit dac2135
Show file tree
Hide file tree
Showing 24 changed files with 155 additions and 409 deletions.
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright 2019 The Bazel Authors. All rights reserved.
// Copyright 2017 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.
Expand All @@ -17,15 +17,13 @@
import static java.util.stream.Collectors.joining;

import com.google.auto.value.AutoValue;
import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.devtools.build.lib.analysis.platform.PlatformInfo;
import com.google.devtools.build.lib.analysis.platform.PlatformProviderUtils;
import com.google.devtools.build.lib.analysis.platform.ToolchainInfo;
import com.google.devtools.build.lib.analysis.platform.ToolchainTypeInfo;
import com.google.devtools.build.lib.analysis.skylark.BazelStarlarkContext;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.cmdline.LabelSyntaxException;
import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
Expand Down Expand Up @@ -187,8 +185,7 @@ public void repr(SkylarkPrinter printer) {
printer.append(">");
}

private Label transformKey(Object key, Location loc, BazelStarlarkContext context)
throws EvalException {
private Label transformKey(Object key, Location loc) throws EvalException {
if (key instanceof Label) {
return (Label) key;
} else if (key instanceof ToolchainTypeInfo) {
Expand All @@ -197,7 +194,7 @@ private Label transformKey(Object key, Location loc, BazelStarlarkContext contex
Label toolchainType;
String rawLabel = (String) key;
try {
toolchainType = Label.parseAbsolute(rawLabel, context.getRepoMapping());
toolchainType = Label.parseAbsolute(rawLabel, ImmutableMap.of());
} catch (LabelSyntaxException e) {
throw new EvalException(
loc, String.format("Unable to parse toolchain %s: %s", rawLabel, e.getMessage()), e);
Expand All @@ -215,8 +212,7 @@ private Label transformKey(Object key, Location loc, BazelStarlarkContext contex
@Override
public ToolchainInfo getIndex(Object key, Location loc, StarlarkContext context)
throws EvalException {
Preconditions.checkArgument(context instanceof BazelStarlarkContext);
Label toolchainTypeLabel = transformKey(key, loc, (BazelStarlarkContext) context);
Label toolchainTypeLabel = transformKey(key, loc);

if (!containsKey(key, loc, context)) {
throw new EvalException(
Expand All @@ -236,8 +232,7 @@ public ToolchainInfo getIndex(Object key, Location loc, StarlarkContext context)
@Override
public boolean containsKey(Object key, Location loc, StarlarkContext context)
throws EvalException {
Preconditions.checkArgument(context instanceof BazelStarlarkContext);
Label toolchainTypeLabel = transformKey(key, loc, (BazelStarlarkContext) context);
Label toolchainTypeLabel = transformKey(key, loc);
Optional<Label> matching =
toolchains().keySet().stream()
.map(ToolchainTypeInfo::typeLabel)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.cmdline.LabelSyntaxException;
import com.google.devtools.build.lib.cmdline.LabelValidator;
import com.google.devtools.build.lib.cmdline.RepositoryName;
import com.google.devtools.build.lib.events.Location;
import com.google.devtools.build.lib.packages.Attribute;
import com.google.devtools.build.lib.packages.AttributeMap;
Expand Down Expand Up @@ -290,7 +289,6 @@ public BaseFunction rule(
throws EvalException {
SkylarkUtils.checkLoadingOrWorkspacePhase(funcallEnv, "rule", ast.getLocation());

Preconditions.checkArgument(context instanceof BazelStarlarkContext);
BazelStarlarkContext bazelContext = (BazelStarlarkContext) context;
// analysis_test=true implies test=true.
test |= Boolean.TRUE.equals(analysisTest);
Expand Down Expand Up @@ -357,9 +355,7 @@ public BaseFunction rule(
funcallEnv.getGlobals().getLabel(), funcallEnv.getTransitiveContentHashCode());
builder.addRequiredToolchains(
collectToolchainLabels(
toolchains.getContents(String.class, "toolchains"),
bazelContext.getRepoMapping(),
ast.getLocation()));
toolchains.getContents(String.class, "toolchains"), ast.getLocation()));

if (!buildSetting.equals(Runtime.NONE) && !cfg.equals(Runtime.NONE)) {
throw new EvalException(
Expand Down Expand Up @@ -400,7 +396,6 @@ public BaseFunction rule(
builder.addExecutionPlatformConstraints(
collectConstraintLabels(
execCompatibleWith.getContents(String.class, "exec_compatile_with"),
bazelContext.getRepoMapping(),
ast.getLocation()));
}

Expand Down Expand Up @@ -439,14 +434,11 @@ private static void addAttribute(
}

private static ImmutableList<Label> collectToolchainLabels(
Iterable<String> rawLabels,
ImmutableMap<RepositoryName, RepositoryName> repoMapping,
Location loc)
throws EvalException {
Iterable<String> rawLabels, Location loc) throws EvalException {
ImmutableList.Builder<Label> requiredToolchains = new ImmutableList.Builder<>();
for (String rawLabel : rawLabels) {
try {
Label toolchainLabel = Label.parseAbsolute(rawLabel, repoMapping);
Label toolchainLabel = Label.parseAbsolute(rawLabel, ImmutableMap.of());
requiredToolchains.add(toolchainLabel);
} catch (LabelSyntaxException e) {
throw new EvalException(
Expand All @@ -458,14 +450,11 @@ private static ImmutableList<Label> collectToolchainLabels(
}

private static ImmutableList<Label> collectConstraintLabels(
Iterable<String> rawLabels,
ImmutableMap<RepositoryName, RepositoryName> repoMapping,
Location loc)
throws EvalException {
Iterable<String> rawLabels, Location loc) throws EvalException {
ImmutableList.Builder<Label> constraintLabels = new ImmutableList.Builder<>();
for (String rawLabel : rawLabels) {
try {
Label constraintLabel = Label.parseAbsolute(rawLabel, repoMapping);
Label constraintLabel = Label.parseAbsolute(rawLabel, ImmutableMap.of());
constraintLabels.add(constraintLabel);
} catch (LabelSyntaxException e) {
throw new EvalException(
Expand All @@ -488,10 +477,8 @@ public SkylarkAspect aspect(
SkylarkList<?> toolchains,
String doc,
FuncallExpression ast,
Environment funcallEnv,
StarlarkContext context)
Environment funcallEnv)
throws EvalException {
Preconditions.checkArgument(context instanceof BazelStarlarkContext);
Location location = ast.getLocation();
ImmutableList.Builder<String> attrAspects = ImmutableList.builder();
for (Object attributeAspect : attributeAspects) {
Expand Down Expand Up @@ -579,9 +566,7 @@ public SkylarkAspect aspect(
HostTransition.INSTANCE,
ImmutableSet.copyOf(hostFragments.getContents(String.class, "host_fragments")),
collectToolchainLabels(
toolchains.getContents(String.class, "toolchains"),
((BazelStarlarkContext) context).getRepoMapping(),
ast.getLocation()));
toolchains.getContents(String.class, "toolchains"), ast.getLocation()));
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -144,15 +144,14 @@ public String getOffset() {
}

/**
* Evaluates the current target pattern, excluding targets under directories in both {@code
* blacklistedSubdirectories} and {@code excludedSubdirectories}, and returns the result.
* Evaluates the current target pattern, excluding targets under directories in both
* {@code blacklistedSubdirectories} and {@code excludedSubdirectories}, and returns the result.
*
* @throws IllegalArgumentException if either {@code blacklistedSubdirectories} or {@code
* excludedSubdirectories} is nonempty and this pattern does not have type {@code
* Type.TARGETS_BELOW_DIRECTORY}.
* @throws IllegalArgumentException if either {@code blacklistedSubdirectories} or
* {@code excludedSubdirectories} is nonempty and this pattern does not have type
* {@code Type.TARGETS_BELOW_DIRECTORY}.
*/
public abstract <T, E extends Exception> void eval(
ImmutableMap<RepositoryName, RepositoryName> repositoryMapping,
TargetPatternResolver<T> resolver,
ImmutableSet<PathFragment> blacklistedSubdirectories,
ImmutableSet<PathFragment> excludedSubdirectories,
Expand All @@ -161,28 +160,21 @@ public abstract <T, E extends Exception> void eval(
throws TargetParsingException, E, InterruptedException;

/**
* Evaluates this {@link TargetPattern} synchronously, feeding the result to the given {@code
* callback}, and then returns an appropriate immediate {@link ListenableFuture}.
* Evaluates this {@link TargetPattern} synchronously, feeding the result to the given
* {@code callback}, and then returns an appropriate immediate {@link ListenableFuture}.
*
* <p>If the returned {@link ListenableFuture}'s {@link ListenableFuture#get} throws an {@link
* ExecutionException}, the cause will be an instance of either {@link TargetParsingException} or
* the given {@code exceptionClass}.
* <p>If the returned {@link ListenableFuture}'s {@link ListenableFuture#get} throws an
* {@link ExecutionException}, the cause will be an instance of either
* {@link TargetParsingException} or the given {@code exceptionClass}.
*/
public final <T, E extends Exception> ListenableFuture<Void> evalAdaptedForAsync(
ImmutableMap<RepositoryName, RepositoryName> repositoryMapping,
TargetPatternResolver<T> resolver,
ImmutableSet<PathFragment> blacklistedSubdirectories,
ImmutableSet<PathFragment> excludedSubdirectories,
ThreadSafeBatchCallback<T, E> callback,
Class<E> exceptionClass) {
try {
eval(
repositoryMapping,
resolver,
blacklistedSubdirectories,
excludedSubdirectories,
callback,
exceptionClass);
eval(resolver, blacklistedSubdirectories, excludedSubdirectories, callback, exceptionClass);
return Futures.immediateFuture(null);
} catch (TargetParsingException e) {
return Futures.immediateFailedFuture(e);
Expand All @@ -197,28 +189,22 @@ public final <T, E extends Exception> ListenableFuture<Void> evalAdaptedForAsync
}

/**
* Returns a {@link ListenableFuture} representing the asynchronous evaluation of this {@link
* TargetPattern} that feeds the results to the given {@code callback}.
* Returns a {@link ListenableFuture} representing the asynchronous evaluation of this
* {@link TargetPattern} that feeds the results to the given {@code callback}.
*
* <p>If the returned {@link ListenableFuture}'s {@link ListenableFuture#get} throws an {@link
* ExecutionException}, the cause will be an instance of either {@link TargetParsingException} or
* the given {@code exceptionClass}.
* <p>If the returned {@link ListenableFuture}'s {@link ListenableFuture#get} throws an
* {@link ExecutionException}, the cause will be an instance of either
* {@link TargetParsingException} or the given {@code exceptionClass}.
*/
public <T, E extends Exception> ListenableFuture<Void> evalAsync(
ImmutableMap<RepositoryName, RepositoryName> repositoryMapping,
TargetPatternResolver<T> resolver,
ImmutableSet<PathFragment> blacklistedSubdirectories,
ImmutableSet<PathFragment> excludedSubdirectories,
ThreadSafeBatchCallback<T, E> callback,
Class<E> exceptionClass,
ListeningExecutorService executor) {
return evalAdaptedForAsync(
repositoryMapping,
resolver,
blacklistedSubdirectories,
excludedSubdirectories,
callback,
exceptionClass);
resolver, blacklistedSubdirectories, excludedSubdirectories, callback, exceptionClass);
}

/**
Expand Down Expand Up @@ -290,8 +276,8 @@ public PackageIdentifier getDirectoryForTargetsUnderDirectory() {
/**
* For patterns of type {@link Type#PATH_AS_TARGET}, returns the path in question.
*
* <p>The interpretation of this path, of course, depends on the existence of packages. See {@link
* TargetPattern#eval}.
* <p>The interpretation of this path, of course, depends on the existence of packages.
* See {@link InterpretPathAsTarget#eval}.
*/
public String getPathForPathAsTarget() {
throw new IllegalStateException();
Expand Down Expand Up @@ -332,15 +318,12 @@ private SingleTarget(

@Override
public <T, E extends Exception> void eval(
ImmutableMap<RepositoryName, RepositoryName> repositoryMapping,
TargetPatternResolver<T> resolver,
ImmutableSet<PathFragment> blacklistedSubdirectories,
ImmutableSet<PathFragment> excludedSubdirectories,
BatchCallback<T, E> callback,
Class<E> exceptionClass)
throws TargetParsingException, E, InterruptedException {
callback.process(
resolver.getExplicitTarget(label(targetName, repositoryMapping)).getTargets());
Class<E> exceptionClass) throws TargetParsingException, E, InterruptedException {
callback.process(resolver.getExplicitTarget(label(targetName)).getTargets());
}

@Override
Expand Down Expand Up @@ -392,17 +375,14 @@ private InterpretPathAsTarget(String path, String originalPattern, String offset

@Override
public <T, E extends Exception> void eval(
ImmutableMap<RepositoryName, RepositoryName> repositoryMapping,
TargetPatternResolver<T> resolver,
ImmutableSet<PathFragment> blacklistedSubdirectories,
ImmutableSet<PathFragment> excludedSubdirectories,
BatchCallback<T, E> callback,
Class<E> exceptionClass)
BatchCallback<T, E> callback, Class<E> exceptionClass)
throws TargetParsingException, E, InterruptedException {
if (resolver.isPackage(PackageIdentifier.createInMainRepo(path))) {
// User has specified a package name. lookout for default target.
callback.process(
resolver.getExplicitTarget(label("//" + path, repositoryMapping)).getTargets());
callback.process(resolver.getExplicitTarget(label("//" + path)).getTargets());
} else {

List<String> pieces = SLASH_SPLITTER.splitToList(path);
Expand All @@ -415,8 +395,7 @@ public <T, E extends Exception> void eval(
String targetName = SLASH_JOINER.join(pieces.subList(i, pieces.size()));
callback.process(
resolver
.getExplicitTarget(
label("//" + packageName + ":" + targetName, repositoryMapping))
.getExplicitTarget(label("//" + packageName + ":" + targetName))
.getTargets());
return;
}
Expand Down Expand Up @@ -481,12 +460,10 @@ private TargetsInPackage(String originalPattern, String offset,

@Override
public <T, E extends Exception> void eval(
ImmutableMap<RepositoryName, RepositoryName> repositoryMapping,
TargetPatternResolver<T> resolver,
ImmutableSet<PathFragment> blacklistedSubdirectories,
ImmutableSet<PathFragment> excludedSubdirectories,
BatchCallback<T, E> callback,
Class<E> exceptionClass)
BatchCallback<T, E> callback, Class<E> exceptionClass)
throws TargetParsingException, E, InterruptedException {
if (checkWildcardConflict) {
ResolvedTargets<T> targets = getWildcardConflict(resolver);
Expand Down Expand Up @@ -592,7 +569,6 @@ private TargetsBelowDirectory(

@Override
public <T, E extends Exception> void eval(
ImmutableMap<RepositoryName, RepositoryName> repositoryMapping,
TargetPatternResolver<T> resolver,
ImmutableSet<PathFragment> blacklistedSubdirectories,
ImmutableSet<PathFragment> excludedSubdirectories,
Expand All @@ -612,7 +588,6 @@ public <T, E extends Exception> void eval(

@Override
public <T, E extends Exception> ListenableFuture<Void> evalAsync(
ImmutableMap<RepositoryName, RepositoryName> repositoryMapping,
TargetPatternResolver<T> resolver,
ImmutableSet<PathFragment> blacklistedSubdirectories,
ImmutableSet<PathFragment> excludedSubdirectories,
Expand Down Expand Up @@ -907,11 +882,9 @@ public String absolutize(String pattern) {

// Parse 'label' as a Label, mapping LabelSyntaxException into
// TargetParsingException.
private static Label label(
String label, ImmutableMap<RepositoryName, RepositoryName> repositoryMapping)
throws TargetParsingException {
private static Label label(String label) throws TargetParsingException {
try {
return Label.parseAbsolute(label, repositoryMapping);
return Label.parseAbsolute(label, ImmutableMap.of());
} catch (LabelSyntaxException e) {
throw new TargetParsingException("invalid target format: '"
+ StringUtilities.sanitizeControlChars(label) + "'; "
Expand Down
Loading

0 comments on commit dac2135

Please sign in to comment.