ARQ-2074 Check if binary is already executable before trying to chmod…

… it regardless
arquillian · Mar 9, 2017 · 7c0e617 · 7c0e617
1 parent 18ce9ed
commit 7c0e617
Show file tree

Hide file tree

Showing 2 changed files with 22 additions and 14 deletions.
diff --git a/.../main/java/org/jboss/arquillian/drone/webdriver/binary/handler/AbstractBinaryHandler.java b/.../main/java/org/jboss/arquillian/drone/webdriver/binary/handler/AbstractBinaryHandler.java
@@ -165,7 +165,7 @@ public File downloadAndPrepare() throws Exception {
      * Takes care of all steps but the first one of the method {@link AbstractBinaryHandler#downloadAndPrepare()}
      *
      * @param targetDir A directory where a downloaded binary should be stored
-     * @param from A url a binary should be downloaded from
+     * @param from      A url a binary should be downloaded from
      * @return An executable binary that was extracted/copied from the downloaded file
      * @throws Exception If anything bad happens
      */
@@ -177,7 +177,7 @@ protected File downloadAndPrepare(File targetDir, String from) throws Exception
      * Takes care of all steps but the first one of the method {@link AbstractBinaryHandler#downloadAndPrepare()}
      *
      * @param targetDir A directory where a downloaded binary should be stored
-     * @param from A url a binary should be downloaded from
+     * @param from      A url a binary should be downloaded from
      * @return An executable binary that was extracted/copied from the downloaded file
      * @throws Exception If anything bad happens
      */
@@ -193,18 +193,20 @@ protected File downloadAndPrepare(File targetDir, URL from) throws Exception {
     }
 
     /**
-     * Sets the given binary to be executable
+     * Sets the given binary to be executable (if it is not already set)
      *
      * @param binaryFile A binary file that should be set to be executable
      * @return the given binary file set to be executable
      */
-    protected File markAsExecutable(File binaryFile){
-        log.info("marking binary file: " + binaryFile.getPath() + " as executable");
-        try {
-            binaryFile.setExecutable(true);
-        } catch (SecurityException se) {
-            log.severe("The downloaded binary: " + binaryFile
-                           + " could not be set as executable. This may cause additional problems.");
+    protected File markAsExecutable(File binaryFile) {
+        if (!Validate.executable(binaryFile.getAbsolutePath())) {
+            log.info("marking binary file: " + binaryFile.getPath() + " as executable");
+            try {
+                binaryFile.setExecutable(true);
+            } catch (SecurityException se) {
+                log.severe("The downloaded binary: " + binaryFile
+                               + " could not be set as executable. This may cause additional problems.");
+            }
         }
         return binaryFile;
     }

diff --git a/drone-webdriver/src/main/java/org/jboss/arquillian/drone/webdriver/utils/Validate.java b/drone-webdriver/src/main/java/org/jboss/arquillian/drone/webdriver/utils/Validate.java
@@ -82,17 +82,23 @@ public static void isValidUrl(String url, String message) throws IllegalArgument
     }
 
     public static void isExecutable(String path, String message) throws IllegalArgumentException {
-        isEmpty(path, message);
+        if (!executable(path)) {
+            throw new IllegalArgumentException(message);
+        }
+    }
+
+    public static boolean executable(String path) {
+        if (empty(path)){
+            return false;
+        }
 
         File file = new File(path);
 
         if (!file.exists()) {
             throw new IllegalArgumentException(String.format("The file %s does not exist", path));
         }
 
-        if (!fileExecutableChecker.canExecute(file)) {
-            throw new IllegalArgumentException(message);
-        }
+        return fileExecutableChecker.canExecute(file);
     }
 
     /**