Skip to content

Commit

Permalink
Refactor TurbineOptions to make jarToTarget/directJars the source of
Browse files Browse the repository at this point in the history 
truth

instead of directJarsToTargets/indirectJarsToTargets.

MOE_MIGRATED_REVID=183404853
  • Loading branch information
@cushon
cushon committed Jan 26, 2018
1 parent 8b6e986 commit 3cf4e7a
Show file tree
Hide file tree
Showing 2 changed files with 51 additions and 38 deletions.
70 changes: 39 additions & 31 deletions java/com/google/turbine/options/TurbineOptions.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,7 @@
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Map;
import java.util.Set;
import javax.annotation.Nullable;

/** Header compilation options. */
Expand All @@ -41,8 +38,8 @@ public class TurbineOptions {
private final ImmutableSet<String> processors;
private final ImmutableList<String> sourceJars;
private final Optional<String> outputDeps;
private final ImmutableMap<String, String> directJarsToTargets;
private final ImmutableMap<String, String> indirectJarsToTargets;
private final ImmutableMap<String, String> jarToTarget;
private final ImmutableSet<String> directJars;
private final Optional<String> targetLabel;
private final ImmutableList<String> depsArtifacts;
private final Optional<String> ruleKind;
Expand All @@ -61,8 +58,8 @@ private TurbineOptions(
ImmutableSet<String> processors,
ImmutableList<String> sourceJars,
@Nullable String outputDeps,
ImmutableMap<String, String> directJarsToTargets,
ImmutableMap<String, String> indirectJarsToTargets,
ImmutableMap<String, String> jarToTarget,
ImmutableSet<String> directJars,
@Nullable String targetLabel,
ImmutableList<String> depsArtifacts,
@Nullable String ruleKind,
Expand All @@ -79,10 +76,8 @@ private TurbineOptions(
this.processors = checkNotNull(processors, "processors must not be null");
this.sourceJars = checkNotNull(sourceJars, "sourceJars must not be null");
this.outputDeps = Optional.fromNullable(outputDeps);
this.directJarsToTargets =
checkNotNull(directJarsToTargets, "directJarsToTargets must not be null");
this.indirectJarsToTargets =
checkNotNull(indirectJarsToTargets, "indirectJarsToTargets must not be null");
this.jarToTarget = checkNotNull(jarToTarget, "jarToTarget must not be null");
this.directJars = checkNotNull(directJars, "directJars must not be null");
this.targetLabel = Optional.fromNullable(targetLabel);
this.depsArtifacts = checkNotNull(depsArtifacts, "depsArtifacts must not be null");
this.ruleKind = Optional.fromNullable(ruleKind);
Expand Down Expand Up @@ -141,14 +136,38 @@ public Optional<String> outputDeps() {
return outputDeps;
}

/** The direct dependencies. */
public ImmutableSet<String> directJars() {
return directJars;
}

/** The mapping from the path to a dependency to its build label. */
public ImmutableMap<String, String> jarToTarget() {
return jarToTarget;
}

/** The mapping from the path to a direct dependency to its build label. */
public ImmutableMap<String, String> directJarsToTargets() {
return directJarsToTargets;
ImmutableMap.Builder<String, String> result = ImmutableMap.builder();
for (Map.Entry<String, String> entry : jarToTarget.entrySet()) {
String jar = entry.getKey();
if (directJars.contains(jar)) {
result.put(jar, entry.getValue());
}
}
return result.build();
}

/** The mapping from the path to an indirect dependency to its build label. */
public ImmutableMap<String, String> indirectJarsToTargets() {
return indirectJarsToTargets;
ImmutableMap.Builder<String, String> result = ImmutableMap.builder();
for (Map.Entry<String, String> entry : jarToTarget.entrySet()) {
String jar = entry.getKey();
if (!directJars.contains(jar)) {
result.put(jar, entry.getValue());
}
}
return result.build();
}

/** The label of the target being compiled. */
Expand Down Expand Up @@ -198,11 +217,8 @@ public static class Builder {
@Nullable private String release;
@Nullable private String system;
private String outputDeps;
private final Map<String, String> jarToTarget = new HashMap<>();
private final Set<String> directJars = new HashSet<>();
private final ImmutableMap.Builder<String, String> directJarsToTargets = ImmutableMap.builder();
private final ImmutableMap.Builder<String, String> indirectJarsToTargets =
ImmutableMap.builder();
private final ImmutableMap.Builder<String, String> jarToTarget = ImmutableMap.builder();
private final ImmutableSet.Builder<String> directJars = ImmutableSet.builder();
@Nullable private String targetLabel;
private final ImmutableList.Builder<String> depsArtifacts = ImmutableList.builder();
@Nullable private String ruleKind;
Expand All @@ -211,15 +227,6 @@ public static class Builder {
private boolean shouldReduceClassPath = true;

public TurbineOptions build() {
for (Map.Entry<String, String> entry : jarToTarget.entrySet()) {
String jar = entry.getKey();
String target = entry.getValue();
if (directJars.contains(jar)) {
directJarsToTargets.put(jar, target);
} else {
indirectJarsToTargets.put(jar, target);
}
}
return new TurbineOptions(
output,
classPath.build(),
Expand All @@ -231,8 +238,8 @@ public TurbineOptions build() {
processors.build(),
sourceJars.build(),
outputDeps,
directJarsToTargets.build(),
indirectJarsToTargets.build(),
jarToTarget.build(),
directJars.build(),
targetLabel,
depsArtifacts.build(),
ruleKind,
Expand Down Expand Up @@ -298,13 +305,14 @@ public Builder setOutputDeps(String outputDeps) {

// TODO(b/72379900): Remove this
public Builder addDirectJarToTarget(String jar, String target) {
directJarsToTargets.put(jar, target);
directJars.add(jar);
jarToTarget.put(jar, target);
return this;
}

// TODO(b/72379900): Remove this
public Builder addIndirectJarToTarget(String jar, String target) {
indirectJarsToTargets.put(jar, target);
jarToTarget.put(jar, target);
return this;
}

Expand Down
19 changes: 12 additions & 7 deletions javatests/com/google/turbine/options/TurbineOptionsTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ public void strictJavaDepsArgs() throws Exception {
"blaze-out/foo/libbaz2.jar",
"//foo/baz2",
"blaze-out/proto/libproto.jar",
"//proto",
"//proto;java_proto_library",
"--direct_dependencies",
"blaze-out/foo/libbar.jar",
"--deps_artifacts",
Expand All @@ -136,15 +136,20 @@ public void strictJavaDepsArgs() throws Exception {
ImmutableMap.of(
"blaze-out/foo/libbaz1.jar", "//foo/baz1",
"blaze-out/foo/libbaz2.jar", "//foo/baz2",
"blaze-out/proto/libproto.jar", "//proto"));
"blaze-out/proto/libproto.jar", "//proto;java_proto_library"));
assertThat(options.jarToTarget())
.containsExactlyEntriesIn(
ImmutableMap.of(
"blaze-out/foo/libbar.jar", "//foo/bar",
"blaze-out/foo/libbaz1.jar", "//foo/baz1",
"blaze-out/foo/libbaz2.jar", "//foo/baz2",
"blaze-out/proto/libproto.jar", "//proto;java_proto_library"));
assertThat(options.directJars()).containsExactly("blaze-out/foo/libbar.jar");
assertThat(options.depsArtifacts()).containsExactly("foo.jdeps", "bar.jdeps");
}

/**
* Makes sure turbine accepts old-style arguments.
*
* <p>TODO(b/72379900): Remove this.
*/
/** Makes sure turbine accepts old-style arguments. */
// TODO(b/72379900): Remove this.
@Test
public void testLegacyStrictJavaDepsArgs() throws Exception {
String[] lines = {
Expand Down

0 comments on commit 3cf4e7a

Please sign in to comment.