search the PATH for zipalign

src/main/java/org/jenkinsci/plugins/androidsigning/SignApksBuilder.java

@@ -254,13 +254,26 @@ public void perform(@Nonnull Run<?, ?> run, @Nonnull FilePath workspace, @Nonnul
             return;
         }
 
-        EnvVars env;
-        if (run instanceof AbstractBuild) {
-            env = run.getEnvironment(listener);
-            env.overrideAll(((AbstractBuild<?,?>) run).getBuildVariables());
+        EnvVars env = new EnvVars();
+        Launcher.ProcStarter getEffectiveEnv = launcher.launch().pwd(workspace).cmdAsSingleString("echo \"resolving effective environment\"");
+        getEffectiveEnv.join();
+        String[] envLines = getEffectiveEnv.envs();
+        EnvVars shellEnv = new EnvVars();
+        for (String envVar : envLines) {
+            // fix double path separator Custom Tools seems to perpetrate
+            envVar.replaceAll(File.pathSeparator + "+", File.pathSeparator);
+            shellEnv.addLine(envVar);
         }
-        else {
-            env = new EnvVars();
+        /*
+         EnvVars.addLine() does not handle the += syntax, which the Custom Tools
+         plugin uses to append to PATH, so use overrideAll() method to handle that
+         syntax.
+         */
+        env.overrideAll(shellEnv);
+        if (run instanceof AbstractBuild) {
+            EnvVars runEnv = run.getEnvironment(listener);
+            env.overrideExpandingAll(runEnv);
+            env.overrideExpandingAll(((AbstractBuild<?,?>) run).getBuildVariables());
         }
 
         FilePath builderDir = workspace.child(BUILDER_DIR);

src/main/java/org/jenkinsci/plugins/androidsigning/ZipalignTool.java

@@ -2,6 +2,7 @@
 
 import org.apache.commons.lang.StringUtils;
 
+import java.io.File;
 import java.io.PrintStream;
 import java.util.List;
 import java.util.SortedMap;
@@ -21,25 +22,33 @@ class ZipalignTool {
 
     static final String ENV_ANDROID_HOME = "ANDROID_HOME";
     static final String ENV_ZIPALIGN_PATH = "ANDROID_ZIPALIGN";
+    static final String ENV_PATH = "PATH";
 
     private static FilePath findFromEnv(EnvVars env, FilePath workspace, PrintStream logger) throws AbortException {
 
         String zipalignPath = env.get(ENV_ZIPALIGN_PATH);
         if (!StringUtils.isEmpty(zipalignPath)) {
             zipalignPath = env.expand(zipalignPath);
             logger.printf("[SignApksBuilder] found zipalign path in env %s=%s%n", ENV_ZIPALIGN_PATH, zipalignPath);
-            return new FilePath(workspace.getChannel(), zipalignPath);
+            FilePath zipalign = new FilePath(workspace.getChannel(), zipalignPath);
+            return zipalignOrZipalignExe(zipalign, logger);
         }
 
         String androidHome = env.get(ENV_ANDROID_HOME);
-        if (StringUtils.isEmpty(androidHome)) {
-            throw new AbortException("failed to find zipalign: no environment variable " + ENV_ZIPALIGN_PATH + " or " + ENV_ANDROID_HOME);
+        if (!StringUtils.isEmpty(androidHome)) {
+            androidHome = env.expand(androidHome);
+            logger.printf("[SignApksBuilder] searching environment variable %s=%s for zipalign...%n", ENV_ANDROID_HOME, androidHome);
+            return findInAndroidHome(androidHome, workspace, logger);
         }
-        androidHome = env.expand(androidHome);
 
-        logger.printf("[SignApksBuilder] searching environment variable %s=%s for zipalign...%n", ENV_ANDROID_HOME, androidHome);
+        String envPath = env.get(ENV_PATH);
+        if (!StringUtils.isEmpty(envPath)) {
+            envPath = env.expand(envPath);
+            logger.printf("[SignApksBuilder] searching environment %s=%s for zipalign...%n", ENV_PATH, envPath);
+            return findInPathEnvVar(envPath, workspace, logger);
+        }
 
-        return findInAndroidHome(androidHome, workspace, logger);
+        throw new AbortException("failed to find zipalign: no environment variable " + ENV_ZIPALIGN_PATH + " or " + ENV_ANDROID_HOME + " or " + ENV_PATH);
     }
 
     private static FilePath findInAndroidHome(String androidHome, FilePath workspace, PrintStream logger) throws AbortException {
@@ -74,36 +83,99 @@ private static FilePath findInAndroidHome(String androidHome, FilePath workspace
 
         VersionNumber latest = versions.lastKey();
         buildTools = versions.get(latest);
-        FilePath zipalign = buildTools.child("zipalign");
+        FilePath zipalign = zipalignOrZipalignExe(buildTools, logger);
+        if (zipalign != null) {
+            logger.printf("[SignApksBuilder] found zipalign in Android SDK's latest build tools: %s%n", zipalign.getRemote());
+            return zipalign;
+        }
+
+        throw new AbortException("failed to find zipalign: no zipalign found in latest Android build tools: " + buildTools);
+    }
+
+    private static FilePath findInPathEnvVar(String envPath, FilePath workspace, PrintStream logger) throws AbortException {
+        String[] dirs = envPath.split(File.pathSeparator);
+        for (String dir : dirs) {
+            FilePath dirPath = workspace.child(dir);
+            FilePath zipalign = zipalignOrZipalignExe(dirPath, logger);
+            if (zipalign != null) {
+                return zipalign;
+            }
+            try {
+                dirPath = androidHomeAncestorOfPath(dirPath);
+            }
+            catch (Exception e) {
+                logger.println("error searching " + ENV_PATH + " environment variable: " + e.getMessage());
+                e.printStackTrace(logger);
+            }
+            if (dirPath != null) {
+                try {
+                    return findInAndroidHome(dirPath.getRemote(), workspace, logger);
+                }
+                catch (AbortException e) {
+                    logger.printf("error searching Android home found in " + ENV_PATH + ": " + e.getMessage());
+                }
+            }
+        }
 
-        logger.printf("[SignApksBuilder] found zipalign in Android SDK's latest build tools: %s%n", zipalign.getRemote());
+        return null;
+    }
+
+    private static FilePath androidHomeAncestorOfPath(FilePath path) throws Exception {
+        if ("bin".equals(path.getName())) {
+            FilePath sdkmanager = path.child("sdkmanager");
+            if (sdkmanager.exists()) {
+                path = path.getParent();
+                if (path != null && "tools".equals(path.getName())) {
+                    return path.getParent();
+                }
+            }
+        }
+        else if ("tools".equals(path.getName())) {
+            FilePath androidTool = path.child("android");
+            if (androidTool.exists()) {
+                return path.getParent();
+            }
+        }
+        else if (path.child("tools").child("android").exists()) {
+            return path;
+        }
 
-        return zipalign;
+        return null;
     }
 
-    private static FilePath ensureZipalignExists(FilePath zipalign, PrintStream logger) throws AbortException {
+    private static FilePath zipalignOrZipalignExe(FilePath zipalignOrDir, PrintStream logger) {
         try {
-            if (zipalign.exists()) {
-                return zipalign;
+            if (zipalignOrDir.isDirectory()) {
+                zipalignOrDir = zipalignOrDir.child("zipalign");
             }
         }
         catch (Exception e) {
+            logger.println("[SignApksBuilder] error checking for zipalign at path " + zipalignOrDir);
             e.printStackTrace(logger);
-            throw new AbortException(e.getMessage());
         }
         try {
-            zipalign = zipalign.getParent().child("zipalign.exe");
-            if (zipalign.exists()) {
-                return zipalign;
+            if (zipalignOrDir.exists()) {
+                return zipalignOrDir;
+            }
+        }
+        catch (Exception e) {
+            logger.println("[SignApksBuilder] error checking for zipalign at path " + zipalignOrDir);
+            e.printStackTrace(logger);
+        }
+        try {
+            zipalignOrDir = zipalignOrDir.getParent().child("zipalign.exe");
+            if (zipalignOrDir.exists()) {
+                return zipalignOrDir;
             }
         }
         catch (Exception e) {
+            logger.println("[SignApksBuilder] error checking for zipalign.exe at path " + zipalignOrDir);
             e.printStackTrace(logger);
-            throw new AbortException(e.getMessage());
         }
 
-        throw new AbortException("failed to find zipalign: no zipalign/zipalign.exe in latest build-tools path " +
-            zipalign.getParent().getRemote());
+        logger.println("[SignApksBuilder] no zipalign found at path " + zipalignOrDir);
+
+        return null;
     }
 
     private final EnvVars env;
@@ -125,7 +197,7 @@ ArgumentListBuilder commandFor(String unsignedApk, String outputApk) throws Abor
         if (zipalign == null) {
             if (!StringUtils.isEmpty(overrideZipalignPath)) {
                 logger.printf("[SignApksBuilder] zipalign path explicitly set to %s%n", overrideZipalignPath);
-                zipalign = workspace.child(env.expand(overrideZipalignPath));
+                zipalign = zipalignOrZipalignExe(workspace.child(env.expand(overrideZipalignPath)), logger);
             }
             else if (!StringUtils.isEmpty(overrideAndroidHome)) {
                 logger.printf("[SignApksBuilder] zipalign %s explicitly set to %s%n", ENV_ANDROID_HOME, overrideAndroidHome);
@@ -136,7 +208,9 @@ else if (!StringUtils.isEmpty(overrideAndroidHome)) {
                 zipalign = findFromEnv(env, workspace, logger);
             }
 
-            zipalign = ensureZipalignExists(zipalign, logger);
+            if (zipalign == null) {
+                throw new AbortException("failed to find zipalign path in parameters or environment");
+            }
         }
 
         return new ArgumentListBuilder()

src/test/java/org/jenkinsci/plugins/androidsigning/FakeZipalign.java

@@ -17,6 +17,9 @@ class FakeZipalign implements FakeLauncher {
 
     @Override
     public Proc onLaunch(Launcher.ProcStarter p) throws IOException {
+        if (!p.cmds().get(0).contains("zipalign")) {
+            return new FinishedProc(0);
+        }
         lastProc = p;
         PrintStream logger = new PrintStream(p.stdout());
         List<String> cmd = p.cmds();

src/test/java/org/jenkinsci/plugins/androidsigning/SignApksBuilderTest.java

@@ -13,7 +13,9 @@
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.RuleChain;
+import org.junit.rules.TemporaryFolder;
 import org.junit.rules.TestName;
+import org.jvnet.hudson.test.FakeLauncher;
 import org.jvnet.hudson.test.JenkinsRule;
 import org.jvnet.hudson.test.PretendSlave;
 import org.jvnet.hudson.test.WithoutJenkins;
@@ -33,14 +35,21 @@
 
 import hudson.EnvVars;
 import hudson.FilePath;
+import hudson.Launcher;
+import hudson.Proc;
 import hudson.model.Build;
+import hudson.model.Descriptor;
 import hudson.model.FreeStyleBuild;
 import hudson.model.FreeStyleProject;
 import hudson.model.Label;
 import hudson.model.Result;
 import hudson.model.Run;
+import hudson.model.TaskListener;
 import hudson.security.ACL;
+import hudson.slaves.ComputerLauncher;
 import hudson.slaves.EnvironmentVariablesNodeProperty;
+import hudson.slaves.NodeProperty;
+import hudson.slaves.NodePropertyDescriptor;
 import jenkins.util.VirtualFile;
 
 import static org.hamcrest.CoreMatchers.containsString;
@@ -70,8 +79,36 @@
 import static org.junit.Assert.fail;
 
 
+@SuppressWarnings("deprecation")
 public class SignApksBuilderTest {
 
+    static class DecoratingLauncherSlave extends PretendSlave {
+
+        private static final long serialVersionUID = 1L;
+
+        List<String> envLines = new ArrayList<>();
+
+        DecoratingLauncherSlave(String name, String remoteFS, String labelString, ComputerLauncher launcher, FakeLauncher faker) throws IOException, Descriptor.FormException {
+            super(name, remoteFS, labelString, launcher, faker);
+        }
+
+        DecoratingLauncherSlave decorateWithEnv(String envLine) {
+            envLines.add(envLine);
+            return this;
+        }
+
+        @Override
+        public Launcher createLauncher(TaskListener listener) {
+            Launcher launcher = super.createLauncher(listener);
+            return new Launcher.DecoratedLauncher(launcher) {
+                @Override
+                public Proc launch(ProcStarter starter) throws IOException {
+                    return getInner().launch(starter.envs(envLines.toArray(new String[envLines.size()])));
+                }
+            };
+        }
+    }
+
     private static BuildArtifact buildArtifact(FreeStyleBuild build, Run.Artifact artifact) {
         return new BuildArtifact(build, artifact);
     }
@@ -89,6 +126,9 @@ private ApkArtifactIsSignedMatcher isSigned() throws KeyStoreException {
     @Rule
     public RuleChain jenkinsChain = RuleChain.outerRule(testJenkins).around(keyStoreRule);
 
+    @Rule
+    public TemporaryFolder testDir = new TemporaryFolder();
+
     @Rule
     public TestName currentTestName = new TestName();
 
@@ -415,6 +455,48 @@ public void usesZipalignPathOverride() throws Exception {
         assertThat(zipalignLauncher.lastProc.cmds().get(0), startsWith(zipalignOverride.getRemote()));
     }
 
+    @Test
+    public void retrievesEnvVarsFromDecoratedLauncherForZipalignCommand() throws Exception {
+
+        FilePath decoratedZipalign = new FilePath(testDir.newFolder("decorated")).createTempFile("decorated-", "-zipalign");
+        DecoratingLauncherSlave decoratingSlave = new DecoratingLauncherSlave("decorating", testJenkins.createTmpDir().getPath(), "decorating", testJenkins.createComputerLauncher(null), zipalignLauncher)
+            .decorateWithEnv("ANDROID_ZIPALIGN=" + decoratedZipalign.getRemote());
+
+        synchronized (testJenkins.jenkins) {
+            testJenkins.jenkins.addNode(decoratingSlave);
+        }
+
+        SignApksBuilder builder = new SignApksBuilder();
+        builder.setKeyStoreId(KEY_STORE_ID);
+        builder.setApksToSign("*-unsigned.apk");
+        FreeStyleProject job = createSignApkJob();
+        job.setAssignedLabel(decoratingSlave.getSelfLabel());
+        job.getBuildersList().add(builder);
+        testJenkins.buildAndAssertSuccess(job);
+
+        assertThat(zipalignLauncher.lastProc.cmds().get(0), startsWith(decoratedZipalign.getRemote()));
+    }
+
+    @Test
+    public void abortsIfZipalignIsNotFound() throws Exception {
+
+        for (NodeProperty property : testJenkins.jenkins.getGlobalNodeProperties()) {
+            if (property instanceof EnvironmentVariablesNodeProperty) {
+                EnvironmentVariablesNodeProperty envProp = (EnvironmentVariablesNodeProperty)property;
+                envProp.getEnvVars().override("ANDROID_HOME", "/null_android");
+            }
+        }
+
+        SignApksBuilder builder = new SignApksBuilder();
+        builder.setKeyStoreId(KEY_STORE_ID);
+        builder.setApksToSign("*-unsigned.apk");
+        FreeStyleProject job = createSignApkJob();
+        job.getBuildersList().add(builder);
+
+        Run run = testJenkins.assertBuildStatus(Result.FAILURE, job.scheduleBuild2(0));
+        testJenkins.assertLogContains("failed to find zipalign", run);
+    }
+
     @Test
     public void skipsZipalign() throws Exception {
         SignApksBuilder builder = new SignApksBuilder();