Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

The input device is not a TTY #353

Merged
merged 6 commits into from
Mar 8, 2022
Merged

Conversation

ribeirobreno
Copy link
Contributor

This PR is backwards compatible with the 1.x version to fix an issue where running sail commands under some circumstances will yield a "the input device is not a TTY" error. It is a resubmission of #343 with the merge conflicts resolved, a more detailed description and removing the sail-test.sh script to avoid confusion.

About the issue and the fix

The sail script under vendor/laravel/sail/bin is the place where the actual docker-compose * commands are executed and the source of the issue is that docker-compose tries by default to allocate a pseudo-tty in situations where none is required. Adding a -T flag whenever a non-tty terminal is detected will cause the tool to skip this behavior. See: https://docs.docker.com/compose/reference/exec/ and https://docs.docker.com/compose/reference/run/

To test if your current terminal has TTY or not, you can run: [ -t 0 ] && echo 'Is TTY' || echo 'Is non-TTY'. For regular terminals made for user interaction this should yield Is TTY; for scenarios where a dumb terminal or similar is expected this should yield Is non-TTY.

Workaround

The possible workaround would be running the docker and docker-compose commands directly with all the required flags in the situations where the error is expected, essentially undermining the sail script usefulness.

Cases where this is helpful:

  • while running a build under CI environments. E.g.: using sail composer install && sail test * to run application tests during a pull request build.
  • while using background processes to run sail commands. E.g.: in a first-setup.sh script, yarn * commands can be run safely in parallel with composer * commands in most, if not all, application setups and to achieve this parallelism some of the easiest ways are sending one or both processes to background with & and waiting or ensuring the host machine has GNU parallel somehow and running them through it.
  • any kind of workflow automation where a sail script is expected to run unattended.

Expected effects

This PR will not change the behavior of sail * commands under "TTY terminals", which is the only working scenario right now. E.g.: running sail php -r "echo (new \DateTime)->format(\DateTime::ISO8601) . PHP_EOL;" will yield the current date and time in iso-8601 format with or without this fix.

This PR will not fix running sail unknown-command, the -T flag must be included manually by the developer using such command whenever needed. E.g.: because exec is treated as an unknown command by sail, running echo 'sail exec redis date; echo $?' | bash will still yield the input device is not a TTY but running echo 'sail exec -T redis date; echo $?' | bash will yield the current date.

By design, the expected behaviors for running some of the commands under a non-tty terminal are:

  • Some commands will not output colors, for example, any command which relies on Symfony/Console stream output like artisan and many others like GNU grep. Try it:
    echo 'ls -lha > /tmp/ls-lha.txt && grep "d" /tmp/ls-lha.txt' | bash
    ls -lha > /tmp/ls-lha.txt && grep "d" /tmp/ls-lha.txt
    echo 'sail artisan' | bash
    sail artisan
  • Commands requiring user input will either fail, lock or wait forever to receive such input.

The reasoning for both behaviors is that the uses of scripts under "non-tty" are cron jobs, CI environments, queue workers and other kinds of background or automation processes which should not require human interaction to work anyway. The output of those processes is redirected to logs or other applications where the escape sequences for colors make no sense and make this output unnecessarily difficult to parse.

Testing:

The test shell script was added in a gist to validate the fix:
https://gist.github.com/ribeirobreno/f9dc65563d1a7326f759592dddcceb73

To run this, save the sail-test.sh file in a Laravel application with Sail, under the directory vendor/laravel/sail/bin and run it in a terminal (with TTY).

Tested under

Ubuntu 20.04 (WSL2) and Mac OS High Sierra 10.13.6 (yes, it's old 😁)

@taylorotwell taylorotwell merged commit effd276 into laravel:1.x Mar 8, 2022
@driesvints
Copy link
Member

Thanks @ribeirobreno!

@driesvints
Copy link
Member

@ribeirobreno don you have any idea why this warning pops up?

Screenshot 2022-05-19 at 10 09 00

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.

3 participants