[jclouds] Fix InitScriptConfigurationForTasks.initScriptPattern on Windows hosts (#677)

[Richard Downer <no...@github.com>]

This method has incorrect results when run on Windows. It is expected to
generate folder names compatible with Unix-like target machines, but by
using File.getParent it will actually be influenced by the host machine
OS type. This results in baseDir being set to a style that will not work
on Unix-like targets.
You can view, comment on, or merge this pull request online at:

  https://github.com/jclouds/jclouds/pull/677

-- Commit Summary --

  * Fix InitScriptConfigurationForTasks.initScriptPattern on Windows

-- File Changes --

    M compute/src/main/java/org/jclouds/compute/callables/InitScriptConfigurationForTasks.java (17)
    M compute/src/test/java/org/jclouds/compute/callables/InitScriptConfigurationForTasksTest.java (22)

-- Patch Links --

https://github.com/jclouds/jclouds/pull/677.patch
https://github.com/jclouds/jclouds/pull/677.diff

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/677

Re: [jclouds] Fix InitScriptConfigurationForTasks.initScriptPattern on Windows hosts (#677)

[Ignasi Barrera <no...@github.com>]

> @@ -76,4 +73,19 @@ public void testIncrementingTimeSupplier() throws InterruptedException {
>        assertEquals(config.getAnonymousTaskSuffixSupplier().get(), "1");
>     }
>  
> +   @Test
> +   public void testInitScriptPattern() throws Exception {
> +      InitScriptConfigurationForTasks config = InitScriptConfigurationForTasks.create();
> +      config.initScriptPattern("/var/tmp/jclouds-%s");
> +      assertEquals(config.getBasedir(), "/var/tmp");
> +      assertEquals(config.getInitScriptPattern(), "/var/tmp/jclouds-%s");
> +   }
> +
> +   @Test
> +   public void testInitScriptPatternAtRoot() throws Exception {
> +      InitScriptConfigurationForTasks config = InitScriptConfigurationForTasks.create();
> +      config.initScriptPattern("/jclouds-%s");
> +      assertEquals(config.getBasedir(), "/");
> +      assertEquals(config.getInitScriptPattern(), "/jclouds-%s");
> +   }

I'd also add a test to verify we now require the initscript to start with a slash. Let's add a test that verifies that the IAE is thrown.

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/677/files#r24507975

Re: [jclouds] Fix InitScriptConfigurationForTasks.initScriptPattern on Windows hosts (#677)

[Ignasi Barrera <no...@github.com>]

Just two minors. Apart from that lgtm. Thanks @rdowner!

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/677#issuecomment-73910510

Re: [jclouds] Fix InitScriptConfigurationForTasks.initScriptPattern on Windows hosts (#677)

[Ignasi Barrera <no...@github.com>]

> @@ -47,7 +47,20 @@ protected InitScriptConfigurationForTasks() {
>     public InitScriptConfigurationForTasks initScriptPattern(
>              @Named(PROPERTY_INIT_SCRIPT_PATTERN) String initScriptPattern) {
>        this.initScriptPattern = checkNotNull(initScriptPattern, "initScriptPattern ex. /tmp/init-%s");
> -      this.basedir = new File(initScriptPattern).getParent();
> +      checkArgument(this.initScriptPattern.startsWith("/"), "initScriptPattern must be a UNIX-style path starting at the root (/)");
> +
> +      int lastSlash = initScriptPattern.lastIndexOf('/');
> +      if (lastSlash == -1) {
> +         // no slash? impossible if the checkArgument above had worked properly!

Then remove this statement, and leave just the comment?

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/677/files#r24507846

Re: [jclouds/jclouds] Fix InitScriptConfigurationForTasks.initScriptPattern on Windows hosts (#677)

[Andrea Turli <no...@github.com>]

@rdowner can you please address the 2 minor comments from @nacx so that we can merge it?
Thanks!

---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/677#issuecomment-238022564

Re: [jclouds/jclouds] Fix InitScriptConfigurationForTasks.initScriptPattern on Windows hosts (#677)

[Ignasi Barrera <no...@github.com>]

Rebased to the latest master version, addressed the minors and merged to master as [199e17ab](http://git-wip-us.apache.org/repos/asf/jclouds/commit/199e17ab).

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/677#issuecomment-256050677

Re: [jclouds/jclouds] Fix InitScriptConfigurationForTasks.initScriptPattern on Windows hosts (#677)

[Ignasi Barrera <no...@github.com>]

Closed #677.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/677#event-835548187