From 6183faff6c627d5c0b36f36624f78de2f9dcbab0 Mon Sep 17 00:00:00 2001 From: lberki Date: Wed, 6 Dec 2017 01:49:14 -0800 Subject: Disallow undeclared access to the $(JAVA) and $(JAVABASE) template variables. If a rule needs these template variables, it will need to declare a dependency on them in the future by adding @bazel_tools//tools/jdk:current_java_runtime to its toolchains= attribute. RELNOTES[INC]: In order to access the template variables $(JAVA) and $(JAVABASE), @bazel_tools//tools/jdk:current_java_runtime needs to be added to the toolchains= attribute from now on. RELNOTES: None. PiperOrigin-RevId: 178070807 --- .../devtools/build/lib/rules/genrule/GenRuleBase.java | 16 ---------------- .../devtools/build/lib/rules/java/JavaOptions.java | 13 ------------- .../com/google/devtools/build/lib/rules/java/Jvm.java | 13 +------------ .../build/lib/rules/java/JvmConfigurationLoader.java | 11 +++++------ 4 files changed, 6 insertions(+), 47 deletions(-) (limited to 'src/main') diff --git a/src/main/java/com/google/devtools/build/lib/rules/genrule/GenRuleBase.java b/src/main/java/com/google/devtools/build/lib/rules/genrule/GenRuleBase.java index dc1ff59a8a..ec27c3acc0 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/genrule/GenRuleBase.java +++ b/src/main/java/com/google/devtools/build/lib/rules/genrule/GenRuleBase.java @@ -46,7 +46,6 @@ import com.google.devtools.build.lib.rules.java.JavaHelper; import com.google.devtools.build.lib.syntax.Type; import com.google.devtools.build.lib.util.LazyString; import com.google.devtools.build.lib.vfs.PathFragment; -import java.util.ArrayList; import java.util.List; import java.util.Map; import java.util.regex.Pattern; @@ -347,21 +346,6 @@ public abstract class GenRuleBase implements RuleConfiguredTargetFactory { return valueFromToolchains; } - // We use the presence of the Java Make variable in the current configuration as a proxy for - // whether Java Make variables are enabled. This lets us avoid declaring a dependency on the - // Jvm fragment (genrules should not depend on Java so they shouldn't do that). - if (JDK_MAKE_VARIABLE.matcher("$(" + variableName + ")").find() - && ruleContext.getConfiguration().getMakeEnvironment().containsKey(variableName)) { - List attributes = new ArrayList<>(); - attributes.addAll(ConfigurationMakeVariableContext.DEFAULT_MAKE_VARIABLE_ATTRIBUTES); - attributes.add(":host_jdk"); - return new ConfigurationMakeVariableContext( - ruleContext.getMakeVariables(attributes), - ruleContext.getTarget().getPackage(), - ruleContext.getHostConfiguration()) - .lookupVariable(variableName); - } - return super.lookupVariable(variableName); } diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaOptions.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaOptions.java index db48af74a1..fc5bca7fad 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaOptions.java +++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaOptions.java @@ -531,18 +531,6 @@ public class JavaOptions extends FragmentOptions { ) public boolean jplPropagateCcLinkParamsStore; - @Option( - name = "experimental_enable_jvm_configuration_make_variables", - defaultValue = "true", - documentationCategory = OptionDocumentationCategory.UNDOCUMENTED, - effectTags = {OptionEffectTag.UNKNOWN}, - metadataTags = {OptionMetadataTag.INCOMPATIBLE_CHANGE}, - help = "If enabled, the Java configuration fragment supplies the JAVA and JAVABASE " - + "Make variables. This option is used in the migration to remove them in favor of " - + "requiring an explicit dependency on the Java runtime for rules that use them." - ) - public boolean enableMakeVariables; - // Plugins are built using the host config. To avoid cycles we just don't propagate // this option to the host config. If one day we decide to use plugins when building // host tools, we can improve this by (for example) creating a compiler configuration that is @@ -588,7 +576,6 @@ public class JavaOptions extends FragmentOptions { host.allowRuntimeDepsOnNeverLink = allowRuntimeDepsOnNeverLink; host.jplPropagateCcLinkParamsStore = jplPropagateCcLinkParamsStore; - host.enableMakeVariables = enableMakeVariables; return host; } diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/Jvm.java b/src/main/java/com/google/devtools/build/lib/rules/java/Jvm.java index f99ca11610..f9724fc2f5 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/java/Jvm.java +++ b/src/main/java/com/google/devtools/build/lib/rules/java/Jvm.java @@ -15,7 +15,6 @@ package com.google.devtools.build.lib.rules.java; import com.google.common.base.Preconditions; -import com.google.common.collect.ImmutableMap.Builder; import com.google.devtools.build.lib.analysis.RuleContext; import com.google.devtools.build.lib.analysis.config.BuildConfiguration; import com.google.devtools.build.lib.cmdline.Label; @@ -40,7 +39,6 @@ public final class Jvm extends BuildConfiguration.Fragment { private final PathFragment javaHome; private final Label jvmLabel; private final PathFragment java; - private final boolean enableMakeVariables; public static final String BIN_JAVA = "bin/java" + OsUtils.executableExtension(); @@ -49,12 +47,11 @@ public final class Jvm extends BuildConfiguration.Fragment { * and/or the {@code jvmLabel} parameter must be non-null. Only the * {@code jvmLabel} is optional. */ - public Jvm(PathFragment javaHome, Label jvmLabel, boolean enableMakeVariables) { + public Jvm(PathFragment javaHome, Label jvmLabel) { Preconditions.checkArgument(javaHome.isAbsolute() || jvmLabel != null); this.javaHome = javaHome; this.jvmLabel = jvmLabel; this.java = javaHome.getRelative(BIN_JAVA); - this.enableMakeVariables = enableMakeVariables; } /** @@ -82,12 +79,4 @@ public final class Jvm extends BuildConfiguration.Fragment { public PathFragment getJavaHome() { return javaHome; } - - @Override - public void addGlobalMakeVariables(Builder globalMakeEnvBuilder) { - if (enableMakeVariables) { - globalMakeEnvBuilder.put("JAVABASE", javaHome.getPathString()); - globalMakeEnvBuilder.put("JAVA", java.getPathString()); - } - } } 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 fe66847260..10367cf631 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 @@ -59,8 +59,7 @@ public final class JvmConfigurationLoader implements ConfigurationFragmentFactor String cpu = buildOptions.get(BuildConfiguration.Options.class).cpu; - return createFromJavaRuntimeSuite(env, javaOptions.javaBase, cpu, - javaOptions.enableMakeVariables); + return createFromJavaRuntimeSuite(env, javaOptions.javaBase, cpu); } @Override @@ -75,7 +74,7 @@ public final class JvmConfigurationLoader implements ConfigurationFragmentFactor @Nullable private static Jvm createFromJavaRuntimeSuite( - ConfigurationEnvironment lookup, Label javaBase, String cpu, boolean enableMakeVariables) + ConfigurationEnvironment lookup, Label javaBase, String cpu) throws InvalidConfigurationException, InterruptedException { try { javaBase = RedirectChaser.followRedirects(lookup, javaBase, "jdk"); @@ -90,7 +89,7 @@ public final class JvmConfigurationLoader implements ConfigurationFragmentFactor + ((Rule) javaHomeTarget).getRuleClass() + "'. Expected java_runtime_suite"); } - return createFromRuntimeSuite(lookup, (Rule) javaHomeTarget, cpu, enableMakeVariables); + return createFromRuntimeSuite(lookup, (Rule) javaHomeTarget, cpu); } throw new InvalidConfigurationException( "No JVM target found under " + javaBase + " that would work for " + cpu); @@ -103,7 +102,7 @@ public final class JvmConfigurationLoader implements ConfigurationFragmentFactor // TODO(b/34175492): eventually the Jvm fragement will containg only the label of a java_runtime // rule, and all of the configuration will be accessed using JavaRuntimeInfo. private static Jvm createFromRuntimeSuite(ConfigurationEnvironment lookup, Rule javaRuntimeSuite, - String cpu, boolean enableMakeVariables) + String cpu) throws InvalidConfigurationException, InterruptedException, NoSuchTargetException, NoSuchPackageException { Label javaRuntimeLabel = selectRuntime(javaRuntimeSuite, cpu); @@ -133,7 +132,7 @@ public final class JvmConfigurationLoader implements ConfigurationFragmentFactor javaHomePath, srcs.toString())); } } - return new Jvm(javaHomePath, javaRuntimeSuite.getLabel(), enableMakeVariables); + return new Jvm(javaHomePath, javaRuntimeSuite.getLabel()); } private static Label selectRuntime(Rule javaRuntimeSuite, String cpu) -- cgit v1.2.3