You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brooklyn.apache.org by neykov <gi...@git.apache.org> on 2014/06/24 16:33:04 UTC

[GitHub] incubator-brooklyn pull request: "Deploy from Windows" fixes

GitHub user neykov opened a pull request:

    https://github.com/apache/incubator-brooklyn/pull/13

    "Deploy from Windows" fixes

    

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/neykov/incubator-brooklyn deploy-from-windows

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/incubator-brooklyn/pull/13.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #13
    
----
commit d3aee2d7845aedc8bcead5b34c2b55d6896422bc
Author: Svetoslav Neykov <sv...@cloudsoftcorp.com>
Date:   2014-06-05T11:39:52Z

    Fixes for "deploy from Windows"

commit 8147a234eeb557b63e0e0596d5a77919068b2e90
Author: Svetoslav Neykov <sv...@cloudsoftcorp.com>
Date:   2014-06-06T06:59:22Z

    Use Freemaker raw strings for dynamic values. This will force Freemaker
    to treat backslash and ${ as regular text.

commit 1a37dc4d6157170fb1b5d3879c7fd620c4b5ced4
Author: Svetoslav Neykov <sv...@cloudsoftcorp.com>
Date:   2014-06-06T07:13:04Z

    The local system temp dir will be used on the entity's remote system. Instead hardcode some universal value ("/tmp"). Analogous to issue #1402.
    Shouldn't we raise an error in the case where it is needed instead of passing an invalid path?

commit ce49f173480936bcb4ef5a79ed2f1b903149d4e2
Author: Svetoslav Neykov <sv...@cloudsoftcorp.com>
Date:   2014-06-06T07:31:32Z

    Replace ad-hock absolute path check with a method call. Rename existing isAbsolute method to isAbsoluteLocal to emphasize invalid usage.

commit 2e14c0e85dd7e3a504db79c04a905912296b7436
Author: Svetoslav Neykov <sv...@cloudsoftcorp.com>
Date:   2014-06-06T14:50:29Z

    Manual changes of the URL lead to an incorrect path on Windows.
    Try to use the platform provided functionality as much as possible.

commit 091454b78495a43bbbcb5213921e72db980c21f6
Author: Svetoslav Neykov <sv...@cloudsoftcorp.com>
Date:   2014-06-06T14:54:08Z

    Manual URL manipulation leads to invalid results.
    
    The test was working by accident on UNIX - with an absolute path the
    beginning of the string would be file:///, which is the expected prefix.
    On Windows though the result was an invalid URL. Use the platform
    provided functionality to construct the URL and remove invalid
    statement in the comment.

commit 18205cd7b10676ff3224c68f2a83629a3ce25a77
Author: Svetoslav Neykov <sv...@cloudsoftcorp.com>
Date:   2014-06-06T14:55:34Z

    No manual path construction.
    
    Use the correct constructor to create a child path.

commit 460cc4da1653d8a163d2370ab437a4f08c24126d
Author: Svetoslav Neykov <sv...@cloudsoftcorp.com>
Date:   2014-06-06T14:56:34Z

    Zip entries use UNIX style (relative) paths by standard.
    
    Why the leading dot though?

commit ea8c45bdd80e632f109e268a028b53f48dbbb630
Author: Svetoslav Neykov <sv...@cloudsoftcorp.com>
Date:   2014-06-06T15:02:47Z

    No echo executable on Windows.
    
    There is no echo executable on Windows, it is a command
    provided by the shell. To test the process builder we need
    OS-specific commands. The alternative is to include shell
    scripts for both UNIX and Windows and call them with an
    uniform command.

commit 677115d90e74b9214c7c09ff975a66b077cda141
Author: Svetoslav Neykov <sv...@cloudsoftcorp.com>
Date:   2014-06-06T15:04:27Z

    Fix check for Windows OS.
    
    The format is "Windows XX", no microsoft in the name.

commit c4dba8559d91d07a6dbfc85d47c7cece338f307b
Author: Svetoslav Neykov <sv...@cloudsoftcorp.com>
Date:   2014-06-23T09:29:25Z

    Add comments explaining the reason for the code changes.

commit fc3e893c33809ae87825f6a78397b43a12760ca8
Author: Svetoslav Neykov <sv...@cloudsoftcorp.com>
Date:   2014-06-24T09:01:13Z

    Replace platform specific isAbsolute check with a common implementation with the tradeoff that it doesn't handle specific cases.

commit fa4794c780f83389942be18bfd1d8df2f0e553b7
Author: Svetoslav Neykov <sv...@cloudsoftcorp.com>
Date:   2014-06-24T09:03:38Z

    Fix dev mode check for Windows

commit 346cedf8955029753927dd3dfee9565500c82565
Author: Svetoslav Neykov <sv...@cloudsoftcorp.com>
Date:   2014-06-24T09:05:56Z

    Replace tilda with explicit home path reference to be compatible with Windows.

commit 97dc4320bfa2a11d73cd74accc9e94eea730b26c
Author: Svetoslav Neykov <sv...@cloudsoftcorp.com>
Date:   2014-06-24T09:17:17Z

    Replace static fallback folder which could be invalid in certain contexts with an invalid folder name which will cause errors when being used. The invalid name will hint the user at the potential problem.

commit 2d17bca4e3f7e8a18888f80469e6243b6a0075b4
Author: Svetoslav Neykov <sv...@cloudsoftcorp.com>
Date:   2014-06-24T13:16:40Z

    Rewrite the .ps1 launcher script in windows batch script since powershell can't execute file scripts out of the box.

commit da365723f0b2eb424b7c69c314d14f650e4b5936
Author: Svetoslav Neykov <sv...@cloudsoftcorp.com>
Date:   2014-06-24T13:46:05Z

    Reduce memory requirements to values which can be run in 32 bit Java on Windows.

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: "Deploy from Windows" fixes

Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the pull request:

    https://github.com/apache/incubator-brooklyn/pull/13#issuecomment-47046593
  
    tremendous work.  let's merge this ASAP!
    
    i have not tested but it can only be an improvement.  i will ping some other windows users to try it out but i would rather we merged it first as it will be easier for them to try it that way.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: "Deploy from Windows" fixes

Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/13#discussion_r14170616
  
    --- Diff: utils/common/src/main/java/brooklyn/util/os/Os.java ---
    @@ -447,21 +450,44 @@ public static String tidyPath(String path) {
             return result;
         }
     
    -    public static boolean isAbsolute(String path) {
    -        if (path.startsWith(File.separator)) return true;
    -        if (Os.isMicrosoftWindows()) {
    -            if (path.length()>=3 && path.substring(1).startsWith(":\\"))
    -                return true;
    -        } else {
    -            if (path.equals("~") || path.startsWith("~/")) return true;
    -        }
    -        return false;
    +    /**
    +     * Checks whether a file system path is absolute in a platform neutral way.
    +     * <p>
    +     * As a consequence of the platform neutrality some edge cases are
    +     * not handled correctly:
    +     * <ul>
    +     *  <li>On Windows relative paths starting with slash (either forward or backward) or ~ are treated as absolute.
    +     *  <li>On UNIX relative paths starting with X:/ are treated as absolute.
    +     * </ul>
    +     *
    +     * @param path A string representing a file system path.
    +     * @return whether the path is absolute under the above constraints.
    +     */
    +    public static boolean isAbsolutish(String path) {
    +        return
    +            path.codePointAt(0) == SEPARATOR_UNIX ||
    +            path.equals("~") || path.startsWith("~" + SEPARATOR_UNIX) ||
    +            path.length()>=3 && path.codePointAt(1) == ':' &&
    +                                isSeparator(path.codePointAt(2));
    +    }
    +
    +    private static boolean isSeparator(int sep) {
    +        return sep == SEPARATOR_UNIX ||
    +               sep == SEPARATOR_WIN;
    +    }
    +    
    +    public static String fromHome(String path) {
    +        return new File(Os.home(), path).getAbsolutePath();
    +    }
    +    
    +    public static String nativePath(String path) {
    +        return new File(path).getPath();
         }
     
         public static boolean isMicrosoftWindows() {
    -        if (System.getProperty("os.name").toLowerCase().indexOf("microsoft")>=0)
    -            return true;
    -        return false;
    +        String os = System.getProperty("os.name").toLowerCase();
    +        //see org.apache.commons.lang.SystemUtils.IS_WINDOWS
    +		return os.startsWith("windows");
    --- End diff --
    
    Removed tabs


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: "Deploy from Windows" fixes

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/incubator-brooklyn/pull/13


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: "Deploy from Windows" fixes

Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/13#discussion_r14163223
  
    --- Diff: utils/common/src/main/java/brooklyn/util/os/Os.java ---
    @@ -447,21 +450,44 @@ public static String tidyPath(String path) {
             return result;
         }
     
    -    public static boolean isAbsolute(String path) {
    -        if (path.startsWith(File.separator)) return true;
    -        if (Os.isMicrosoftWindows()) {
    -            if (path.length()>=3 && path.substring(1).startsWith(":\\"))
    -                return true;
    -        } else {
    -            if (path.equals("~") || path.startsWith("~/")) return true;
    -        }
    -        return false;
    +    /**
    +     * Checks whether a file system path is absolute in a platform neutral way.
    +     * <p>
    +     * As a consequence of the platform neutrality some edge cases are
    +     * not handled correctly:
    +     * <ul>
    +     *  <li>On Windows relative paths starting with slash (either forward or backward) or ~ are treated as absolute.
    +     *  <li>On UNIX relative paths starting with X:/ are treated as absolute.
    +     * </ul>
    +     *
    +     * @param path A string representing a file system path.
    +     * @return whether the path is absolute under the above constraints.
    +     */
    +    public static boolean isAbsolutish(String path) {
    +        return
    +            path.codePointAt(0) == SEPARATOR_UNIX ||
    +            path.equals("~") || path.startsWith("~" + SEPARATOR_UNIX) ||
    +            path.length()>=3 && path.codePointAt(1) == ':' &&
    +                                isSeparator(path.codePointAt(2));
    +    }
    +
    +    private static boolean isSeparator(int sep) {
    +        return sep == SEPARATOR_UNIX ||
    +               sep == SEPARATOR_WIN;
    +    }
    +    
    +    public static String fromHome(String path) {
    +        return new File(Os.home(), path).getAbsolutePath();
    +    }
    +    
    +    public static String nativePath(String path) {
    +        return new File(path).getPath();
         }
     
         public static boolean isMicrosoftWindows() {
    -        if (System.getProperty("os.name").toLowerCase().indexOf("microsoft")>=0)
    -            return true;
    -        return false;
    +        String os = System.getProperty("os.name").toLowerCase();
    +        //see org.apache.commons.lang.SystemUtils.IS_WINDOWS
    +		return os.startsWith("windows");
    --- End diff --
    
    indentation is weird here


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---