Skip to content

Commit

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

Breaks Skylib tests: https://buildkite.com/bazel/bazel-at-head-plus-downstream/builds/1082#95e91cd3-c796-408b-83ab-9991efcf1292
Culprit finder: https://buildkite.com/bazel/culprit-finder/builds/183#985aa488-33c9-4dcc-8561-a96c6e1fb4ed

*** Original change description ***

register_{toolchains, execution_platforms}: honor renaming

When handling the registration of toolchains or execution platforms,
take the applicable renaming into account. This, in particular, includes
the renaming from the name of the main workspace to the canonical '@'.

Fixes bazelbuild#7773.

Change-Id: I22fe84e337ad6d33df2a9263e6671e2e0d0a284d
PiperOrigin-RevId: 257214983
  • Loading branch information
vladmos authored and irengrig committed Jul 15, 2019
1 parent dc4df1d commit 094225e
Show file tree
Hide file tree
Showing 3 changed files with 8 additions and 70 deletions.
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
40 changes: 0 additions & 40 deletions src/test/shell/bazel/workspace_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -830,44 +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"
}

run_suite "workspace tests"

0 comments on commit 094225e

Please sign in to comment.