Skip to content

Conversation

@ulyssessouza
Copy link
Contributor

What I did
Do not try to guess when to allocate a TTY and keep it as default

Related issue
Resolves #8908

@ulyssessouza ulyssessouza force-pushed the fix-out-with-redirection branch 4 times, most recently from eb418ff to 5321188 Compare December 14, 2021 18:52
@ulyssessouza ulyssessouza marked this pull request as ready for review December 14, 2021 19:02
@ulyssessouza ulyssessouza requested a review from ndeloof December 14, 2021 19:02
@ndeloof
Copy link
Contributor

ndeloof commented Dec 15, 2021

I can't find a better fix for this, but still sad we have to give up with tty detection, and wonder this will trigger more issues from users who expect compose to automatically adjust to environment :-/

@ulyssessouza ulyssessouza force-pushed the fix-out-with-redirection branch from 5321188 to 63fcd1a Compare December 15, 2021 09:45
@ulyssessouza
Copy link
Contributor Author

@ndeloof Yep. I also tried to save the automatic behaviour, but without success...
But at least the possible issues will have just a "-T" as "fix"

Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SGTM

just to double-check; this brings it to the same behaviour / defaults as compose v1, correct?

@rfay
Copy link
Contributor

rfay commented Mar 8, 2022

Since panic is a result of this I don't think it's probably satisfactory, see https://stackoverflow.com/questions/70855915/fix-panic-provided-file-is-not-a-console-from-docker-compose-in-github-action

@ndeloof
Copy link
Contributor

ndeloof commented Mar 8, 2022

I don't like this either. I expect we get rid of this as we share more code with docker/cli, especially relying on the exact same RunExec function

@rfay
Copy link
Contributor

rfay commented Mar 8, 2022

Right... but not preventing a panic? that doesn't seem like the right thing...

@ndeloof
Copy link
Contributor

ndeloof commented Mar 8, 2022

We can burn some cycles trying to find a better way to manage this and avoid a panic, which is obviously the worst possible user experience to offer :'(
but with #9198 and docker/cli#3436 we are close to just drop all the custom exec code implemented by docker/compose to adopt the battle-tested RunExec code used by the docker CLI for years.

@rfay
Copy link
Contributor

rfay commented Mar 8, 2022

Sounds good. I had actually planned long ago and half-implemented a switch to using docker's exec for ddev exec instead of using docker-compose exec but didn't actually pull the trigger, just worried about unexpected results.

@rostislav-simonik-plc
Copy link

Hello, i just tested with version 2.6.1 of docker compose and still having the issue. Once i remove exec with tee it starts working.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

docker compose run hangs indefinitely if stdout/err were duplicated to a file

5 participants