Skip to content

Commit

Permalink
Split classpaths more eagerly
Browse files Browse the repository at this point in the history 
Allow classpath command line options to be whitespace-separated, and store
as lists instead of joined strings. This avoids some unnecessary
string operations, and keeps command lines below the per-arg limit.

A future change will update Bazel to pass unjoined classpaths.

PiperOrigin-RevId: 156085128
  • Loading branch information
@cushon @dslomov
cushon authored and dslomov committed May 15, 2017
1 parent c78c947 commit 431ce43
Show file tree
Hide file tree
Showing 6 changed files with 81 additions and 103 deletions.
65 changes: 23 additions & 42 deletions src/java_tools/buildjar/java/com/google/devtools/build/buildjar/JavaLibraryBuildRequest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,17 +14,18 @@

package com.google.devtools.build.buildjar;

import static com.google.common.collect.ImmutableList.toImmutableList;

import com.google.common.base.Joiner;
import com.google.common.base.Splitter;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables;
import com.google.common.collect.Streams;
import com.google.devtools.build.buildjar.instrumentation.JacocoInstrumentationProcessor;
import com.google.devtools.build.buildjar.javac.BlazeJavacArguments;
import com.google.devtools.build.buildjar.javac.plugins.BlazeJavaCompilerPlugin;
import com.google.devtools.build.buildjar.javac.plugins.dependency.DependencyModule;
import com.google.devtools.build.buildjar.javac.plugins.processing.AnnotationProcessingModule;
import java.io.File;
import java.io.IOException;
import java.nio.file.Path;
import java.nio.file.Paths;
Expand Down Expand Up @@ -53,12 +54,12 @@ public final class JavaLibraryBuildRequest {
/** Resource files that should be put in the root of the output jar. */
private final ImmutableList<String> rootResourceFiles;

private final String sourcePath;
private final String classPath;
private final String bootClassPath;
private final String extdir;
private final ImmutableList<String> sourcePath;
private final ImmutableList<String> classPath;
private final ImmutableList<String> bootClassPath;
private final ImmutableList<String> extClassPath;

private final String processorPath;
private final ImmutableList<String> processorPath;
private final List<String> processorNames;

private final String outputJar;
Expand Down Expand Up @@ -150,11 +151,11 @@ public JavaLibraryBuildRequest(
this.resourceFiles = ImmutableList.copyOf(optionsParser.getResourceFiles());
this.resourceJars = ImmutableList.copyOf(optionsParser.getResourceJars());
this.rootResourceFiles = ImmutableList.copyOf(optionsParser.getRootResourceFiles());
this.classPath = optionsParser.getClassPath();
this.sourcePath = optionsParser.getSourcePath();
this.bootClassPath = optionsParser.getBootClassPath();
this.extdir = optionsParser.getExtdir();
this.processorPath = optionsParser.getProcessorPath();
this.classPath = ImmutableList.copyOf(optionsParser.getClassPath());
this.sourcePath = ImmutableList.copyOf(optionsParser.getSourcePath());
this.bootClassPath = ImmutableList.copyOf(optionsParser.getBootClassPath());
this.extClassPath = ImmutableList.copyOf(optionsParser.getExtClassPath());
this.processorPath = ImmutableList.copyOf(optionsParser.getProcessorPath());
this.processorNames = optionsParser.getProcessorNames();
// Since the default behavior of this tool with no arguments is "rm -fr <classDir>", let's not
// default to ".", shall we?
Expand Down Expand Up @@ -197,7 +198,7 @@ public String getSourceGenDir() {
return sourceGenDir;
}

public String getSourcePath() {
public ImmutableList<String> getSourcePath() {
return sourcePath;
}

Expand Down Expand Up @@ -234,19 +235,19 @@ public ImmutableList<String> getRootResourceFiles() {
return rootResourceFiles;
}

public String getClassPath() {
public ImmutableList<String> getClassPath() {
return classPath;
}

public String getBootClassPath() {
public ImmutableList<String> getBootClassPath() {
return bootClassPath;
}

public String getExtdir() {
return extdir;
public ImmutableList<String> getExtClassPath() {
return extClassPath;
}

public String getProcessorPath() {
public ImmutableList<String> getProcessorPath() {
return processorPath;
}

Expand Down Expand Up @@ -288,13 +289,11 @@ public ImmutableList<BlazeJavaCompilerPlugin> getPlugins() {
return plugins;
}

public BlazeJavacArguments toBlazeJavacArguments(final String classPath) {
public BlazeJavacArguments toBlazeJavacArguments(ImmutableList<String> classPath) {
return BlazeJavacArguments.builder()
.classPath(toPaths(classPath))
.classOutput(Paths.get(getClassDir()))
.bootClassPath(
ImmutableList.copyOf(
Iterables.concat(toPaths(getBootClassPath()), toPaths(getExtdir()))))
.bootClassPath(toPaths(Iterables.concat(getBootClassPath(), getExtClassPath())))
.javacOptions(makeJavacArguments())
.sourceFiles(ImmutableList.copyOf(getSourceFiles()))
.processors(null)
Expand All @@ -305,26 +304,8 @@ public BlazeJavacArguments toBlazeJavacArguments(final String classPath) {
.build();
}

static ImmutableList<Path> toPaths(List<String> files) {
if (files == null) {
return ImmutableList.of();
}
ImmutableList.Builder<Path> result = ImmutableList.builder();
for (String e : files) {
result.add(Paths.get(e));
}
return result.build();
}

static ImmutableList<Path> toPaths(String classPath) {
if (classPath == null) {
return ImmutableList.of();
}
ImmutableList.Builder<Path> result = ImmutableList.builder();
for (String e : Splitter.on(File.pathSeparatorChar).split(classPath)) {
result.add(Paths.get(e));
}
return result.build();
static ImmutableList<Path> toPaths(Iterable<String> files) {
return Streams.stream(files).map(Paths::get).collect(toImmutableList());
}

/** Constructs a command line that can be used for a javac invocation. */
Expand Down
51 changes: 34 additions & 17 deletions src/java_tools/buildjar/java/com/google/devtools/build/buildjar/OptionsParser.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@

import com.google.common.base.Preconditions;
import com.google.common.base.Splitter;
import com.google.common.collect.Iterables;
import java.io.File;
import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Paths;
Expand Down Expand Up @@ -64,12 +66,12 @@ public final class OptionsParser {
private final List<String> resourceJars = new ArrayList<>();
private final List<String> rootResourceFiles = new ArrayList<>();

private String classPath = "";
private String sourcePath;
private String bootClassPath;
private String extdir;
private final List<String> classPath = new ArrayList<>();
private final List<String> sourcePath = new ArrayList<>();
private final List<String> bootClassPath = new ArrayList<>();
private final List<String> extClassPath = new ArrayList<>();

private String processorPath = "";
private final List<String> processorPath = new ArrayList<>();
private final List<String> processorNames = new ArrayList<>();

private String outputJar;
Expand Down Expand Up @@ -170,23 +172,24 @@ private void processCommandlineArgs(Deque<String> argQueue) throws InvalidComman
collectFlagArguments(rootResourceFiles, argQueue, "-");
break;
case "--classpath":
classPath = getArgument(argQueue, arg);
collectClassPathArguments(classPath, argQueue);
break;
// TODO(#970): Consider wether we want to use --sourcepath for resolving of #970.
case "--sourcepath":
sourcePath = getArgument(argQueue, arg);
collectClassPathArguments(sourcePath, argQueue);
break;
case "--bootclasspath":
bootClassPath = getArgument(argQueue, arg);
collectClassPathArguments(bootClassPath, argQueue);
break;
case "--processorpath":
processorPath = getArgument(argQueue, arg);
collectClassPathArguments(processorPath, argQueue);
break;
case "--processors":
collectProcessorArguments(processorNames, argQueue, "-");
break;
case "--extclasspath":
case "--extdir":
extdir = getArgument(argQueue, arg);
collectClassPathArguments(extClassPath, argQueue);
break;
case "--output":
outputJar = getArgument(argQueue, arg);
Expand Down Expand Up @@ -228,7 +231,7 @@ private void sourcePathFromJavacOpts() {
String curr = it.next();
if (curr.equals("-sourcepath") && it.hasNext()) {
it.remove();
sourcePath = it.next();
Iterables.addAll(sourcePath, CLASSPATH_SPLITTER.split(it.next()));
it.remove();
}
}
Expand Down Expand Up @@ -303,6 +306,20 @@ private static void collectFlagArguments(
}
}

private static final Splitter CLASSPATH_SPLITTER =
Splitter.on(File.pathSeparatorChar).trimResults().omitEmptyStrings();

// TODO(cushon): stop splitting classpaths once cl/127006119 is released
private static void collectClassPathArguments(Collection<String> output, Deque<String> args) {
for (String arg = args.pollFirst(); arg != null; arg = args.pollFirst()) {
if (arg.startsWith("-")) {
args.addFirst(arg);
break;
}
Iterables.addAll(output, CLASSPATH_SPLITTER.split(arg));
}
}

/**
* Collects the arguments for the --processors command line flag until it finds a flag that starts
* with the terminatorPrefix.
Expand Down Expand Up @@ -411,23 +428,23 @@ public List<String> getRootResourceFiles() {
return rootResourceFiles;
}

public String getClassPath() {
public List<String> getClassPath() {
return classPath;
}

public String getBootClassPath() {
public List<String> getBootClassPath() {
return bootClassPath;
}

public String getSourcePath() {
public List<String> getSourcePath() {
return sourcePath;
}

public String getExtdir() {
return extdir;
public List<String> getExtClassPath() {
return extClassPath;
}

public String getProcessorPath() {
public List<String> getProcessorPath() {
return processorPath;
}

Expand Down
3 changes: 2 additions & 1 deletion .../buildjar/java/com/google/devtools/build/buildjar/ReducedClasspathJavaLibraryBuilder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

package com.google.devtools.build.buildjar;

import com.google.common.collect.ImmutableList;
import com.google.devtools.build.buildjar.javac.BlazeJavacResult;
import com.google.devtools.build.buildjar.javac.FormattedDiagnostic;
import com.google.devtools.build.buildjar.javac.JavacRunner;
Expand Down Expand Up @@ -41,7 +42,7 @@ BlazeJavacResult compileSources(JavaLibraryBuildRequest build, JavacRunner javac
throws IOException {
// Minimize classpath, but only if we're actually compiling some sources (some invocations of
// JavaBuilder are only building resource jars).
String compressedClasspath = build.getClassPath();
ImmutableList<String> compressedClasspath = build.getClassPath();
if (!build.getSourceFiles().isEmpty()) {
compressedClasspath =
build.getDependencyModule().computeStrictClasspath(build.getClassPath());
Expand Down
7 changes: 3 additions & 4 deletions src/java_tools/buildjar/java/com/google/devtools/build/buildjar/VanillaJavaBuilder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
import static java.nio.charset.StandardCharsets.UTF_8;
import static java.util.Locale.ENGLISH;

import com.google.common.base.Splitter;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableList.Builder;
import com.google.common.collect.Iterables;
Expand Down Expand Up @@ -244,7 +243,7 @@ private static void setLocations(OptionsParser optionsParser, StandardJavaFileMa
fileManager.setLocation(
StandardLocation.PLATFORM_CLASS_PATH,
Iterables.concat(
toFiles(optionsParser.getBootClassPath()), toFiles(optionsParser.getExtdir())));
toFiles(optionsParser.getBootClassPath()), toFiles(optionsParser.getExtClassPath())));
fileManager.setLocation(
StandardLocation.ANNOTATION_PROCESSOR_PATH, toFiles(optionsParser.getProcessorPath()));
if (optionsParser.getSourceGenDir() != null) {
Expand Down Expand Up @@ -307,12 +306,12 @@ private static void writeOutput(OptionsParser optionsParser) throws IOException
jar.execute();
}

private static ImmutableList<File> toFiles(String classPath) {
private static ImmutableList<File> toFiles(List<String> classPath) {
if (classPath == null) {
return ImmutableList.of();
}
ImmutableList.Builder<File> files = ImmutableList.builder();
for (String path : Splitter.on(File.pathSeparatorChar).split(classPath)) {
for (String path : classPath) {
files.add(new File(path));
}
return files.build();
Expand Down
Loading

0 comments on commit 431ce43

Please sign in to comment.