Make the "actual" argument of bind() optional and do not point //external:android/sdk anywhere so that no Android-specific package is loaded when the user doesn't use an android_sdk_repository rule.
To this end, SkyframePackageLoaderWithValueEnvironment.getLoadedTarget() doesn't resolve //external: labels anymore. This was only needed for JVM resolution, which was dealt with by adding and extra RedirectChaser.followRedirect() call to JvmConfigurationLoader. One hack less.
On the flip side, BazelConfigurationCollection.collectTransitiveClosure() grew a hack to handle bind(), but that method is awful enough as it is anyway.
--
MOS_MIGRATED_REVID=97307779
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelConfigurationCollection.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelConfigurationCollection.java
index d6dd55e..61ef8f4 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelConfigurationCollection.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelConfigurationCollection.java
@@ -250,6 +250,13 @@
}
}
}
+
+ if (rule.getRuleClass().equals("bind")) {
+ Label actual = AggregatingAttributeMapper.of(rule).get("actual", Type.LABEL);
+ if (actual != null) {
+ collectTransitiveClosure(loadedPackageProvider, reachableLabels, actual);
+ }
+ }
}
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/android/android.WORKSPACE b/src/main/java/com/google/devtools/build/lib/bazel/rules/android/android.WORKSPACE
index a3f4130..17b8ac1 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/rules/android/android.WORKSPACE
+++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/android/android.WORKSPACE
@@ -12,4 +12,4 @@
bind(name = "android/shuffle_jars", actual = "//tools/android:shuffle_jars")
bind(name = "android/merge_dexzips", actual = "//tools/android:merge_dexzips")
bind(name = "android/debug_keystore", actual = "//tools/android:debug_keystore")
-bind(name = "android/sdk", actual = "//tools/android:sdk")
+bind(name = "android/sdk")
diff --git a/src/main/java/com/google/devtools/build/lib/packages/ExternalPackage.java b/src/main/java/com/google/devtools/build/lib/packages/ExternalPackage.java
index 9a20f7b..5f5c173 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/ExternalPackage.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/ExternalPackage.java
@@ -166,6 +166,10 @@
private void resolveLabel(final Label virtual, Binding binding)
throws NoSuchBindingException {
Label actual = binding.getActual();
+ if (actual == null) {
+ return;
+ }
+
Label tortoise = virtual;
Label hare = actual;
boolean moveTortoise = true;
@@ -202,7 +206,9 @@
// Bound rules don't have a name field, but this works because we don't want more than one
// with the same virtual name.
attributes.put("name", virtual.getName());
- attributes.put("actual", actual);
+ if (actual != null) {
+ attributes.put("actual", actual);
+ }
StoredEventHandler handler = new StoredEventHandler();
Rule rule = RuleFactory.createAndAddRule(this, klass, attributes, handler, null, location);
rule.setVisibility(ConstantRuleVisibility.PUBLIC);
diff --git a/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactory.java b/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactory.java
index a304c6d..ddb1471 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactory.java
@@ -91,13 +91,14 @@
private static BuiltinFunction newBindFunction(final Builder builder) {
return new BuiltinFunction("bind",
- FunctionSignature.namedOnly("name", "actual"), BuiltinFunction.USE_LOC) {
+ FunctionSignature.namedOnly(1, "name", "actual"), BuiltinFunction.USE_LOC) {
public Object invoke(String name, String actual, Location loc)
throws EvalException, InterruptedException {
Label nameLabel = null;
try {
nameLabel = Label.parseAbsolute("//external:" + name);
- builder.addBinding(nameLabel, new Binding(Label.parseAbsolute(actual), loc));
+ Binding binding = new Binding(actual == null ? null : Label.parseAbsolute(actual), loc);
+ builder.addBinding(nameLabel, binding);
} catch (Label.SyntaxException e) {
throw new EvalException(loc, e.getMessage());
}
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidConfiguration.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidConfiguration.java
index 1eb3a16..e7892bc 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidConfiguration.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidConfiguration.java
@@ -18,7 +18,6 @@
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Multimap;
import com.google.devtools.build.lib.Constants;
-import com.google.devtools.build.lib.analysis.RedirectChaser;
import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
import com.google.devtools.build.lib.analysis.config.BuildConfiguration.Fragment;
import com.google.devtools.build.lib.analysis.config.BuildConfiguration.LabelConverter;
@@ -151,9 +150,7 @@
@Override
public Fragment create(ConfigurationEnvironment env, BuildOptions buildOptions)
throws InvalidConfigurationException {
- Options options = buildOptions.get(Options.class);
- Label sdk = RedirectChaser.followRedirects(env, options.realSdk(), "android_sdk");
- return new AndroidConfiguration(options, sdk);
+ return new AndroidConfiguration(buildOptions.get(Options.class));
}
@Override
@@ -176,8 +173,8 @@
private final boolean useJackForDexing;
private final boolean jackSanityChecks;
- AndroidConfiguration(Options options, Label sdk) {
- this.sdk = sdk;
+ AndroidConfiguration(Options options) {
+ this.sdk = options.realSdk();
this.strictDeps = options.strictDeps;
this.legacyNativeSupport = options.legacyNativeSupport;
this.cpu = options.cpu;
diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JvmConfigurationLoader.java b/src/main/java/com/google/devtools/build/lib/rules/java/JvmConfigurationLoader.java
index a19461a..9ff7651 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/java/JvmConfigurationLoader.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/java/JvmConfigurationLoader.java
@@ -112,6 +112,12 @@
List<Label> labels = javaHomeAttributes.get("srcs", Type.LABEL_LIST);
for (Label jvmLabel : labels) {
if (jvmLabel.getName().endsWith("-" + cpu)) {
+ jvmLabel = RedirectChaser.followRedirects(
+ lookup, jvmLabel, "Architecture-specific JDK");
+ if (jvmLabel == null) {
+ return null;
+ }
+
Target jvmTarget = lookup.getTarget(jvmLabel);
if (jvmTarget == null) {
return null;
diff --git a/src/main/java/com/google/devtools/build/lib/rules/workspace/Bind.java b/src/main/java/com/google/devtools/build/lib/rules/workspace/Bind.java
index 49f829a..e87fe30 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/workspace/Bind.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/workspace/Bind.java
@@ -25,6 +25,8 @@
import com.google.devtools.build.lib.analysis.RuleContext;
import com.google.devtools.build.lib.analysis.TransitiveInfoProvider;
import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
+import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
+import com.google.devtools.build.lib.collect.nestedset.Order;
import com.google.devtools.build.lib.packages.Target;
import com.google.devtools.build.lib.rules.RuleConfiguredTargetFactory;
import com.google.devtools.build.lib.syntax.ClassObject;
@@ -56,7 +58,7 @@
@Override
public <P extends TransitiveInfoProvider> P getProvider(Class<P> provider) {
- return configuredTarget.getProvider(provider);
+ return configuredTarget == null ? null : configuredTarget.getProvider(provider);
}
@Override
@@ -66,17 +68,19 @@
@Override
public Object get(String providerKey) {
- return configuredTarget.get(providerKey);
+ return configuredTarget == null ? null : configuredTarget.get(providerKey);
}
@Override
public UnmodifiableIterator<TransitiveInfoProvider> iterator() {
- return configuredTarget.iterator();
+ return configuredTarget == null
+ ? ImmutableList.<TransitiveInfoProvider>of().iterator()
+ : configuredTarget.iterator();
}
@Override
public Target getTarget() {
- return configuredTarget.getTarget();
+ return configuredTarget == null ? null : configuredTarget.getTarget();
}
@Override
@@ -94,20 +98,22 @@
// A shortcut for files to build in Skylark. FileConfiguredTarget and RunleConfiguredTarget
// always has FileProvider and Error- and PackageGroupConfiguredTarget-s shouldn't be
// accessible in Skylark.
- return SkylarkNestedSet.of(
- Artifact.class, getProvider(FileProvider.class).getFilesToBuild());
+ return SkylarkNestedSet.of(Artifact.class, configuredTarget == null
+ ? NestedSetBuilder.<Artifact>emptySet(Order.STABLE_ORDER)
+ : getProvider(FileProvider.class).getFilesToBuild());
}
- return configuredTarget.get(name);
+ return configuredTarget == null ? null : configuredTarget.get(name);
}
@SuppressWarnings("cast")
@Override
public ImmutableCollection<String> getKeys() {
- return new ImmutableList.Builder<String>()
- .add("label", "files")
- .addAll(configuredTarget.getProvider(RuleConfiguredTarget.SkylarkProviders.class)
- .getKeys())
- .build();
+ ImmutableList.Builder<String> result = ImmutableList.<String>builder().add("label", "files");
+ if (configuredTarget != null) {
+ result.addAll(
+ configuredTarget.getProvider(RuleConfiguredTarget.SkylarkProviders.class).getKeys());
+ }
+ return result.build();
}
@Override
@@ -121,5 +127,4 @@
public ConfiguredTarget create(RuleContext ruleContext) throws InterruptedException {
return new BindConfiguredTarget(ruleContext);
}
-
}
diff --git a/src/main/java/com/google/devtools/build/lib/rules/workspace/BindRule.java b/src/main/java/com/google/devtools/build/lib/rules/workspace/BindRule.java
index ce2aa2e..8db1f69 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/workspace/BindRule.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/workspace/BindRule.java
@@ -37,6 +37,12 @@
${SYNOPSIS}
<p>This target must exist, but can be any type of rule (including bind).</p>
+
+ <p>If this attribute is omitted, rules referring to this target in <code>//external</code>
+ will simply not see this dependency edge. Note that this is different from omitting the
+ <code>bind</code> rule completely: it is an error if an <code>//external</code> dependency
+ does not have an associated <code>bind</code> rule.
+ </p>
<!-- #END_BLAZE_RULE.ATTRIBUTE --> */
.add(attr("actual", LABEL).allowedFileTypes(FileTypeSet.ANY_FILE))
.setWorkspaceOnly()
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetFunction.java
index dff91d6..c0f7908 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetFunction.java
@@ -15,6 +15,7 @@
import com.google.common.base.Function;
import com.google.common.base.Preconditions;
+import com.google.common.base.Verify;
import com.google.common.collect.ArrayListMultimap;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables;
@@ -248,8 +249,14 @@
Dependency dep = entry.getValue();
SkyKey depKey = TO_KEYS.apply(dep);
ConfiguredTarget depConfiguredTarget = depConfiguredTargetMap.get(depKey);
- result.put(entry.getKey(),
- RuleConfiguredTarget.mergeAspects(depConfiguredTarget, depAspectMap.get(depKey)));
+ Verify.verify(depConfiguredTarget != null);
+ if (depConfiguredTarget.getTarget() != null) {
+ // This condition will be false if this is a //external label defined by a bind() rule
+ // without an actual= attribute. In this case, we pretend that the dependency does not
+ // exist.
+ result.put(entry.getKey(),
+ RuleConfiguredTarget.mergeAspects(depConfiguredTarget, depAspectMap.get(depKey)));
+ }
}
return result;
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframePackageLoaderWithValueEnvironment.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframePackageLoaderWithValueEnvironment.java
index 9478daf..e467ae0 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframePackageLoaderWithValueEnvironment.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframePackageLoaderWithValueEnvironment.java
@@ -18,7 +18,6 @@
import com.google.devtools.build.lib.analysis.config.BuildOptions;
import com.google.devtools.build.lib.analysis.config.InvalidConfigurationException;
import com.google.devtools.build.lib.analysis.config.PackageProviderForConfigurations;
-import com.google.devtools.build.lib.packages.ExternalPackage;
import com.google.devtools.build.lib.packages.NoSuchPackageException;
import com.google.devtools.build.lib.packages.NoSuchTargetException;
import com.google.devtools.build.lib.packages.Package;
@@ -78,12 +77,6 @@
public Target getLoadedTarget(Label label) throws NoSuchPackageException,
NoSuchTargetException {
Package pkg = getLoadedPackage(label.getPackageIdentifier());
- if (ExternalPackage.isExternal(pkg)) {
- label = ((ExternalPackage) pkg).getActualLabel(label);
- if (label != null) {
- pkg = getLoadedPackage(label.getPackageIdentifier());
- }
- }
return pkg == null ? null : pkg.getTarget(label.getName());
}
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/mock/BazelAnalysisMock.java b/src/test/java/com/google/devtools/build/lib/analysis/mock/BazelAnalysisMock.java
index 0e4b323..8a66f0f 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/mock/BazelAnalysisMock.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/mock/BazelAnalysisMock.java
@@ -53,7 +53,8 @@
"filegroup(name='langtools', srcs=['jdk/lib/tools.jar'])",
"filegroup(name='bootclasspath', srcs=['jdk/jre/lib/rt.jar'])",
"filegroup(name='extdir', srcs=glob(['jdk/jre/lib/ext/*']))",
- "filegroup(name='java', srcs = ['jdk/jre/bin/java'])",
+ // "dummy" is needed so that RedirectChaser stops here
+ "filegroup(name='java', srcs = ['jdk/jre/bin/java', 'dummy'])",
"exports_files(['JavaBuilder_deploy.jar','SingleJar_deploy.jar',",
" 'JavaBuilderCanary_deploy.jar', 'ijar'])");
config.create("tools/cpp/BUILD",
