Uploaded image for project: 'Jenkins'
  1. Jenkins
  2. JENKINS-30396

Incorrect Unity executable path "suffix" for OSX in 1.2 release

    XMLWordPrintable

    Details

    • Type: Bug
    • Status: Resolved (View Workflow)
    • Priority: Blocker
    • Resolution: Fixed
    • Component/s: unity3d-plugin
    • Labels:
      None
    • Environment:
      unity3d-plugin version 1.2
      jenkins core version 1.619
    • Similar Issues:

      Description

      New version 1.2 seems to have introduced this bug:

      ====
      FATAL: The configured Unity3d installation directory (/Applications/Unity/Unity.app) is not recognized as a Unity3d home directory. Remember that the plugin adds per-platform suffixes and is searching for the executable at /Applications/Unity/Unity.app/Editor/Unity.
      ====

      That is not the correct path to Unity executable on OSX. It is "/Applications/Unity/Unity.app/Contents/MacOS/Unity"

        Attachments

          Activity

          Hide
          joensson Flemming Joensson added a comment - - edited

          Same here - unable to install and configure 1.2 due to this
          The help text is correct for OSx, but the failure indicates that something is assymmetric between the check and the help.

          https://www.dropbox.com/s/22ckebttczejxg8/Screenshot%202015-09-11%2014.04.06.png?dl=0

          Show
          joensson Flemming Joensson added a comment - - edited Same here - unable to install and configure 1.2 due to this The help text is correct for OSx, but the failure indicates that something is assymmetric between the check and the help. https://www.dropbox.com/s/22ckebttczejxg8/Screenshot%202015-09-11%2014.04.06.png?dl=0
          Hide
          joensson Flemming Joensson added a comment - - edited

          I did a little digging in the code - the problem is in Functions2 - the OS_NAME static is initialized too late.

          When everything is static order is important.

          class Functions2 {
          	public static boolean isMac() {
          		return IS_OS_MAC;
          	}
          	public static boolean isLinux() {
          		return IS_OS_LINUX;
          	}
              private static final boolean IS_OS_LINUX = getOSMatchesName("Linux") || getOSMatchesName("LINUX");
              private static final boolean IS_OS_MAC = getOSMatchesName("Mac");
          
              private static final String OS_NAME = System.getProperty("os.name");
          

          ////

          The IS_OS_LINUX and IS_OS_MAC lines get initialized by calling getOSMatchesName

          getOSMatchesName in turn calls isOSNameMatch(OS_NAME, osNamePrefix);

          But at this point OS_NAME has not yet been initialized so all the checks fail because OS_NAME is null.

          So when org.jenkinsci.plugins.unity3d.Unity3dInstallation#checkUnity3dExecutablePath calls org.jenkinsci.plugins.unity3d.Unity3dInstallation.Unity3dExecutablePath#check which calls org.jenkinsci.plugins.unity3d.Unity3dInstallation.Unity3dExecutablePath#getExeFile that does this check

                  private static File getExeFile(File unityHome) {
                      if (Functions.isWindows()) {
                          return new File(unityHome, "Editor/Unity.exe");
                      } else if (Functions2.isMac()) {
                          return new File(unityHome, "Contents/MacOS/Unity");
                      } else { // Linux assumed
                          return new File(unityHome, "Editor/Unity");
                      }
                  }
          

          You always end up getting the Linux path appended unless the Windows check is true.

          Suggested Fix:
          Move the OS_NAME initializer up to the first line in Functions2 so OS_NAME is the first value initialized or drop the field completely and change

          
              private static boolean getOSMatchesName(String osNamePrefix) {
                  return isOSNameMatch(OS_NAME, osNamePrefix);
              }
          

          Into this:

          
              private static boolean getOSMatchesName(String osNamePrefix) {
                  return isOSNameMatch(System.getProperty("os.name"), osNamePrefix);
              }
          

          Both solutions should fix the issue for Mac users

          Show
          joensson Flemming Joensson added a comment - - edited I did a little digging in the code - the problem is in Functions2 - the OS_NAME static is initialized too late. When everything is static order is important. class Functions2 { public static boolean isMac() { return IS_OS_MAC; } public static boolean isLinux() { return IS_OS_LINUX; } private static final boolean IS_OS_LINUX = getOSMatchesName( "Linux" ) || getOSMatchesName( "LINUX" ); private static final boolean IS_OS_MAC = getOSMatchesName( "Mac" ); private static final String OS_NAME = System .getProperty( "os.name" ); //// The IS_OS_LINUX and IS_OS_MAC lines get initialized by calling getOSMatchesName getOSMatchesName in turn calls isOSNameMatch(OS_NAME, osNamePrefix); But at this point OS_NAME has not yet been initialized so all the checks fail because OS_NAME is null. So when org.jenkinsci.plugins.unity3d.Unity3dInstallation#checkUnity3dExecutablePath calls org.jenkinsci.plugins.unity3d.Unity3dInstallation.Unity3dExecutablePath#check which calls org.jenkinsci.plugins.unity3d.Unity3dInstallation.Unity3dExecutablePath#getExeFile that does this check private static File getExeFile(File unityHome) { if (Functions.isWindows()) { return new File(unityHome, "Editor/Unity.exe" ); } else if (Functions2.isMac()) { return new File(unityHome, "Contents/MacOS/Unity" ); } else { // Linux assumed return new File(unityHome, "Editor/Unity" ); } } You always end up getting the Linux path appended unless the Windows check is true. Suggested Fix: Move the OS_NAME initializer up to the first line in Functions2 so OS_NAME is the first value initialized or drop the field completely and change private static boolean getOSMatchesName( String osNamePrefix) { return isOSNameMatch(OS_NAME, osNamePrefix); } Into this: private static boolean getOSMatchesName( String osNamePrefix) { return isOSNameMatch( System .getProperty( "os.name" ), osNamePrefix); } Both solutions should fix the issue for Mac users
          Hide
          scm_issue_link SCM/JIRA link daemon added a comment -

          Code changed in jenkins
          User: Jerome Lacoste
          Path:
          src/main/java/org/jenkinsci/plugins/unity3d/Functions2.java
          http://jenkins-ci.org/commit/unity3d-plugin/e7c8ad2bf825265a83b13787d2c796ad7c50c2fe
          Log:
          [FIXED JENKINS-30396] incorrect field initilization order broke Mac/Linux detection

          Show
          scm_issue_link SCM/JIRA link daemon added a comment - Code changed in jenkins User: Jerome Lacoste Path: src/main/java/org/jenkinsci/plugins/unity3d/Functions2.java http://jenkins-ci.org/commit/unity3d-plugin/e7c8ad2bf825265a83b13787d2c796ad7c50c2fe Log: [FIXED JENKINS-30396] incorrect field initilization order broke Mac/Linux detection
          Hide
          lacostej lacostej added a comment -

          Really sorry about that. I only tested on Linux and didn't do the test on my own server... That will teach me from copy pasting things quickly.

          Thanks a lot Flemming for the dig. Pushed a fix, and making a new release right now. Not sure how long it takes for the release to appear online.

          Show
          lacostej lacostej added a comment - Really sorry about that. I only tested on Linux and didn't do the test on my own server... That will teach me from copy pasting things quickly. Thanks a lot Flemming for the dig. Pushed a fix, and making a new release right now. Not sure how long it takes for the release to appear online.
          Hide
          joensson Flemming Joensson added a comment -

          Sweet - thank you for the quick fix and have a nice weekend

          Show
          joensson Flemming Joensson added a comment - Sweet - thank you for the quick fix and have a nice weekend

            People

            • Assignee:
              lacostej lacostej
              Reporter:
              lockarm Dickinson Lo
            • Votes:
              1 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: