The Android SDK should not be required when building a pure Java proj…

…ect. Summary: Instead of throwing `NoAndroidSdkException` as part of the buildup to create an `ExecutionContext`, only throw `NoAndroidSdkException` when `ExecutionContext.getAndroidPlatformTarget()` is invoked and the Android SDK has not been specified by the user. This way, a `Build` can create an `ExecutionContext` and run its course without the creation of an `AndroidPlatformTarget` as a requirement. This will make it easier to use Buck for a non-Android Java project. Also changed `NoAndroidSdkException` to be an unchecked exception, as it is not recoverable. This helps eliminate a number of catch blocks in the code. See facebook#7. Test Plan: As follows: unset ANDROID_HOME unset ANDROID_SDK rm -rf local.properties buck build buck
aulanov · May 2, 2013 · ef014b3 · ef014b3
1 parent 2b242c8
commit ef014b3
Showing 21 changed files with 86 additions and 98 deletions.
diff --git a/src/com/facebook/buck/cli/AbstractCommandOptions.java b/src/com/facebook/buck/cli/AbstractCommandOptions.java
@@ -21,7 +21,6 @@
 import com.facebook.buck.shell.ExecutionContext;
 import com.facebook.buck.shell.Verbosity;
 import com.facebook.buck.util.AndroidPlatformTarget;
-import com.facebook.buck.util.NoAndroidSdkException;
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Optional;
 import com.google.common.base.Preconditions;
@@ -85,14 +84,15 @@ public Verbosity getVerbosity() {
   }
 
   /** @return androidSdkDir */
-  protected File findAndroidSdkDir() throws NoAndroidSdkException {
+  protected Optional<File> findAndroidSdkDir() {
     Optional<File> androidSdkDir = findDirectoryByPropertiesThenEnvironmentVariable(
         "sdk.dir", "ANDROID_SDK", "ANDROID_HOME");
-    if (!androidSdkDir.isPresent()) {
-      throw new NoAndroidSdkException();
-    } else {
-      return androidSdkDir.get();
+    if (androidSdkDir.isPresent()) {
+      Preconditions.checkArgument(androidSdkDir.get().isDirectory(),
+          "The location of your Android SDK %s must be a directory",
+          androidSdkDir.get());
     }
+    return androidSdkDir;
   }
 
   /** @return androidNdkDir */
@@ -149,14 +149,11 @@ private Optional<File> findDirectoryByPropertiesThenEnvironmentVariable(
   protected ExecutionContext createExecutionContext(
       AbstractCommandRunner<?> commandRunner,
       File projectDirectoryRoot,
-      DependencyGraph dependencyGraph) throws NoAndroidSdkException {
-    Optional<AndroidPlatformTarget> androidPlatformTarget = Build.findAndroidPlatformTarget(
-        dependencyGraph, findAndroidSdkDir(), commandRunner.stdErr);
-
+      DependencyGraph dependencyGraph) {
     return new ExecutionContext(
         getVerbosity(),
         projectDirectoryRoot,
-        androidPlatformTarget,
+        findAndroidPlatformTarget(dependencyGraph, commandRunner.stdErr),
         findAndroidNdkDir(),
         commandRunner.ansi,
         false /* isCodeCoverageEnabled */,
@@ -166,7 +163,7 @@ protected ExecutionContext createExecutionContext(
   }
 
   protected Optional<AndroidPlatformTarget> findAndroidPlatformTarget(
-      DependencyGraph dependencyGraph, PrintStream stdErr) throws NoAndroidSdkException {
+      DependencyGraph dependencyGraph, PrintStream stdErr) {
     return Build.findAndroidPlatformTarget(dependencyGraph, findAndroidSdkDir(), stdErr);
   }
 }
diff --git a/src/com/facebook/buck/cli/BuildCommand.java b/src/com/facebook/buck/cli/BuildCommand.java
@@ -28,7 +28,6 @@
 import com.facebook.buck.shell.Verbosity;
 import com.facebook.buck.util.ExceptionWithHumanReadableMessage;
 import com.facebook.buck.util.HumanReadableException;
-import com.facebook.buck.util.NoAndroidSdkException;
 import com.facebook.buck.util.ProjectFilesystem;
 import com.google.common.base.Joiner;
 import com.google.common.base.Preconditions;
@@ -108,14 +107,9 @@ private int runCommandWithOptionsWithTracerRunning(BuildCommandOptions options)
     }
 
     // Create and execute the build.
-    try {
-      this.build = options.createBuild(dependencyGraph,
-          getProjectFilesystem().getProjectRoot(),
-          console);
-    } catch (NoAndroidSdkException e) {
-      console.printFailureWithoutStacktrace(e);
-      return 1;
-    }
+    this.build = options.createBuild(dependencyGraph,
+        getProjectFilesystem().getProjectRoot(),
+        console);
     stdErr.printf("BUILDING %s\n", Joiner.on(' ').join(buildTargets));
     int exitCode = executeBuildAndPrintAnyFailuresToConsole(build, console);
 

diff --git a/src/com/facebook/buck/cli/BuildCommandOptions.java b/src/com/facebook/buck/cli/BuildCommandOptions.java
@@ -19,7 +19,6 @@
 import com.facebook.buck.command.Build;
 import com.facebook.buck.rules.DependencyGraph;
 import com.facebook.buck.util.HumanReadableException;
-import com.facebook.buck.util.NoAndroidSdkException;
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.Lists;
@@ -96,14 +95,9 @@ public ListeningExecutorService getListeningExecutorService() {
     return listeningExecutorService;
   }
 
-  Build createBuild(DependencyGraph graph,
-      File projectDirectoryRoot,
-      Console console) throws NoAndroidSdkException {
-    // First determine the android SDK dir.
-    File androidSdkDir = findAndroidSdkDir();
-
-    Build build = new Build(graph,
-        androidSdkDir,
+  Build createBuild(DependencyGraph graph, File projectDirectoryRoot, Console console) {
+    return new Build(graph,
+        findAndroidSdkDir(),
         findAndroidNdkDir(),
         projectDirectoryRoot,
         getVerbosity(),
@@ -114,6 +108,5 @@ Build createBuild(DependencyGraph graph,
         console.getStdErr(),
         isCodeCoverageEnabled(),
         isDebugEnabled());
-    return build;
   }
 }
diff --git a/src/com/facebook/buck/cli/ProjectCommand.java b/src/com/facebook/buck/cli/ProjectCommand.java
@@ -23,9 +23,6 @@
 import com.facebook.buck.parser.RawRulePredicate;
 import com.facebook.buck.rules.BuildRuleType;
 import com.facebook.buck.shell.ExecutionContext;
-import com.facebook.buck.util.AndroidPlatformTarget;
-import com.facebook.buck.util.NoAndroidSdkException;
-import com.google.common.base.Optional;
 import com.google.common.io.Files;
 
 import java.io.File;
@@ -68,19 +65,10 @@ int runCommandWithOptions(ProjectCommandOptions options) throws IOException {
       return 1;
     }
 
-    Optional<AndroidPlatformTarget> androidPlatformTarget;
-    try {
-      androidPlatformTarget = options.findAndroidPlatformTarget(
-          partialGraph.getDependencyGraph(), stdErr);
-    } catch (NoAndroidSdkException e) {
-      console.printFailureWithoutStacktrace(e);
-      return 1;
-    }
-
     ExecutionContext executionContext = new ExecutionContext(
         options.getVerbosity(),
         getProjectFilesystem().getProjectRoot(),
-        androidPlatformTarget,
+        options.findAndroidPlatformTarget(partialGraph.getDependencyGraph(), stdErr),
         options.findAndroidNdkDir(),
         ansi,
         false /* isCodeCoverageEnabled */,

diff --git a/src/com/facebook/buck/cli/TestCommand.java b/src/com/facebook/buck/cli/TestCommand.java
@@ -39,7 +39,6 @@
 import com.facebook.buck.shell.MakeCleanDirectoryCommand;
 import com.facebook.buck.util.Escaper;
 import com.facebook.buck.util.HumanReadableException;
-import com.facebook.buck.util.NoAndroidSdkException;
 import com.facebook.buck.util.ProjectFilesystem;
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Charsets;
@@ -254,13 +253,7 @@ public boolean isMatch(
     testRules = filterTestRules(options, testRules);
 
     // Build all of the test rules.
-    Build build;
-    try {
-      build = options.createBuild(graph, getProjectFilesystem().getProjectRoot(), console);
-    } catch (NoAndroidSdkException e) {
-      console.printFailureWithoutStacktrace(e);
-      return 1;
-    }
+    Build build = options.createBuild(graph, getProjectFilesystem().getProjectRoot(), console);
     int exitCode = BuildCommand.executeBuildAndPrintAnyFailuresToConsole(build, console);
     if (exitCode != 0) {
       return exitCode;

diff --git a/src/com/facebook/buck/cli/UninstallCommand.java b/src/com/facebook/buck/cli/UninstallCommand.java
@@ -27,7 +27,6 @@
 import com.facebook.buck.rules.InstallableBuildRule;
 import com.facebook.buck.shell.ExecutionContext;
 import com.facebook.buck.shell.Verbosity;
-import com.facebook.buck.util.NoAndroidSdkException;
 import com.google.common.collect.ImmutableList;
 
 import java.io.IOException;
@@ -83,13 +82,10 @@ int runCommandWithOptions(UninstallCommandOptions options) throws IOException {
     InstallableBuildRule installableBuildRule = (InstallableBuildRule)buildRule;
 
     // We need this in case adb isn't already running.
-    ExecutionContext context;
-    try{
-      context = options.createExecutionContext(this, getProjectFilesystem().getProjectRoot(),
-          dependencyGraph);
-    } catch (NoAndroidSdkException e) {
-      context = null;
-    }
+    ExecutionContext context = options.createExecutionContext(
+        this,
+        getProjectFilesystem().getProjectRoot(),
+        dependencyGraph);
 
     // Find application package name from manifest and uninstall from matching devices.
     String appId = tryToExtractPackageNameFromManifest(installableBuildRule);

diff --git a/src/com/facebook/buck/command/Build.java b/src/com/facebook/buck/command/Build.java
@@ -67,7 +67,7 @@ public class Build {
    */
   public Build(
       DependencyGraph dependencyGraph,
-      File androidSdkDir,
+      Optional<File> androidSdkDir,
       Optional<File> ndkRoot,
       File projectDirectoryRoot,
       Verbosity verbosity,
@@ -78,9 +78,6 @@ public Build(
       PrintStream stdErr,
       boolean isCodeCoverageEnabled,
       boolean isDebugEnabled) {
-    Preconditions.checkNotNull(androidSdkDir);
-    Preconditions.checkArgument(androidSdkDir.exists());
-    Preconditions.checkArgument(androidSdkDir.isDirectory());
     this.dependencyGraph = Preconditions.checkNotNull(dependencyGraph);
 
     Optional<AndroidPlatformTarget> androidPlatformTarget = findAndroidPlatformTarget(
@@ -118,7 +115,18 @@ public BuildContext getBuildContext() {
   }
 
   public static Optional<AndroidPlatformTarget> findAndroidPlatformTarget(
-      DependencyGraph dependencyGraph, final File androidSdkDir, final PrintStream stdErr) {
+      DependencyGraph dependencyGraph, Optional<File> androidSdkDirOption, PrintStream stdErr) {
+    if (androidSdkDirOption.isPresent()) {
+      File androidSdkDir = androidSdkDirOption.get();
+      return findAndroidPlatformTarget(dependencyGraph, androidSdkDir, stdErr);
+    } else {
+      // If the Android SDK has not been specified, then no AndroidPlatformTarget can be found.
+      return Optional.<AndroidPlatformTarget>absent();
+    }
+  }
+
+  private static Optional<AndroidPlatformTarget> findAndroidPlatformTarget(
+      final DependencyGraph dependencyGraph, final File androidSdkDir, final PrintStream stdErr) {
     // Traverse the dependency graph to determine androidPlatformTarget.
     AbstractBottomUpTraversal<BuildRule, Optional<AndroidPlatformTarget>> traversal =
         new AbstractBottomUpTraversal<BuildRule, Optional<AndroidPlatformTarget>>(dependencyGraph) {
@@ -195,7 +203,8 @@ public ListenableFuture<List<BuildRuleSuccess>> executeBuild(EventBus events)
         .setProjectFilesystem(projectFilesystem)
         .setJavaPackageFinder(javaPackageFinder)
         .setEventBus(events)
-        .setAndroidBootclasspathForAndroidPlatformTarget(executionContext.getAndroidPlatformTarget())
+        .setAndroidBootclasspathForAndroidPlatformTarget(
+            executionContext.getAndroidPlatformTargetOptional())
         .build();
 
     return Builder.getInstance().buildRules(rulesToBuild, buildContext);

diff --git a/src/com/facebook/buck/rules/BuildContext.java b/src/com/facebook/buck/rules/BuildContext.java
@@ -18,7 +18,6 @@
 
 import com.facebook.buck.shell.CommandRunner;
 import com.facebook.buck.util.AndroidPlatformTarget;
-import com.facebook.buck.util.HumanReadableException;
 import com.facebook.buck.util.NoAndroidSdkException;
 import com.facebook.buck.util.ProjectFilesystem;
 import com.google.common.base.Joiner;
@@ -174,7 +173,7 @@ private void setDefaultAndroidBootclasspathSupplier() {
       this.androidBootclasspathSupplier = new Supplier<String>() {
         @Override
         public String get() {
-          throw new HumanReadableException(new NoAndroidSdkException());
+          throw new NoAndroidSdkException();
         }
       };
     }

diff --git a/src/com/facebook/buck/shell/AaptCommand.java b/src/com/facebook/buck/shell/AaptCommand.java
@@ -54,7 +54,7 @@ public AaptCommand(
   protected ImmutableList<String> getShellCommandInternal(ExecutionContext context) {
     ImmutableList.Builder<String> builder = ImmutableList.builder();
 
-    AndroidPlatformTarget androidPlatformTarget = context.getAndroidPlatformTarget().get();
+    AndroidPlatformTarget androidPlatformTarget = context.getAndroidPlatformTarget();
     builder.add(androidPlatformTarget.getAaptExecutable().getAbsolutePath(), "package");
 
     // verbose flag, if appropriate.

diff --git a/src/com/facebook/buck/shell/AidlCommand.java b/src/com/facebook/buck/shell/AidlCommand.java
@@ -44,7 +44,7 @@ protected ImmutableList<String> getShellCommandInternal(ExecutionContext context
 
     // The arguments passed to aidl are based off of what I observed when running Ant in verbose
     // mode.
-    AndroidPlatformTarget androidPlatformTarget = context.getAndroidPlatformTarget().get();
+    AndroidPlatformTarget androidPlatformTarget = context.getAndroidPlatformTarget();
     args.add(androidPlatformTarget.getAidlExecutable().getAbsolutePath());
 
     // For some reason, all of the flags to aidl do not permit a space between the flag name and

diff --git a/src/com/facebook/buck/shell/DxCommand.java b/src/com/facebook/buck/shell/DxCommand.java
@@ -42,7 +42,7 @@ public DxCommand(String outputDexFile, Iterable<String> filesToDex) {
   protected ImmutableList<String> getShellCommandInternal(ExecutionContext context) {
     ImmutableList.Builder<String> builder = ImmutableList.builder();
 
-    AndroidPlatformTarget androidPlatformTarget = context.getAndroidPlatformTarget().get();
+    AndroidPlatformTarget androidPlatformTarget = context.getAndroidPlatformTarget();
     builder.add(androidPlatformTarget.getDxExecutable().getAbsolutePath());
     builder.add("--dex");
 

diff --git a/src/com/facebook/buck/shell/ExecutionContext.java b/src/com/facebook/buck/shell/ExecutionContext.java
@@ -18,6 +18,7 @@
 
 import com.facebook.buck.util.AndroidPlatformTarget;
 import com.facebook.buck.util.Ansi;
+import com.facebook.buck.util.NoAndroidSdkException;
 import com.facebook.buck.util.ProcessExecutor;
 import com.google.common.base.Optional;
 import com.google.common.base.Preconditions;
@@ -80,16 +81,39 @@ public Ansi getAnsi() {
     return ansi;
   }
 
-  public Optional<AndroidPlatformTarget> getAndroidPlatformTarget() {
+  /**
+   * Returns the {@link AndroidPlatformTarget}, if present. If not, throws a
+   * {@link NoAndroidSdkException}. Use this when your logic requires the user to specify the
+   * location of an Android SDK. A user who is building a "pure Java" (i.e., not Android) project
+   * using Buck should never have to exercise this code path.
+   * <p>
+   * If the location of an Android SDK is optional, then use
+   * {@link #getAndroidPlatformTargetOptional()}.
+   * @throws NoAndroidSdkException if no AndroidPlatformTarget is available
+   */
+  public AndroidPlatformTarget getAndroidPlatformTarget() throws NoAndroidSdkException {
+    if (androidPlatformTarget.isPresent()) {
+      return androidPlatformTarget.get();
+    } else {
+      throw new NoAndroidSdkException();
+    }
+  }
+
+  /**
+   * Returns an {@link AndroidPlatformTarget} if the user specified one via {@code local.properties}
+   * or some other mechanism. If the user failed to specify one, {@link Optional#absent()} will be
+   * returned.
+   */
+  public Optional<AndroidPlatformTarget> getAndroidPlatformTargetOptional() {
     return androidPlatformTarget;
   }
 
   public Optional<File> getNdkRoot() {
     return ndkRoot;
   }
 
-  public String getPathToAdbExecutable() {
-    return androidPlatformTarget.get().getAdbExecutable().getAbsolutePath();
+  public String getPathToAdbExecutable() throws NoAndroidSdkException {
+    return getAndroidPlatformTarget().getAdbExecutable().getAbsolutePath();
   }
 
   public ProcessExecutor getProcessExecutor() {

diff --git a/src/com/facebook/buck/shell/GenProGuardConfigCommand.java b/src/com/facebook/buck/shell/GenProGuardConfigCommand.java
@@ -47,7 +47,7 @@ public String getShortName(ExecutionContext context) {
   @Override
   protected ImmutableList<String> getShellCommandInternal(ExecutionContext context) {
     ImmutableList.Builder<String> args = ImmutableList.builder();
-    AndroidPlatformTarget androidPlatformTarget = context.getAndroidPlatformTarget().get();
+    AndroidPlatformTarget androidPlatformTarget = context.getAndroidPlatformTarget();
 
     args.add(androidPlatformTarget.getAaptExecutable().getAbsolutePath()).add("package");
 

diff --git a/src/com/facebook/buck/shell/GenRDotJavaCommand.java b/src/com/facebook/buck/shell/GenRDotJavaCommand.java
@@ -98,7 +98,7 @@ public GenRDotJavaCommand(
   @Override
   protected ImmutableList<String> getShellCommandInternal(ExecutionContext context) {
     ImmutableList.Builder<String> builder = ImmutableList.builder();
-    AndroidPlatformTarget androidPlatformTarget = context.getAndroidPlatformTarget().get();
+    AndroidPlatformTarget androidPlatformTarget = context.getAndroidPlatformTarget();
 
     builder.add(androidPlatformTarget.getAaptExecutable().getAbsolutePath()).add("package");
 

diff --git a/src/com/facebook/buck/shell/JUnitCommand.java b/src/com/facebook/buck/shell/JUnitCommand.java
@@ -144,8 +144,8 @@ protected ImmutableList<String> getShellCommandInternal(ExecutionContext context
     }
 
     // Next, add the bootclasspath entries specific to the Android platform being targeted.
-    if (context.getAndroidPlatformTarget().isPresent()) {
-      AndroidPlatformTarget androidPlatformTarget = context.getAndroidPlatformTarget().get();
+    if (context.getAndroidPlatformTargetOptional().isPresent()) {
+      AndroidPlatformTarget androidPlatformTarget = context.getAndroidPlatformTarget();
       for (File bootclasspathEntry : androidPlatformTarget.getBootclasspathEntries()) {
         classpath.add(bootclasspathEntry.getAbsolutePath());
       }

diff --git a/src/com/facebook/buck/shell/ProGuardObfuscateCommand.java b/src/com/facebook/buck/shell/ProGuardObfuscateCommand.java
@@ -84,7 +84,7 @@ public String getShortName(ExecutionContext context) {
   @Override
   protected ImmutableList<String> getShellCommandInternal(ExecutionContext context) {
     ImmutableList.Builder<String> args = ImmutableList.builder();
-    AndroidPlatformTarget androidPlatformTarget = context.getAndroidPlatformTarget().get();
+    AndroidPlatformTarget androidPlatformTarget = context.getAndroidPlatformTarget();
     Joiner pathJoiner = Joiner.on(':');
 
     // Run ProGuard as a standalone executable JAR file.

diff --git a/src/com/facebook/buck/shell/ZipalignCommand.java b/src/com/facebook/buck/shell/ZipalignCommand.java
@@ -34,7 +34,7 @@ public ZipalignCommand(String inputFile, String outputFile) {
   protected ImmutableList<String> getShellCommandInternal(ExecutionContext context) {
     ImmutableList.Builder<String> args = ImmutableList.builder();
 
-    AndroidPlatformTarget androidPlatformTarget = context.getAndroidPlatformTarget().get();
+    AndroidPlatformTarget androidPlatformTarget = context.getAndroidPlatformTarget();
     args.add(androidPlatformTarget.getZipalignExecutable().getAbsolutePath());
     args.add("-f").add("4");
     args.add(inputFile);