Skip to content

Commit

Permalink
Also record execution platforms in resolved files
Browse files Browse the repository at this point in the history
The concept of execution platforms was introduced after the introduction
of the workspace resolved file. However, when it was added, no addition
was made to the resolved file to contain that information as well. Do so
now, to ensure the resolved file still can be used as a full replacement
for the workspace file.

Fixes #8523

Change-Id: I3e31845084393d13bc917071bc458ea8a78a4336
PiperOrigin-RevId: 252016507
  • Loading branch information
aehlig authored and copybara-github committed Jun 7, 2019
1 parent 12c7942 commit 4a24f5f
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 20 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@
import java.util.HashSet;
import java.util.List;
import java.util.Set;
import java.util.stream.Collectors;

/** Syncs all repositories specified in the workspace file */
@Command(
Expand Down Expand Up @@ -146,7 +147,15 @@ public BlazeCommandResult exec(CommandEnvironment env, OptionsParsingResult opti
}
workspace = fileValue.next();
}
env.getReporter().post(toolchainEvent(fileValue.getPackage().getRegisteredToolchains()));
env.getReporter()
.post(
genericArgsCall(
"register_toolchains", fileValue.getPackage().getRegisteredToolchains()));
env.getReporter()
.post(
genericArgsCall(
"register_execution_platforms",
fileValue.getPackage().getRegisteredExecutionPlatforms()));
env.getReporter().post(new RepositoryOrderEvent(repositoryOrder.build()));

// take all skylark workspace rules and get their values
Expand Down Expand Up @@ -211,7 +220,7 @@ private static boolean shouldSync(Rule rule) {
return WHITELISTED_NATIVE_RULES.contains(rule.getRuleClassObject().getName());
}

private ResolvedEvent resolveBind(Rule rule) {
private static ResolvedEvent resolveBind(Rule rule) {
String name = rule.getName();
Object actual = rule.getAttributeContainer().getAttr("actual");
String nativeCommand =
Expand Down Expand Up @@ -240,22 +249,18 @@ public Object getResolvedInformation() {
};
}

private ResolvedEvent toolchainEvent(List<String> toolchains) {
private static ResolvedEvent genericArgsCall(String ruleName, List<String> args) {
// For the name attribute we are in a slightly tricky situation, as the ResolvedEvents are
// designed for external repositories and hence are indexted by their unique
// names. Technically, however, the list of toolchains is not associated with ant external
// repository (but still a workspace command); so we take a name that syntactially can never
// be the name of a repository, as it starts with a '//'.
String name = "//external/toolchains";
StringBuilder nativeCommandBuilder = new StringBuilder().append("register_toolchains(");
boolean firstEntryAdded = false;
for (String toolchain : toolchains) {
if (firstEntryAdded) {
nativeCommandBuilder.append(", ");
}
nativeCommandBuilder.append(Printer.getPrinter().repr(toolchain));
firstEntryAdded = true;
}
// names. Technically, however, things like the list of toolchains are not associated with any
// external repository (but still a workspace command); so we take a name that syntactially can
// never be the name of a repository, as it starts with a '//'.
String name = "//external/" + ruleName;
StringBuilder nativeCommandBuilder = new StringBuilder().append(ruleName).append("(");
nativeCommandBuilder.append(
args.stream()
.map(arg -> Printer.getPrinter().repr(arg).toString())
.collect(Collectors.joining(", ")));
nativeCommandBuilder.append(")");
String nativeCommand = nativeCommandBuilder.toString();

Expand All @@ -268,19 +273,19 @@ public String getName() {
@Override
public Object getResolvedInformation() {
return ImmutableMap.<String, Object>builder()
.put(ResolvedHashesFunction.ORIGINAL_RULE_CLASS, "register_toolchains")
.put(ResolvedHashesFunction.ORIGINAL_RULE_CLASS, ruleName)
.put(
ResolvedHashesFunction.ORIGINAL_ATTRIBUTES,
// The original attributes are a bit of a problem, as the arguments to
// register_toolchains do not at all look like those of a repository rule:
// the rule do not at all look like those of a repository rule:
// they're all positional, and, in particular, there is no keyword argument
// called "name". A lot of uses of the resolved file, however, blindly assume
// that "name" is always part of the original arguments; so we provide our
// fake name here as well, and the actual arguments under the keyword "*args",
// which hopefully reminds everyone inspecting the file of the actual syntax of
// that rule. Note that the original arguments are always ignored when bazel uses
// a resolved file instead of a workspace file.
ImmutableMap.<String, Object>of("name", name, "*args", toolchains))
ImmutableMap.<String, Object>of("name", name, "*args", args))
.put(ResolvedHashesFunction.NATIVE, nativeCommand)
.build();
}
Expand Down
6 changes: 5 additions & 1 deletion src/test/shell/bazel/workspace_resolved_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -1078,7 +1078,8 @@ EOF
}

test_toolchain_recorded() {
# Verify that the registration of toolchains is recorded in the resolved file
# Verify that the registration of toolchains and execution platforms is
# recorded in the resolved file
EXTREPODIR=`pwd`
tar xvf ${TEST_SRCDIR}/test_WORKSPACE_files/archives.tar

Expand All @@ -1087,6 +1088,7 @@ test_toolchain_recorded() {
cat > ext/toolchains.bzl <<'EOF'
def ext_toolchains():
native.register_toolchains("@ext//:toolchain")
native.register_execution_platforms("@ext//:platform")
EOF
tar cvf ext.tar ext
rm -rf ext
Expand All @@ -1110,6 +1112,8 @@ EOF

grep 'register_toolchains.*ext//:toolchain' resolved.bzl \
|| fail "tool chain not registered in resolved file"
grep 'register_execution_platforms.*ext//:platform' resolved.bzl \
|| fail "execution platform not registered in resolved file"
}

test_definition_location_recorded() {
Expand Down

0 comments on commit 4a24f5f

Please sign in to comment.