[FIXED JENKINS-20349] improved configuration error detection for Unit…

…y3d installation directory path
jenkinsci · May 26, 2015 · 4d94511 · 4d94511
1 parent 453756d
commit 4d94511
Show file tree

Hide file tree

Showing 3 changed files with 33 additions and 25 deletions.
diff --git a/src/main/java/org/jenkinsci/plugins/unity3d/Unity3dBuilder.java b/src/main/java/org/jenkinsci/plugins/unity3d/Unity3dBuilder.java
@@ -39,6 +39,7 @@
  * @author Jerome Lacoste
  */
 public class Unity3dBuilder extends Builder {
+    //private static final Logger log = Logger.getLogger(Unity3dInstallation.class.getName());
 
     private String unity3dName;
     private String argLine;
@@ -151,9 +152,11 @@ private String findLogFileArgument(ArgumentListBuilder args) {
     }
 
     private ArgumentListBuilder prepareCommandlineArguments(AbstractBuild<?,?> build, Launcher launcher, Unity3dInstallation ui, EnvVars vars) throws IOException, InterruptedException, PerformException {
-        String exe = ui.getExecutable(launcher);
-        if (exe==null) {
-            throw new PerformException(Messages.Unity3d_ExecutableNotFound(ui.getName()));
+        String exe;
+        try {
+            exe = ui.getExecutable(launcher);
+        } catch (RuntimeException re) {
+            throw new PerformException(re.getMessage());
         }
 
         FilePath moduleRoot = build.getModuleRoot();
@@ -238,7 +241,7 @@ public String getGlobalArgLine() {
         }
 
         public void setGlobalArgLine(String globalArgLine) {
-            System.out.println("HeLLO: " + globalArgLine);
+            //log.info("setGlobalArgLine: " + globalArgLine);
             this.globalArgLine = globalArgLine;
             save();
         }

diff --git a/src/main/java/org/jenkinsci/plugins/unity3d/Unity3dInstallation.java b/src/main/java/org/jenkinsci/plugins/unity3d/Unity3dInstallation.java
@@ -2,6 +2,7 @@
 
 import hudson.EnvVars;
 import hudson.Extension;
+import hudson.FilePath;
 import hudson.Functions;
 import hudson.Launcher;
 import hudson.Util;
@@ -26,6 +27,7 @@
 import java.util.Collections;
 import java.util.List;
 import java.util.concurrent.Future;
+import java.util.logging.Logger;
 
 /**
  * Represents a Unity3d installation (name, home_dir, etc.)
@@ -36,7 +38,7 @@ public class Unity3dInstallation
         extends ToolInstallation
         implements EnvironmentSpecific<Unity3dInstallation>, NodeSpecific<Unity3dInstallation> {
 
-    // private static final Logger log = LoggerFactory.getLogger(Unity3dInstallation.class);
+    private static final Logger log = Logger.getLogger(Unity3dInstallation.class.getName());
 
     @DataBoundConstructor
     public Unity3dInstallation(final String name, final String home, final List<? extends ToolProperty<?>> properties) {
@@ -61,17 +63,25 @@ public Unity3dInstallation forNode(Node node, TaskListener log) throws IOExcepti
     public String getExecutable(Launcher launcher) throws IOException, InterruptedException {
         return launcher.getChannel().call(new Callable<String, IOException>() {
             public String call() throws IOException {
-                File exe = getExeFile();
-                if (exe.exists())
-                    return exe.getPath();
-                return null;
+                return checkUnity3dExecutablePath(getHome());
             }
         });
     }
 
-    private File getExeFile() {
-        String unityHome = Util.replaceMacro(getHome(), EnvVars.masterEnvVars);
-        return getExeFile(new File(unityHome));
+    private static String checkUnity3dExecutablePath(String home) {
+        String unityHome = Util.replaceMacro(home, EnvVars.masterEnvVars);
+        log.fine("UNITY_HOME:" + unityHome);
+        File value = new File(unityHome);
+
+        File unityExe = getExeFile(value);
+
+        String path = unityExe.getAbsolutePath();
+
+        if (!value.isDirectory() || !unityExe.exists()) {
+            throw new RuntimeException(FormValidation.error(Messages.Unity3d_InvalidUnityHomeConfiguration(value, path)).getMessage());
+        }
+
+        return path;
     }
 
     private static File getExeFile(File unityHome) {
@@ -157,22 +167,19 @@ public List<? extends ToolInstaller> getDefaultInstallers() {
         /**
          * Checks if the UNITY_HOME is valid.
          */
-        public FormValidation doCheckHome(@QueryParameter File value) {
+        public FormValidation doCheckHome(@QueryParameter String value) {
             // this can be used to check the existence of a file on the server, so needs to be protected
             if (!Hudson.getInstance().hasPermission(Hudson.ADMINISTER))
                 return FormValidation.ok();
 
-            if (value.getPath().equals(""))
+            if (value.equals(""))
                 return FormValidation.ok();
 
-            if (!value.isDirectory())
-                return FormValidation.error(Messages.Unity3d_NotADirectory(value));
-
-            File unityExe = getExeFile(value);
-
-            if (!unityExe.exists())
-                return FormValidation.error(Messages.Unity3d_NotUnity3dHomeDirectory(value));
-
+            try {
+                checkUnity3dExecutablePath(value);
+            } catch (RuntimeException re) {
+                return FormValidation.error(re.getMessage());
+            }
             return FormValidation.ok();
         }
 

diff --git a/src/main/resources/org/jenkinsci/plugins/unity3d/Messages.properties b/src/main/resources/org/jenkinsci/plugins/unity3d/Messages.properties
@@ -1,6 +1,4 @@
 Unity3d.ExecUnexpectedlyFailed=Unity3d plugin execution unexpectedly failed
 Unity3d.UnityExecFailed=Unity3d command line execution failed with status {0}
-Unity3d.ExecutableNotFound=Executable not found {0}
-Unity3d.NotUnity3dHomeDirectory=Not Unity3d home directory {0}
-Unity3d.NotADirectory=Not a directory {0}
+Unity3d.InvalidUnityHomeConfiguration=The configured Unity3d installation directory ({0}) is not recognized as a Unity3d home directory. Remember that the plugin adds per-platform suffixes and is searching for the executable at {1}.
 Unity3d.NoUnity3dInstallation=No Unity3d Installation on this node