Skip to content

Commit

Permalink
Remove ExternalJavac.actualPath
Browse files Browse the repository at this point in the history
Summary:
This is an Either<PathSourcePath, SourcePath> where, when the right
side is a PathSourcePath we just have the same behavior as when it's the left
side... That's silly.

Reviewed By: styurin

fbshipit-source-id: 14ad75a
  • Loading branch information
cjhopman authored and facebook-github-bot committed Jun 6, 2018
1 parent 72dc3fc commit b6e91b2
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 25 deletions.
12 changes: 1 addition & 11 deletions src/com/facebook/buck/jvm/java/ExternalJavac.java
Original file line number Diff line number Diff line change
Expand Up @@ -58,14 +58,12 @@
/** javac implemented in a separate binary. */
public class ExternalJavac implements Javac {
@AddToRuleKey private final Supplier<Tool> javac;
private final Either<PathSourcePath, BuildTargetSourcePath> actualPath;
private final String shortName;

public ExternalJavac(final Either<PathSourcePath, SourcePath> pathToJavac) {
if (pathToJavac.isRight() && pathToJavac.getRight() instanceof BuildTargetSourcePath) {
BuildTargetSourcePath buildTargetPath = (BuildTargetSourcePath) pathToJavac.getRight();
this.shortName = buildTargetPath.getTarget().toString();
this.actualPath = Either.ofRight(buildTargetPath);
this.javac =
MoreSuppliers.memoize(
() ->
Expand All @@ -89,7 +87,6 @@ public ImmutableMap<String, String> getEnvironment(
} else {
PathSourcePath actualPath =
pathToJavac.transform(path -> path, path -> (PathSourcePath) path);
this.actualPath = Either.ofLeft(actualPath);
this.shortName = actualPath.toString();
this.javac =
MoreSuppliers.memoize(
Expand Down Expand Up @@ -117,11 +114,6 @@ public ImmutableMap<String, String> getEnvironment(
}
}

@VisibleForTesting
Either<PathSourcePath, BuildTargetSourcePath> getActualPath() {
return actualPath;
}

@Override
public ImmutableList<String> getCommandPrefix(SourcePathResolver resolver) {
return javac.get().getCommandPrefix(resolver);
Expand Down Expand Up @@ -198,9 +190,7 @@ public int buildClasses() throws InterruptedException {
abiGenerationMode == AbiGenerationMode.CLASS,
"Cannot compile ABI jars with external javac");
ImmutableList.Builder<String> command = ImmutableList.builder();
command.add(
actualPath.transform(
Object::toString, path -> sourcePathResolver.getAbsolutePath(path).toString()));
command.addAll(javac.get().getCommandPrefix(sourcePathResolver));
ImmutableList<Path> expandedSources;
try {
expandedSources =
Expand Down
37 changes: 23 additions & 14 deletions test/com/facebook/buck/jvm/java/JavacSpecTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -30,26 +30,35 @@
import com.facebook.buck.core.sourcepath.DefaultBuildTargetSourcePath;
import com.facebook.buck.core.sourcepath.PathSourcePath;
import com.facebook.buck.core.sourcepath.SourcePath;
import com.facebook.buck.core.sourcepath.resolver.SourcePathResolver;
import com.facebook.buck.core.sourcepath.resolver.impl.DefaultSourcePathResolver;
import com.facebook.buck.model.BuildTargetFactory;
import com.facebook.buck.rules.FakeSourcePath;
import com.facebook.buck.testutil.FakeProjectFilesystem;
import com.facebook.buck.testutil.TemporaryPaths;
import com.facebook.buck.util.types.Either;
import com.google.common.collect.ImmutableList;
import java.io.IOException;
import java.nio.file.Path;
import java.nio.file.Paths;
import org.hamcrest.Matchers;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;

public class JavacSpecTest {
private ActionGraphBuilder graphBuilder;
private SourcePathResolver sourcePathResolver;
private SourcePathRuleFinder ruleFinder;
private JavacSpec.Builder specBuilder;

@Rule public TemporaryPaths tmp = new TemporaryPaths();

@Before
public void setUp() {
graphBuilder = new TestActionGraphBuilder();
ruleFinder = new SourcePathRuleFinder(graphBuilder);
sourcePathResolver = DefaultSourcePathResolver.from(ruleFinder);
specBuilder = JavacSpec.builder();
}

Expand All @@ -70,33 +79,33 @@ public void returnsBuiltInJavacWhenCompilerArgHasDefault() {
}

@Test
public void returnsExternalCompilerIfJavacPathPresent() {
Path externalPath = Paths.get("/foo/bar/path/to/javac");
SourcePath javacPath = FakeSourcePath.of(externalPath);
public void returnsExternalCompilerIfJavacPathPresent() throws IOException {
Path externalPath = tmp.newExecutableFile();

SourcePath javacPath = FakeSourcePath.of(externalPath);
specBuilder.setJavacPath(Either.ofRight(javacPath));
ExternalJavac javac = (ExternalJavac) getJavac();
assertTrue(javac.getActualPath().isLeft());
assertEquals(javacPath, javac.getActualPath().getLeft());

assertEquals(
ImmutableList.of(externalPath.toString()), javac.getCommandPrefix(sourcePathResolver));
}

@Test
public void returnsExternalCompilerIfCompilerArgHasPath() {
Path externalJavac = Paths.get("/foo/bar/javac.exe").toAbsolutePath();
public void returnsExternalCompilerIfCompilerArgHasPath() throws IOException {
Path externalJavac = tmp.newExecutableFile();
SourcePath sourcePath = FakeSourcePath.of(externalJavac.toString());
Either<BuiltInJavac, SourcePath> either = Either.ofRight(sourcePath);

specBuilder.setCompiler(either);
ExternalJavac javac = (ExternalJavac) getJavac();

assertTrue(javac.getActualPath().isLeft());
PathSourcePath actualPath = javac.getActualPath().getLeft();
assertEquals(sourcePath, actualPath);
assertEquals(
ImmutableList.of(externalJavac.toString()), javac.getCommandPrefix(sourcePathResolver));
}

@Test
public void compilerArgTakesPrecedenceOverJavacPathArg() {
Path externalJavacPath = Paths.get("/foo/bar/javac.exe").toAbsolutePath();
public void compilerArgTakesPrecedenceOverJavacPathArg() throws IOException {
Path externalJavacPath = tmp.newExecutableFile();
SourcePath sourcePath = FakeSourcePath.of(externalJavacPath.toString());
Either<BuiltInJavac, SourcePath> either = Either.ofRight(sourcePath);

Expand All @@ -105,8 +114,8 @@ public void compilerArgTakesPrecedenceOverJavacPathArg() {
.setJavacPath(Either.ofLeft(FakeSourcePath.of("does-not-exist")));
ExternalJavac javac = (ExternalJavac) getJavac();

assertTrue(javac.getActualPath().isLeft());
assertEquals(sourcePath, javac.getActualPath().getLeft());
assertEquals(
ImmutableList.of(externalJavacPath.toString()), javac.getCommandPrefix(sourcePathResolver));
}

@Test
Expand Down

0 comments on commit b6e91b2

Please sign in to comment.