From 91c8f20b054d8bc3c3a860197fab69167218af67 Mon Sep 17 00:00:00 2001 From: tomlu Date: Wed, 24 Jan 2018 15:49:26 -0800 Subject: Change JavaBuilder and Turbine command lines to not require CustomMultiArgv. Instead of passing: --direct_dependency jar1 jarowner1 aspect1 --indirect_dependency jar2 jarowner2 aspect2 --classpath jar1 jar2 we pass --dependencies jar1 jarowner1;aspect1 jar2 jarowner2:aspect2 --direct_dependencies jar1 This formats doesn't need to inspect each item in the incoming nested set, and thus doesn't need CustomMultiArgv. This change needs to be rolled out in phases, so this CL changes JavaBuilder and Turbine to accept either format. RELNOTES: None PiperOrigin-RevId: 183155036 --- .../devtools/build/buildjar/OptionsParser.java | 83 +++++++++++++++++++++- .../build/java/turbine/javac/JavacTurbine.java | 17 ++--- 2 files changed, 86 insertions(+), 14 deletions(-) (limited to 'src/java_tools/buildjar') 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 551c4f1d36..920a22baec 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 @@ -45,6 +45,8 @@ public final class OptionsParser { private static final Splitter SPACE_SPLITTER = Splitter.on(' '); private final List javacOpts = new ArrayList<>(); + private final Map jarsToTargets = new HashMap<>(); + private final Set directJars = new HashSet<>(); private final Map directJarsToTargets = new HashMap<>(); private final Map indirectJarsToTargets = new HashMap<>(); @@ -103,24 +105,39 @@ public final class OptionsParser { * @throws InvalidCommandLineException on an invalid option being passed. */ private void processCommandlineArgs(Deque argQueue) throws InvalidCommandLineException { + boolean foundNewDependencyArgument = false; + boolean foundLegacyDependencyArgument = false; + for (String arg = argQueue.pollFirst(); arg != null; arg = argQueue.pollFirst()) { switch (arg) { case "--javacopts": readJavacopts(javacOpts, argQueue); sourcePathFromJavacOpts(); break; + case "--dependencies": + collectDependencies(classPath, jarsToTargets, argQueue, arg, "--"); + foundNewDependencyArgument = true; + break; + case "--direct_dependencies": + collectFlagArguments(directJars, argQueue, "--"); + foundNewDependencyArgument = true; + break; case "--direct_dependency": { + // TODO(b/72379900): Remove this String jar = getArgument(argQueue, arg); - JarOwner owner = parseJarOwner(getArgument(argQueue, arg)); + JarOwner owner = parseJarOwnerLegacy(getArgument(argQueue, arg)); directJarsToTargets.put(jar, owner); + foundLegacyDependencyArgument = true; break; } case "--indirect_dependency": { + // TODO(b/72379900): Remove this String jar = getArgument(argQueue, arg); - JarOwner owner = parseJarOwner(getArgument(argQueue, arg)); + JarOwner owner = parseJarOwnerLegacy(getArgument(argQueue, arg)); indirectJarsToTargets.put(jar, owner); + foundLegacyDependencyArgument = true; break; } case "--strict_java_deps": @@ -154,7 +171,9 @@ public final class OptionsParser { collectFlagArguments(sourceJars, argQueue, "-"); break; case "--classpath": + // TODO(b/72379900): Remove this collectFlagArguments(classPath, argQueue, "-"); + foundLegacyDependencyArgument = true; break; case "--sourcepath": // TODO(#970): Consider whether we want to use --sourcepath for resolving of #970. @@ -208,6 +227,24 @@ public final class OptionsParser { throw new InvalidCommandLineException("unknown option : '" + arg + "'"); } } + // TODO(b/72379900): Remove the legacy part of this code + if (foundLegacyDependencyArgument && foundNewDependencyArgument) { + throw new InvalidCommandLineException( + "Found both new-style and old-style dependency arguments: " + + "Cannot use arguments from both " + + "(--dependencies, --direct_dependencies) and " + + "(--direct_dependency, --indirect_dependency, --classpath) " + + "at the same time."); + } + for (Map.Entry entry : jarsToTargets.entrySet()) { + String jar = entry.getKey(); + JarOwner jarOwner = entry.getValue(); + if (directJars.contains(jar)) { + directJarsToTargets.put(jar, jarOwner); + } else { + indirectJarsToTargets.put(jar, jarOwner); + } + } } private void sourcePathFromJavacOpts() { @@ -222,7 +259,47 @@ public final class OptionsParser { } } - private JarOwner parseJarOwner(String line) { + private static void collectDependencies( + Collection classPath, + Map jarsToTargets, + Deque args, + String arg, + String terminatorPrefix) + throws InvalidCommandLineException { + while (true) { + String nextArg = args.pollFirst(); + if (nextArg == null) { + break; + } + if (nextArg.startsWith(terminatorPrefix)) { + args.addFirst(nextArg); + break; + } + String jar = nextArg; + JarOwner jarOwner; + try { + jarOwner = parseJarOwner(args.remove()); + } catch (NoSuchElementException e) { + throw new InvalidCommandLineException(arg + ": missing argument"); + } + classPath.add(jar); + jarsToTargets.put(jar, jarOwner); + } + } + + private static JarOwner parseJarOwner(String line) { + int separatorIndex = line.indexOf(';'); + final JarOwner owner; + if (separatorIndex == -1) { + owner = JarOwner.create(line); + } else { + owner = + JarOwner.create(line.substring(0, separatorIndex), line.substring(separatorIndex + 1)); + } + return owner; + } + + private JarOwner parseJarOwnerLegacy(String line) { List ownerStringParts = SPACE_SPLITTER.splitToList(line); JarOwner owner; Preconditions.checkState(ownerStringParts.size() == 1 || ownerStringParts.size() == 2); diff --git a/src/java_tools/buildjar/java/com/google/devtools/build/java/turbine/javac/JavacTurbine.java b/src/java_tools/buildjar/java/com/google/devtools/build/java/turbine/javac/JavacTurbine.java index 37d1ef90a7..2580545034 100644 --- a/src/java_tools/buildjar/java/com/google/devtools/build/java/turbine/javac/JavacTurbine.java +++ b/src/java_tools/buildjar/java/com/google/devtools/build/java/turbine/javac/JavacTurbine.java @@ -18,8 +18,6 @@ import static java.nio.charset.StandardCharsets.UTF_8; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Joiner; -import com.google.common.base.Preconditions; -import com.google.common.base.Splitter; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; @@ -48,7 +46,6 @@ import java.nio.file.SimpleFileVisitor; import java.nio.file.attribute.BasicFileAttributes; import java.util.Arrays; import java.util.HashMap; -import java.util.List; import java.util.Map; import java.util.regex.Pattern; import java.util.zip.ZipOutputStream; @@ -67,8 +64,6 @@ import org.objectweb.asm.Opcodes; */ public class JavacTurbine implements AutoCloseable { - private static final Splitter SPACE_SPLITTER = Splitter.on(' '); - public static void main(String[] args) throws IOException { System.exit(compile(TurbineOptionsParser.parse(Arrays.asList(args))).exitCode()); } @@ -284,13 +279,13 @@ public class JavacTurbine implements AutoCloseable { } private static JarOwner parseJarOwner(String line) { - List ownerStringParts = SPACE_SPLITTER.splitToList(line); - JarOwner owner; - Preconditions.checkState(ownerStringParts.size() == 1 || ownerStringParts.size() == 2); - if (ownerStringParts.size() == 1) { - owner = JarOwner.create(ownerStringParts.get(0)); + int separatorIndex = line.indexOf(';'); + final JarOwner owner; + if (separatorIndex == -1) { + owner = JarOwner.create(line); } else { - owner = JarOwner.create(ownerStringParts.get(0), ownerStringParts.get(1)); + owner = + JarOwner.create(line.substring(0, separatorIndex), line.substring(separatorIndex + 1)); } return owner; } -- cgit v1.2.3