Skip to content

Commit

Permalink
Automated rollback of commit 9569407.
Browse files Browse the repository at this point in the history
PiperOrigin-RevId: 261338193
  • Loading branch information
haxorz authored and copybara-github committed Aug 2, 2019
1 parent bbf6473 commit 7ed66f7
Show file tree
Hide file tree
Showing 5 changed files with 19 additions and 170 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,8 @@
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.cmdline.RepositoryName;
import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe;
import com.google.devtools.build.lib.events.Location;
Expand Down Expand Up @@ -191,21 +189,16 @@ public void repr(SkylarkPrinter printer) {
printer.append(">");
}

private static Label transformKey(Object key, Location loc, StarlarkContext context)
throws EvalException {
private Label transformKey(Object key, Location loc) throws EvalException {
if (key instanceof Label) {
return (Label) key;
} else if (key instanceof ToolchainTypeInfo) {
return ((ToolchainTypeInfo) key).typeLabel();
} else if (key instanceof String) {
Label toolchainType;
String rawLabel = (String) key;
ImmutableMap<RepositoryName, RepositoryName> repoMapping = ImmutableMap.of();
if (context instanceof BazelStarlarkContext) {
repoMapping = ((BazelStarlarkContext) context).getRepoMapping();
}
try {
toolchainType = Label.parseAbsolute(rawLabel, repoMapping);
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 @@ -223,7 +216,7 @@ private static Label transformKey(Object key, Location loc, StarlarkContext cont
@Override
public ToolchainInfo getIndex(Object key, Location loc, StarlarkContext context)
throws EvalException {
Label toolchainTypeLabel = transformKey(key, loc, context);
Label toolchainTypeLabel = transformKey(key, loc);

if (!containsKey(key, loc, context)) {
// TODO(bazel-configurability): The list of available toolchain types is confusing in the
Expand All @@ -246,7 +239,7 @@ public ToolchainInfo getIndex(Object key, Location loc, StarlarkContext context)
@Override
public boolean containsKey(Object key, Location loc, StarlarkContext context)
throws EvalException {
Label toolchainTypeLabel = transformKey(key, loc, context);
Label toolchainTypeLabel = transformKey(key, loc);
return requestedToolchainTypeLabels().containsKey(toolchainTypeLabel);
}

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 @@ -355,14 +354,9 @@ public BaseFunction rule(
builder.setConfiguredTargetFunction(implementation);
builder.setRuleDefinitionEnvironmentLabelAndHashCode(
funcallEnv.getGlobals().getLabel(), funcallEnv.getTransitiveContentHashCode());

ImmutableMap<RepositoryName, RepositoryName> repoMapping = ImmutableMap.of();
if (context instanceof BazelStarlarkContext) {
repoMapping = ((BazelStarlarkContext) context).getRepoMapping();
}
builder.addRequiredToolchains(
collectToolchainLabels(
toolchains.getContents(String.class, "toolchains"), ast.getLocation(), repoMapping));
toolchains.getContents(String.class, "toolchains"), ast.getLocation()));

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

if (executionPlatformConstraintsAllowed) {
Expand Down Expand Up @@ -455,14 +448,11 @@ private static void addAttribute(
}

private static ImmutableList<Label> collectToolchainLabels(
Iterable<String> rawLabels,
Location loc,
ImmutableMap<RepositoryName, RepositoryName> mapping)
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, mapping);
Label toolchainLabel = Label.parseAbsolute(rawLabel, ImmutableMap.of());
requiredToolchains.add(toolchainLabel);
} catch (LabelSyntaxException e) {
throw new EvalException(
Expand All @@ -474,14 +464,11 @@ private static ImmutableList<Label> collectToolchainLabels(
}

private static ImmutableList<Label> collectConstraintLabels(
Iterable<String> rawLabels,
Location loc,
ImmutableMap<RepositoryName, RepositoryName> mapping)
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, mapping);
Label constraintLabel = Label.parseAbsolute(rawLabel, ImmutableMap.of());
constraintLabels.add(constraintLabel);
} catch (LabelSyntaxException e) {
throw new EvalException(
Expand Down Expand Up @@ -593,11 +580,7 @@ public SkylarkAspect aspect(
HostTransition.INSTANCE,
ImmutableSet.copyOf(hostFragments.getContents(String.class, "host_fragments")),
collectToolchainLabels(
toolchains.getContents(String.class, "toolchains"),
ast.getLocation(),
// TODO(https://github.com/bazelbuild/bazel/issues/7773): Update to get the
// repository mapping from a context, like in rule().
ImmutableMap.of()));
toolchains.getContents(String.class, "toolchains"), ast.getLocation()));
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -806,7 +806,7 @@ public void onLoadingComplete(

// The map from each repository to that repository's remappings map.
// This is only used in the //external package, it is an empty map for all other packages.
public final HashMap<RepositoryName, HashMap<RepositoryName, RepositoryName>>
private final HashMap<RepositoryName, HashMap<RepositoryName, RepositoryName>>
externalPackageRepositoryMappings = new HashMap<>();
// The map of repository reassignments for BUILD packages loaded within external repositories.
// It contains an entry from "@<main workspace name>" to "@" for packages within
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,15 +16,13 @@

import static com.google.devtools.build.lib.syntax.Runtime.NONE;

import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Lists;
import com.google.common.collect.Maps;
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.cmdline.TargetPattern;
import com.google.devtools.build.lib.events.Location;
import com.google.devtools.build.lib.packages.Package.NameConflictException;
import com.google.devtools.build.lib.packages.RuleFactory.InvalidRuleException;
Expand All @@ -41,7 +39,6 @@
import java.util.TreeMap;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
import javax.annotation.Nullable;

/** A collection of global skylark build API functions that apply to WORKSPACE files. */
public class WorkspaceGlobals implements WorkspaceGlobalsApi {
Expand Down Expand Up @@ -139,8 +136,7 @@ private void parseManagedDirectories(
}
}

private static RepositoryName createRepositoryName(String key, Location location)
throws EvalException {
private RepositoryName createRepositoryName(String key, Location location) throws EvalException {
if (!key.startsWith("@")) {
throw new EvalException(
location,
Expand All @@ -154,7 +150,7 @@ private static RepositoryName createRepositoryName(String key, Location location
}
}

private static List<PathFragment> getManagedDirectoriesPaths(
private List<PathFragment> getManagedDirectoriesPaths(
Object directoriesList, Location location, Map<PathFragment, String> nonNormalizedPathsMap)
throws EvalException {
if (!(directoriesList instanceof SkylarkList)) {
Expand Down Expand Up @@ -199,33 +195,15 @@ public Map<PathFragment, RepositoryName> getManagedDirectories() {
return managedDirectoriesMap;
}

private static RepositoryName getRepositoryName(@Nullable Label label) {
if (label == null) {
// registration happened directly in the main WORKSPACE
return RepositoryName.MAIN;
}

// registeration happened in a loaded bzl file
return label.getPackageIdentifier().getRepository();
}

private static List<String> renamePatterns(
List<String> patterns, Package.Builder builder, Environment env) {
RepositoryName myName = getRepositoryName(env.getGlobals().getLabel());
Map<RepositoryName, RepositoryName> renaming = builder.getRepositoryMappingFor(myName);
return patterns.stream()
.map(patternEntry -> TargetPattern.renameRepository(patternEntry, renaming))
.collect(ImmutableList.toImmutableList());
}

@Override
public NoneType registerExecutionPlatforms(
SkylarkList<?> platformLabels, Location location, Environment env)
throws EvalException, InterruptedException {
// Add to the package definition for later.
Package.Builder builder = PackageFactory.getContext(env, location).pkgBuilder;
List<String> patterns = platformLabels.getContents(String.class, "platform_labels");
builder.addRegisteredExecutionPlatforms(renamePatterns(patterns, builder, env));
builder.addRegisteredExecutionPlatforms(
platformLabels.getContents(String.class, "platform_labels"));

return NONE;
}

Expand All @@ -235,8 +213,8 @@ public NoneType registerToolchains(
throws EvalException, InterruptedException {
// Add to the package definition for later.
Package.Builder builder = PackageFactory.getContext(env, location).pkgBuilder;
List<String> patterns = toolchainLabels.getContents(String.class, "toolchain_labels");
builder.addRegisteredToolchains(renamePatterns(patterns, builder, env));
builder.addRegisteredToolchains(toolchainLabels.getContents(String.class, "toolchain_labels"));

return NONE;
}

Expand Down
105 changes: 0 additions & 105 deletions src/test/shell/bazel/workspace_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -830,109 +830,4 @@ EOF
expect_log '//external:true.*build rule.*expected'
}

function test_remap_execution_platform() {
# Regression test for issue https://github.com/bazelbuild/bazel/issues/7773,
# using the reproduction case as reported
cat > WORKSPACE <<'EOF'
workspace(name = "my_ws")
register_execution_platforms("@my_ws//platforms:my_host_platform")
EOF
mkdir platforms
cat > platforms/BUILD <<'EOF'
package(default_visibility = ["//visibility:public"])
constraint_setting(name = "machine_size")
constraint_value(name = "large_machine", constraint_setting = ":machine_size")
constraint_value(name = "small_machine", constraint_setting = ":machine_size")
platform(
name = "my_host_platform",
parents = ["@bazel_tools//platforms:host_platform"],
constraint_values = [
":large_machine"
]
)
EOF
mkdir code
cat > code/BUILD <<'EOF'
sh_library(
name = "foo",
srcs = ["foo.sh"],
exec_compatible_with = ["@my_ws//platforms:large_machine"]
)
EOF
echo exit 0 > code/foo.sh
chmod u+x code/foo.sh


bazel build --incompatible_remap_main_repo=true //code/... \
> "${TEST_log}" 2>&1 || fail "expected success"
}

function test_remap_toolchain_deps() {
# Regression test for the registration pattern of toolchains used in
# bazel-skylib, where the failure to handle it correctly caused the
# roll back of the first attempt of enabling renaming in tool chains.
cat > WORKSPACE <<'EOF'
workspace(name = "my_ws")
register_toolchains("@my_ws//toolchains:sample_toolchain")
EOF
mkdir toolchains
cat > toolchains/toolchain.bzl <<'EOF'
def _impl(ctx):
return [platform_common.ToolchainInfo()]
mytoolchain = rule(
implementation = _impl,
attrs = {},
)
EOF
cat > toolchains/rule.bzl <<'EOF'
def _impl(ctx):
# Ensure the toolchain is available under the requested (non-canonical)
# name
print("toolchain is %s" %
(ctx.toolchains["@my_ws//toolchains:my_toolchain_type"],))
pass
testrule = rule(
implementation = _impl,
attrs = {},
toolchains = ["@my_ws//toolchains:my_toolchain_type"],
)
EOF
cat > toolchains/BUILD <<'EOF'
load(":toolchain.bzl", "mytoolchain")
load(":rule.bzl", "testrule")
toolchain_type(name = "my_toolchain_type")
mytoolchain(name = "thetoolchain")
toolchain(
name = "sample_toolchain",
toolchain = ":thetoolchain",
toolchain_type = "@my_ws//toolchains:my_toolchain_type",
)
testrule(
name = "emptytoolchainconsumer",
)
EOF

bazel build --incompatible_remap_main_repo=true //toolchains/... \
|| fail "expected success"

echo; echo Without remapping of main repo; echo
# Regression test for invalidation of `--incompabtile_remap_main_repo`
# (https://github.com/bazelbuild/bazel/issues/8937). As
# building without remapping works on a clean checkout, it should also work
# on a running bazel.
bazel build --incompatible_remap_main_repo=false //toolchains/... \
|| fail "expected success"

}


run_suite "workspace tests"

0 comments on commit 7ed66f7

Please sign in to comment.