Look for "docker-compose" in path

[diamondT]

Adds directories found in PATH to the list of possible paths for docker executables.
It would be nice to work out-of-the-box, i.e. without needing a special env variable (DOCKER_LOCATION or DOCKER_COMPOSE_LOCATION) if the executable(s) already exists in path, even if not in one of the well-known locations.

[hpryce]

I really like this idea, it should reduce the amount of configuration necessary for people to use the library. However, this constructor now seems to be getting quite complicated and I don't think it's possible to write a test for finding the executable in the path with how this is written.

This seems like a really good candidate for using Immutables (https://immutables.github.io/), which we use elsewhere in the codebase - see DockerComposeExecArgument for a simple example. Using immutables you could do something like this:

@Value.Immutable
public abstract class DockerCommandLocations {

  protected String executableName();
  protected List<String> additionalSearchLocations();

  @Value.Default
   protected List<String> path() {
      // read path environment variable
   }

  @Value.Derived
  public Optional<String> preferredLocation() { ... }

  public static Builder builder();

}

Then, using the builder, it is possible to write unit tests by setting the path @Value.Default value so we can ensure this continues working in the future.

[hpryce]

See comment on DockerCommandLocation

[diamondT]

additionalSearchLocations() has to be a list of optionals since System.getenv might or might not return a null value.
Alternatively, immutables optionally (sic) allows null values for plain lists but that would require an update to the latest version and wouldn't make a big difference imho.

[alicederyn]

Why not filter out the nulls?

[diamondT]

it's the immutables generated method that does not allow nulls. having something like this

DOCKER_COMPOSE_LOCATIONS = DockerCommandLocations.builder()
            .executableName("docker-compose")
            .additionalSearchLocations(Arrays.asList(
                System.getenv("DOCKER_COMPOSE_LOCATION"),
                "/usr/local/bin/docker-compose",
                "/usr/bin/docker-compose"
            )).build();

for an unset DOCKER_COMPOSE_LOCATION would fail.

[alicederyn]

Stream.of(...).filter(o -> o != null).collect(toList()) ought to do it

[diamondT]

agreed. however, having a signature of List<String> would fail in runtime (under certain circumstances). the Stream.of() solution is fine but the caller would have to know that they should call this method using this specific construct and not by using any other plain list.
putting the Optional in the signature ensures null-safety at compile time, right?

[hpryce]

@diamondT sorry for the delay here. I've pulled these changes into #175 to include support for Windows, all going well we should be able to ship them soon.

I'm going to close this PR out as the linked one includes everything this does.

[diamondT]

@hpryce Cool, thanks for the update.