Fix incompatible in TestComposeExecTTY
#2650
Conversation
yankay
force-pushed
the
remove-incompatible
branch
2 times, most recently
from
915c3f5
to
27b72f7
Compare
yankay
changed the title
TestComposeExecTTYTestComposeExecTTY
yankay
force-pushed
the
remove-incompatible
branch
2 times, most recently
from
016808c
to
9f6e5f2
Compare
| } | ||
|
|
||
| composeExecCommand.Flags().MarkHidden("interactive") | ||
| composeExecCommand.Flags().MarkHidden("tty") |
There was a problem hiding this comment.
I guess we can just remove --tty
| return nil | ||
| } | ||
|
|
||
| composeExecCommand.Flags().MarkHidden("interactive") |
There was a problem hiding this comment.
This doesn't seem to exist in Docker Compose, so we can just remove it
| @@ -165,7 +165,6 @@ services: | |||
| func TestComposeExecTTY(t *testing.T) { | |||
| // `-i` in `compose run & exec` is only supported in compose v2. | |||
| // Currently CI is using compose v1. | |||
There was a problem hiding this comment.
Docker Compose v2 doesn't seem to have -i
| @@ -165,7 +165,6 @@ services: | |||
| func TestComposeExecTTY(t *testing.T) { | |||
| // `-i` in `compose run & exec` is only supported in compose v2. | |||
| // Currently CI is using compose v1. | |||
There was a problem hiding this comment.
" Currently CI is using compose v1" is no longer true
There was a problem hiding this comment.
Thanks , it has changed. :-)
| @@ -69,7 +60,7 @@ func composeExecAction(cmd *cobra.Command, args []string) error { | |||
| if err != nil { | |||
| return err | |||
| } | |||
| tty, err := cmd.Flags().GetBool("tty") | |||
| noTty, err := cmd.Flags().GetBool("no-TTY") | |||
There was a problem hiding this comment.
Shouldn't we still check --tty too?
It is hidden in Docker Compose v2, but it still exists
https://github.com/docker/compose/blob/b1a26dac1d3dc201d0d026cc16cb9baadaacffd7/cmd/compose/exec.go#L77
There was a problem hiding this comment.
HI @AkihiroSuda
Thanks for the PR review.
When using docker compose exec -t=true --no-TTY, the docker do not check the --tty.
root@kay200:/tmp# docker compose -f a.yaml exec --tty=true --no-TTY svc0 stty
stty: standard input: Not a tty
So it be removed to keep compatibility :-)
There was a problem hiding this comment.
👀 Could you add a code comment to explain that behavior?
There was a problem hiding this comment.
Thanks
The comments has been on it :-)
yankay
force-pushed
the
remove-incompatible
branch
2 times, most recently
from
6e29251
to
afe990d
Compare
Signed-off-by: Kay Yan <kay.yan@daocloud.io>
fix: #2639
using similar code in https://github.com/docker/compose/blob/v2.23.1/cmd/compose/exec.go#L77