diff options
7 files changed, 105 insertions, 27 deletions
diff --git a/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/OptionsParser.java b/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/OptionsParser.java index a2556b17bf..f4cc69544d 100644 --- a/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/OptionsParser.java +++ b/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/OptionsParser.java @@ -110,7 +110,6 @@ public final class OptionsParser { // terminator to the passed arguments. collectFlagArguments(javacOpts, argQueue, "--"); bootClassPathFromJavacOpts(); - sourcePathFromJavacOpts(); break; case "--direct_dependency": { @@ -338,19 +337,6 @@ public final class OptionsParser { } } - // TODO(#970): Delete that function (either set --sourcepath from Bazel or just drop support). - private void sourcePathFromJavacOpts() { - Iterator<String> it = javacOpts.iterator(); - while (it.hasNext()) { - String curr = it.next(); - if (curr.equals("-sourcepath") && it.hasNext()) { - it.remove(); - sourcePath = it.next(); - it.remove(); - } - } - } - public List<String> getJavacOpts() { return javacOpts; } diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompilationHelper.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompilationHelper.java index a874203aac..af636c20a7 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompilationHelper.java +++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompilationHelper.java @@ -176,6 +176,7 @@ public final class JavaCompilationHelper { builder.setResourceJars(attributes.getResourceJars()); builder.addClasspathResources(attributes.getClassPathResources()); builder.setBootclasspathEntries(getBootclasspathOrDefault()); + builder.setSourcePathEntries(attributes.getSourcePath()); builder.setExtdirInputs(getExtdirInputs()); builder.setLangtoolsJar(javaToolchain.getJavac()); builder.setJavaBuilderJar(javaToolchain.getJavaBuilder()); diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompileAction.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompileAction.java index dc8a328ac1..bbd00c237a 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompileAction.java +++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompileAction.java @@ -110,6 +110,9 @@ public final class JavaCompileAction extends AbstractAction { /** The list of bootclasspath entries to specify to javac. */ private final ImmutableList<Artifact> bootclasspathEntries; + /** The list of sourcepath entries to specify to javac. */ + private final ImmutableList<Artifact> sourcePathEntries; + /** * The path to the extdir to specify to javac. */ @@ -193,6 +196,7 @@ public final class JavaCompileAction extends AbstractAction { Artifact outputJar, NestedSet<Artifact> classpathEntries, ImmutableList<Artifact> bootclasspathEntries, + ImmutableList<Artifact> sourcePathEntries, ImmutableList<Artifact> extdirInputs, List<Artifact> processorPath, List<String> processorNames, @@ -224,6 +228,7 @@ public final class JavaCompileAction extends AbstractAction { this.outputJar = outputJar; this.classpathEntries = classpathEntries; this.bootclasspathEntries = ImmutableList.copyOf(bootclasspathEntries); + this.sourcePathEntries = ImmutableList.copyOf(sourcePathEntries); this.extdirInputs = extdirInputs; this.processorPath = ImmutableList.copyOf(processorPath); this.processorNames = ImmutableList.copyOf(processorNames); @@ -275,6 +280,12 @@ public final class JavaCompileAction extends AbstractAction { return bootclasspathEntries; } + /** Returns the list of paths that represents the sourcepath. */ + @VisibleForTesting + public Collection<Artifact> getSourcePathEntries() { + return sourcePathEntries; + } + /** * Returns the path to the extdir. */ @@ -639,6 +650,7 @@ public final class JavaCompileAction extends AbstractAction { private NestedSet<Artifact> classpathEntries = NestedSetBuilder.emptySet(Order.NAIVE_LINK_ORDER); private ImmutableList<Artifact> bootclasspathEntries = ImmutableList.of(); + private ImmutableList<Artifact> sourcePathEntries = ImmutableList.of(); private ImmutableList<Artifact> extdirInputs = ImmutableList.of(); private Artifact javaBuilderJar; private Artifact langtoolsJar; @@ -721,6 +733,7 @@ public final class JavaCompileAction extends AbstractAction { classpathResources, javabaseInputs, bootclasspathEntries, + sourcePathEntries, extdirInputs)); Preconditions.checkState(javaExecutable != null, owner); @@ -779,6 +792,7 @@ public final class JavaCompileAction extends AbstractAction { outputJar, classpathEntries, bootclasspathEntries, + sourcePathEntries, extdirInputs, processorPath, processorNames, @@ -829,6 +843,9 @@ public final class JavaCompileAction extends AbstractAction { result.addJoinExecPaths( "--bootclasspath", pathSeparator, bootclasspathEntries); } + if (!sourcePathEntries.isEmpty()) { + result.addJoinExecPaths("--sourcepath", pathSeparator, sourcePathEntries); + } if (!processorPath.isEmpty() || !processorPathDirs.isEmpty()) { ImmutableList.Builder<String> execPathStrings = ImmutableList.<String>builder(); execPathStrings.addAll(Artifact.toExecPaths(processorPath)); @@ -1049,6 +1066,11 @@ public final class JavaCompileAction extends AbstractAction { return this; } + public Builder setSourcePathEntries(Iterable<Artifact> sourcePathEntries) { + this.sourcePathEntries = ImmutableList.copyOf(sourcePathEntries); + return this; + } + public Builder setExtdirInputs(Iterable<Artifact> extdirEntries) { this.extdirInputs = ImmutableList.copyOf(extdirEntries); return this; diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaLibraryHelper.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaLibraryHelper.java index d67a197d26..e14fc2c6fc 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaLibraryHelper.java +++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaLibraryHelper.java @@ -50,7 +50,7 @@ public final class JavaLibraryHelper { */ private final List<JavaCompilationArgsProvider> deps = new ArrayList<>(); private ImmutableList<String> javacOpts = ImmutableList.of(); - + private ImmutableList<Artifact> sourcePathEntries = ImmutableList.of(); private StrictDepsMode strictDepsMode = StrictDepsMode.OFF; private JavaClasspathMode classpathMode = JavaClasspathMode.OFF; @@ -113,6 +113,11 @@ public final class JavaLibraryHelper { return this; } + public JavaLibraryHelper setSourcePathEntries(List<Artifact> sourcepathEntries) { + this.sourcePathEntries = ImmutableList.copyOf(sourcepathEntries); + return this; + } + /** * When in strict mode, compiling the source-jars passed to this JavaLibraryHelper will break if * they depend on classes not in any of the {@link @@ -145,6 +150,7 @@ public final class JavaLibraryHelper { attributes.setStrictJavaDeps(strictDepsMode); attributes.setRuleKind(ruleContext.getRule().getRuleClass()); attributes.setTargetLabel(ruleContext.getLabel()); + attributes.setSourcePath(sourcePathEntries); if (isStrict() && classpathMode != JavaClasspathMode.OFF) { JavaCompilationHelper.addDependencyArtifactsToAttributes( diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaSkylarkCommon.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaSkylarkCommon.java index 1d834c0c82..434b40574d 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaSkylarkCommon.java +++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaSkylarkCommon.java @@ -161,6 +161,14 @@ public class JavaSkylarkCommon { type = ConfiguredTarget.class, doc = "A label pointing to a JDK to be used for this compilation. Mandatory." ), + @Param( + name = "sourcepath", + positional = false, + named = true, + type = SkylarkList.class, + generic1 = Artifact.class, + defaultValue = "[]" + ) } ) public JavaProvider createJavaCompileAction( @@ -172,12 +180,15 @@ public class JavaSkylarkCommon { SkylarkList<JavaProvider> deps, String strictDepsMode, ConfiguredTarget javaToolchain, - ConfiguredTarget hostJavabase) { + ConfiguredTarget hostJavabase, + SkylarkList<Artifact> sourcepathEntries) { + JavaLibraryHelper helper = new JavaLibraryHelper(skylarkRuleContext.getRuleContext()) .setOutput(outputJar) .addSourceJars(sourceJars) .addSourceFiles(sourceFiles) + .setSourcePathEntries(sourcepathEntries) .setJavacOpts(javacOpts); List<JavaCompilationArgsProvider> compilationArgsProviders = diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaTargetAttributes.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaTargetAttributes.java index f1775189a3..ab9e96422d 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaTargetAttributes.java +++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaTargetAttributes.java @@ -67,6 +67,7 @@ public class JavaTargetAttributes { NestedSetBuilder.naiveLinkOrder(); private final List<Artifact> bootClassPath = new ArrayList<>(); + private final List<Artifact> sourcePath = new ArrayList<>(); private final List<Artifact> nativeLibraries = new ArrayList<>(); private final Set<Artifact> processorPath = new LinkedHashSet<>(); @@ -207,6 +208,16 @@ public class JavaTargetAttributes { return this; } + /** + * Sets the sourcepath to be passed to the Java compiler. + */ + public Builder setSourcePath(List<Artifact> artifacts) { + Preconditions.checkArgument(!built); + Preconditions.checkArgument(sourcePath.isEmpty()); + sourcePath.addAll(artifacts); + return this; + } + public Builder addExcludedArtifacts(NestedSet<Artifact> toExclude) { Preconditions.checkArgument(!built); excludedArtifacts.addTransitive(toExclude); @@ -361,6 +372,7 @@ public class JavaTargetAttributes { runtimeClassPath, compileTimeClassPath, bootClassPath, + sourcePath, nativeLibraries, processorPath, processorPathDirs, @@ -414,6 +426,7 @@ public class JavaTargetAttributes { private final NestedSet<Artifact> compileTimeClassPath; private final ImmutableList<Artifact> bootClassPath; + private final ImmutableList<Artifact> sourcePath; private final ImmutableList<Artifact> nativeLibraries; private final ImmutableSet<Artifact> processorPath; @@ -448,6 +461,7 @@ public class JavaTargetAttributes { NestedSetBuilder<Artifact> runtimeClassPath, NestedSetBuilder<Artifact> compileTimeClassPath, List<Artifact> bootClassPath, + List<Artifact> sourcePath, List<Artifact> nativeLibraries, Set<Artifact> processorPath, Set<PathFragment> processorPathDirs, @@ -476,6 +490,7 @@ public class JavaTargetAttributes { .addTransitive(compileTimeClassPath.build()) .build(); this.bootClassPath = ImmutableList.copyOf(bootClassPath); + this.sourcePath = ImmutableList.copyOf(sourcePath); this.nativeLibraries = ImmutableList.copyOf(nativeLibraries); this.processorPath = ImmutableSet.copyOf(processorPath); this.processorPathDirs = ImmutableSet.copyOf(processorPathDirs); @@ -565,6 +580,10 @@ public class JavaTargetAttributes { return bootClassPath; } + public ImmutableList<Artifact> getSourcePath() { + return sourcePath; + } + public ImmutableSet<Artifact> getProcessorPath() { return processorPath; } diff --git a/src/test/shell/bazel/bazel_java_test.sh b/src/test/shell/bazel/bazel_java_test.sh index f1f9357b60..554eb551d1 100755 --- a/src/test/shell/bazel/bazel_java_test.sh +++ b/src/test/shell/bazel/bazel_java_test.sh @@ -105,11 +105,9 @@ function test_build_hello_world() { bazel build //java/main:main &> $TEST_log || fail "build failed" } -# Regression test for #2606: support for passing -sourcepath -# TODO(#2606): Update when a final solution is found for #2606. -function test_build_with_sourcepath() { - mkdir -p g - cat >g/A.java <<'EOF' + function test_java_common_compile_sourcepath() { + mkdir -p g + cat >g/A.java <<'EOF' package g; public class A { public A() { @@ -126,7 +124,8 @@ public class B { } EOF - cat >g/BUILD <<'EOF' + cat >g/BUILD <<'EOF' +load(':java_custom_library.bzl', 'java_custom_library') genrule( name = "stub", srcs = ["B.java"], @@ -134,15 +133,49 @@ genrule( cmd = "zip $@ $(SRCS)", ) -java_library( +java_custom_library( name = "test", srcs = ["A.java"], - javacopts = ["-sourcepath $(GENDIR)/$(location :stub)", "-implicit:none"], - deps = [":stub"] + sourcepath = [":stub"] ) EOF - bazel build //g:test >$TEST_log || fail "Failed to build //g:test" -} + + cat >g/java_custom_library.bzl <<'EOF' +def _impl(ctx): + output_jar = ctx.new_file("lib" + ctx.label.name + ".jar") + + compilation_provider = java_common.compile( + ctx, + source_files = ctx.files.srcs, + output = output_jar, + javac_opts = java_common.default_javac_opts(ctx, java_toolchain_attr = "_java_toolchain"), + deps = [], + sourcepath = ctx.files.sourcepath, + strict_deps = "ERROR", + java_toolchain = ctx.attr._java_toolchain, + host_javabase = ctx.attr._host_javabase + ) + return struct( + files = set([output_jar]), + providers = [compilation_provider] + ) + +java_custom_library = rule( + implementation = _impl, + attrs = { + "srcs": attr.label_list(allow_files=True), + "sourcepath": attr.label_list(), + "_java_toolchain": attr.label(default = Label("@bazel_tools//tools/jdk:toolchain")), + "_host_javabase": attr.label(default = Label("//tools/defaults:jdk")) + }, + fragments = ["java"] +) +EOF + + # TODO(elenairina): Check that B.jar is not on the output jar after -implicit:none will be turned + # on by default. + bazel build //g:test >$TEST_log || fail "Failed to build //g:test" + } # Runfiles is disabled by default on Windows, but we can test it on Unix by # adding flag --experimental_enable_runfiles=0 |