diff options
Diffstat (limited to 'src')
9 files changed, 17 insertions, 82 deletions
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<String> 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<String, String> 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) diff --git a/src/test/java/com/google/devtools/build/lib/bazel/rules/genrule/GenRuleConfiguredTargetTest.java b/src/test/java/com/google/devtools/build/lib/bazel/rules/genrule/GenRuleConfiguredTargetTest.java index a68b5450f5..e5d1e88dc0 100644 --- a/src/test/java/com/google/devtools/build/lib/bazel/rules/genrule/GenRuleConfiguredTargetTest.java +++ b/src/test/java/com/google/devtools/build/lib/bazel/rules/genrule/GenRuleConfiguredTargetTest.java @@ -350,32 +350,6 @@ public class GenRuleConfiguredTargetTest extends BuildViewTestCase { assertThat(getPrerequisites(getConfiguredTarget("//foo:cc"), ccToolchainAttr)).isNotEmpty(); } - /** Ensure that Java make variables get expanded under the *host* configuration. */ - @Test - public void testJavaMakeVarExpansion() throws Exception { - String ruleTemplate = - "genrule(name = '%s'," - + " srcs = []," - + " cmd = 'echo $(%s) > $@'," - + " outs = ['%s'])"; - - scratch.file( - "foo/BUILD", - String.format(ruleTemplate, "java_rule", "JAVA", "java.txt"), - String.format(ruleTemplate, "javabase_rule", "JAVABASE", "javabase.txt")); - - Artifact javaOutput = getFileConfiguredTarget("//foo:java.txt").getArtifact(); - Artifact javabaseOutput = getFileConfiguredTarget("//foo:javabase.txt").getArtifact(); - - String javaCommand = - ((SpawnAction) getGeneratingAction(javaOutput)).getArguments().get(2); - assertThat(javaCommand).containsMatch("jdk/bin/java(.exe)? >"); - - String javabaseCommand = - ((SpawnAction) getGeneratingAction(javabaseOutput)).getArguments().get(2); - assertThat(javabaseCommand).contains("jdk >"); - } - // Returns the expansion of 'cmd' for the specified genrule. private String getCommand(String label) throws Exception { return getSpawnAction(label).getArguments().get(2); diff --git a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleImplementationFunctionsTest.java b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleImplementationFunctionsTest.java index af8b8aa2e6..8dfb22dc5a 100644 --- a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleImplementationFunctionsTest.java +++ b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleImplementationFunctionsTest.java @@ -1563,11 +1563,10 @@ public class SkylarkRuleImplementationFunctionsTest extends SkylarkTestCase { @Test public void testDefinedMakeVariable() throws Exception { + useConfiguration("--define=FOO=bar"); SkylarkRuleContext ctx = createRuleContext("//foo:baz"); - String java = (String) evalRuleContextCode(ctx, "ruleContext.var['JAVA']"); - // Get the last path segment - java = java.substring(java.lastIndexOf('/')); - assertThat(java).isEqualTo("/java" + OsUtils.executableExtension()); + String foo = (String) evalRuleContextCode(ctx, "ruleContext.var['FOO']"); + assertThat(foo).isEqualTo("bar"); } @Test diff --git a/src/test/shell/bazel/bazel_rules_test.sh b/src/test/shell/bazel/bazel_rules_test.sh index 5e70ebb689..9b291c4e47 100755 --- a/src/test/shell/bazel/bazel_rules_test.sh +++ b/src/test/shell/bazel/bazel_rules_test.sh @@ -325,6 +325,7 @@ function test_genrule_toolchain_dependency { genrule( name = "toolchain_check", outs = ["version"], + toolchains = ['@bazel_tools//tools/jdk:current_java_runtime'], cmd = "ls -al \$(JAVABASE) > \$@", ) EOF diff --git a/src/test/shell/integration/java_integration_test.sh b/src/test/shell/integration/java_integration_test.sh index f2f69332ff..e780b8ba6c 100755 --- a/src/test/shell/integration/java_integration_test.sh +++ b/src/test/shell/integration/java_integration_test.sh @@ -635,22 +635,23 @@ EOF function test_jvm_flags_are_passed_verbatim() { local -r pkg="${FUNCNAME[0]}" mkdir -p $pkg/java/com/google/jvmflags || fail "mkdir" - cat >$pkg/java/com/google/jvmflags/BUILD <<'EOF' + cat >$pkg/java/com/google/jvmflags/BUILD <<EOF java_binary( name = 'foo', srcs = ['Foo.java'], main_class = 'com.google.jvmflags.Foo', + toolchains = ['${TOOLS_REPOSITORY}//tools/jdk:current_java_runtime'], jvm_flags = [ # test quoting - '--a=\'single_single\'', + '--a=\\'single_single\\'', '--b="single_double"', "--c='double_single'", - "--d=\"double_double\"", + "--d=\\"double_double\\"", '--e=no_quotes', # no escaping expected - '--f=stuff$$to"escape\\', + '--f=stuff\$\$to"escape\\\\', # test make variable expansion - '--g=$(JAVABASE)', + '--g=\$(JAVABASE)', ], ) EOF diff --git a/src/test/shell/testenv.sh b/src/test/shell/testenv.sh index b9d641fac8..fde278ee30 100755 --- a/src/test/shell/testenv.sh +++ b/src/test/shell/testenv.sh @@ -545,6 +545,7 @@ setup_clean_workspace # Setting up the environment for our legacy integration tests. # PRODUCT_NAME=bazel +TOOLS_REPOSITORY="@bazel_tools" WORKSPACE_NAME=main bazelrc=$TEST_TMPDIR/bazelrc |