$ cquery --show_config_fragments: include Starlark-declared requirements
Supports https://github.com/bazelbuild/bazel/issues/10613.
PiperOrigin-RevId: 293647011
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java
index d9647af..c85ebe2 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java
@@ -289,7 +289,8 @@
* @param configuration the configuration for this rule
* @param universallyRequiredFragments fragments that are always required even if not explicitly
* specified for this rule
- * @param directlyRequiredFragments fragments directly required by this rule's definition
+ * @param configurationFragmentPolicy source of truth for the fragments required by this rule's
+ * rule class
* @param configConditions {@link FragmentOptions} required by {@code select}s on this rule. This
* is a different type than the others: options and fragments are different concepts. There's
* some subtlety to their relationship (e.g. a {@link FragmentOptions} can be associated with
@@ -304,7 +305,7 @@
Rule rule,
BuildConfiguration configuration,
Collection<Class<? extends BuildConfiguration.Fragment>> universallyRequiredFragments,
- Collection<Class<?>> directlyRequiredFragments,
+ ConfigurationFragmentPolicy configurationFragmentPolicy,
Collection<ConfigMatchingProvider> configConditions,
Iterable<ConfiguredTargetAndData> prerequisites) {
TreeSet<String> requiredFragments = new TreeSet<>();
@@ -317,9 +318,19 @@
// Add directly required fragments:
- // Fragments explicitly required by this rule:
- directlyRequiredFragments.forEach(
- fragment -> requiredFragments.add(ClassName.getSimpleNameWithOuter(fragment)));
+ // Fragments explicitly required by this rule via the native rule definition API:
+ configurationFragmentPolicy
+ .getRequiredConfigurationFragments()
+ .forEach(fragment -> requiredFragments.add(ClassName.getSimpleNameWithOuter(fragment)));
+ // Fragments explicitly required by this rule via the Starlark rule definition API:
+ configurationFragmentPolicy
+ .getRequiredStarlarkFragments()
+ .forEach(
+ starlarkName -> {
+ requiredFragments.add(
+ ClassName.getSimpleNameWithOuter(
+ configuration.getSkylarkFragmentByName(starlarkName)));
+ });
// Fragments universally required by all rules:
universallyRequiredFragments.forEach(
fragment -> requiredFragments.add(ClassName.getSimpleNameWithOuter(fragment)));
@@ -392,7 +403,7 @@
rule,
configuration,
ruleClassProvider.getUniversalFragments(),
- configurationFragmentPolicy.getRequiredConfigurationFragments(),
+ configurationFragmentPolicy,
configConditions.values(),
prerequisiteMap.values()))
.build();
diff --git a/src/main/java/com/google/devtools/build/lib/packages/ConfigurationFragmentPolicy.java b/src/main/java/com/google/devtools/build/lib/packages/ConfigurationFragmentPolicy.java
index 773210a..7dd4437 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/ConfigurationFragmentPolicy.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/ConfigurationFragmentPolicy.java
@@ -71,11 +71,14 @@
private final SetMultimap<ConfigurationTransition, Class<?>> requiredConfigurationFragments
= LinkedHashMultimap.create();
/**
- * Sets of configuration fragment names required by this rule, a set for each configuration.
- * Duplicate entries will automatically be ignored by the SetMultimap.
+ * Sets of configuration fragments required by this rule, as defined by their Starlark names
+ * (see {@link SkylarkModule}, a set for each configuration.
+ *
+ * <p>Duplicate entries will automatically be ignored by the SetMultimap.
*/
- private final SetMultimap<ConfigurationTransition, String> requiredConfigurationFragmentNames
- = LinkedHashMultimap.create();
+ private final SetMultimap<ConfigurationTransition, String>
+ starlarkRequiredConfigurationFragments = LinkedHashMultimap.create();
+
private MissingFragmentPolicy missingFragmentPolicy = MissingFragmentPolicy.FAIL_ANALYSIS;
/**
@@ -107,12 +110,11 @@
}
/**
- * Declares that the implementation of the associated rule class requires the given
- * fragments to be present in this rule's target configuration only.
+ * Declares that the implementation of the associated rule class requires the given fragments to
+ * be present in this rule's target configuration only.
*
* <p>In contrast to {@link #requiresConfigurationFragments(Collection)}, this method takes the
- * names of fragments (as determined by {@link SkylarkModule.Resolver}) instead of their
- * classes.
+ * names of fragments (as determined by {@link SkylarkModule}) instead of their classes.
*
* <p>The value is inherited by subclasses.
*/
@@ -126,13 +128,12 @@
/**
* Declares the configuration fragments that are required by this rule for the specified
- * configuration. Valid transition values are HOST for the host configuration and NONE for
- * the target configuration.
+ * configuration. Valid transition values are HOST for the host configuration and NONE for the
+ * target configuration.
*
* <p>In contrast to {@link #requiresConfigurationFragments(ConfigurationTransition,
- * Collection)}, this method takes the names of fragments (as determined by
- * {@link SkylarkModule.Resolver}) instead of their
- * classes.
+ * Collection)}, this method takes the names of fragments (as determined by {@link
+ * SkylarkModule}) instead of their classes.
*/
public Builder requiresConfigurationFragmentsBySkylarkModuleName(
ConfigurationTransition transition, Collection<String> configurationFragmentNames) {
@@ -140,7 +141,7 @@
// than its own configuration. So we don't want to casually proliferate this pattern.
Preconditions.checkArgument(
transition == NoTransition.INSTANCE || transition.isHostTransition());
- requiredConfigurationFragmentNames.putAll(transition, configurationFragmentNames);
+ starlarkRequiredConfigurationFragments.putAll(transition, configurationFragmentNames);
return this;
}
@@ -151,7 +152,7 @@
*/
public Builder includeConfigurationFragmentsFrom(ConfigurationFragmentPolicy other) {
requiredConfigurationFragments.putAll(other.requiredConfigurationFragments);
- requiredConfigurationFragmentNames.putAll(other.requiredConfigurationFragmentNames);
+ starlarkRequiredConfigurationFragments.putAll(other.starlarkRequiredConfigurationFragments);
return this;
}
@@ -167,7 +168,7 @@
public ConfigurationFragmentPolicy build() {
return new ConfigurationFragmentPolicy(
ImmutableSetMultimap.copyOf(requiredConfigurationFragments),
- ImmutableSetMultimap.copyOf(requiredConfigurationFragmentNames),
+ ImmutableSetMultimap.copyOf(starlarkRequiredConfigurationFragments),
missingFragmentPolicy);
}
}
@@ -184,7 +185,7 @@
* configuration) to lists of Skylark module names of required configuration fragments.
*/
private final ImmutableSetMultimap<ConfigurationTransition, String>
- requiredConfigurationFragmentNames;
+ starlarkRequiredConfigurationFragments;
/**
* What to do during analysis if a configuration fragment is missing.
@@ -194,10 +195,10 @@
@AutoCodec.VisibleForSerialization
ConfigurationFragmentPolicy(
ImmutableSetMultimap<ConfigurationTransition, Class<?>> requiredConfigurationFragments,
- ImmutableSetMultimap<ConfigurationTransition, String> requiredConfigurationFragmentNames,
+ ImmutableSetMultimap<ConfigurationTransition, String> starlarkRequiredConfigurationFragments,
MissingFragmentPolicy missingFragmentPolicy) {
this.requiredConfigurationFragments = requiredConfigurationFragments;
- this.requiredConfigurationFragmentNames = requiredConfigurationFragmentNames;
+ this.starlarkRequiredConfigurationFragments = starlarkRequiredConfigurationFragments;
this.missingFragmentPolicy = missingFragmentPolicy;
}
@@ -210,6 +211,17 @@
}
/**
+ * Returns the fragments required by Starlark definitions (e.g. <code>fragments = ["cpp"]</code>
+ * with the naming form seen in the Starlark API.
+ *
+ * <p>{@link
+ * com.google.devtools.build.lib.analysis.config.BuildConfiguration#getSkylarkFragmentByName} can
+ * be used to convert this to Java fragment instances.
+ */
+ public Collection<String> getRequiredStarlarkFragments() {
+ return starlarkRequiredConfigurationFragments.values();
+ }
+ /**
* Checks if the configuration fragment may be accessed (i.e., if it's declared) in the specified
* configuration (target or host).
*
@@ -240,8 +252,7 @@
SkylarkModule fragmentModule = SkylarkInterfaceUtils.getSkylarkModule(configurationFragment);
return fragmentModule != null
- ? requiredConfigurationFragmentNames.containsEntry(transition, fragmentModule.name())
- : false;
+ && starlarkRequiredConfigurationFragments.containsEntry(transition, fragmentModule.name());
}
/**
@@ -252,8 +263,7 @@
SkylarkModule fragmentModule = SkylarkInterfaceUtils.getSkylarkModule(configurationFragment);
return fragmentModule != null
- ? requiredConfigurationFragmentNames.containsValue(fragmentModule.name())
- : false;
+ && starlarkRequiredConfigurationFragments.containsValue(fragmentModule.name());
}
/**
diff --git a/src/test/shell/integration/configured_query_test.sh b/src/test/shell/integration/configured_query_test.sh
index f52143b..018143c 100755
--- a/src/test/shell/integration/configured_query_test.sh
+++ b/src/test/shell/integration/configured_query_test.sh
@@ -217,6 +217,12 @@
EOF
}
+# TODO(gregce): --show_config_fragments and RequiredConfigFragmentsProvider
+# (the native Java code that powers --show_config_fragments) were originally
+# conceived as two pieces of the same functionality. But the former is just
+# a provider, which means it can be consumed by other logic. Consider moving
+# these tests out of cquery and into proper Java integration tests.
+
function test_show_transitive_config_fragments() {
local -r pkg=$FUNCNAME
mkdir -p $pkg
@@ -618,6 +624,31 @@
assert_not_contains "//$pkg:cclib_with_select .*--define:b" output
}
+function test_show_config_fragments_on_starlark_required_fragments() {
+ local -r pkg=$FUNCNAME
+ mkdir -p $pkg
+ cat > $pkg/defs.bzl <<EOF
+def _impl(ctx):
+ pass
+
+java_requiring_rule = rule(
+ implementation = _impl,
+ fragments = ["java"],
+ attrs = {}
+)
+EOF
+ cat > $pkg/BUILD <<EOF
+load("//$pkg:defs.bzl", "java_requiring_rule")
+java_requiring_rule(
+ name = "buildme"
+)
+EOF
+
+ bazel cquery "//$pkg:all" --show_config_fragments=direct \
+ > output 2>"$TEST_log" || fail "Expected success"
+ assert_contains "//$pkg:buildme .*JavaConfiguration" output
+}
+
function test_manual_tagged_targets_always_included_for_queries() {
local -r pkg=$FUNCNAME
mkdir -p $pkg
