support --compose-file - as stdin

[mmariani]

As discussed in moby/moby#34036

[codecov-io]

Codecov Report

Merging #347 into master will increase coverage by 0.01%.
The diff coverage is 70.83%.

@@            Coverage Diff             @@
##           master     #347      +/-   ##
==========================================
+ Coverage   47.02%   47.04%   +0.01%     
==========================================
  Files         198      198              
  Lines       16336    16349      +13     
==========================================
+ Hits         7682     7691       +9     
- Misses       8260     8263       +3     
- Partials      394      395       +1

[mmariani]

I would have added a test but I'm pretty new at go and don't see a clean way to mock /dev/stdin.

[dnephin]

Thanks! this is a good start

[dnephin]

I think this would be better in the help text for the --compose-file flag. Something like

Path to a Compose file, use "-" to read from stdin

[mdlinville]

I agree, then it doesn't need to be specifically dealt with in the reference file at all, unless you wanted to add an example (which would probably be good to have).

[dnephin]

I think we can make this easier to test, and supported on windows.

Instead of setting composefile = "/dev/stdin" here, only set WorkingDir.

You'll also need to pass dockerCli.In() as an io.Reader from deployCompose() where this is called, so we have access to it in getConfigFile().

In getConfigFile before calling ioutil.ReadFile() check if filename == "-" and use this instead to get the bytes:

bytes, err = ioutil.ReadAll(stdin)

[mmariani]

this one should do, thanks

[thaJeztah]

Wondering if this should just take an io.Reader in both cases @dnephin wdyt?

[dnephin]

Ya, that sounds like a good idea

[mmariani]

I did not do that, because it needs the filename in the return.

[thaJeztah]

Ah, I see; yes, we may have to refactor some things. None of these are exported functions, so I guess we can do that in a follow up.

[mdlinville]

I agree, then it doesn't need to be specifically dealt with in the reference file at all, unless you wanted to add an example (which would probably be good to have).

[thaJeztah]

one nit, but LGTM otherwise; can you squash your commits? I think it's ok to keep the docs and code changes in the same commit

[thaJeztah]

Looks like the indentation got lost here. We're not testing if the content is actually parsed, so won't make a difference (currently); I see the other test also doesn't do this, but wondering if we should, e.g.;

diff --git a/cli/command/stack/deploy_composefile_test.go b/cli/command/stack/deploy_composefile_test.go
index 90d866a2..6e2a1f2c 100644
--- a/cli/command/stack/deploy_composefile_test.go
+++ b/cli/command/stack/deploy_composefile_test.go
@@ -37,15 +37,16 @@ func TestGetConfigDetailsStdin(t *testing.T) {
        content := `
 version: "3.0"
 services:
-foo:
-image: alpine:3.5
+  foo:
+    image: alpine:3.5
 `
        details, err := getConfigDetails("-", strings.NewReader(content))
        require.NoError(t, err)
        cwd, err := os.Getwd()
        require.NoError(t, err)
        assert.Equal(t, cwd, details.WorkingDir)
-       assert.Len(t, details.ConfigFiles, 1)
+       require.Len(t, details.ConfigFiles, 1)
+       assert.Equal(t, "3.0", details.ConfigFiles[0].Config["version"])
        assert.Len(t, details.Environment, len(os.Environ()))
 }

[vdemeester]

SGTM
@mmariani can you take care of @thaJeztah's comment (and squash your commits) ? 👼

[mmariani]

done, thanks for reminding. also updated to master

[dnephin]

LGTM