Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 actualdocker-compose *
commands are executed and the source of the issue is thatdocker-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 yieldIs TTY
; for scenarios where a dumb terminal or similar is expected this should yieldIs non-TTY
.Workaround
The possible workaround would be running the
docker
anddocker-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:
sail composer install && sail test *
to run application tests during a pull request build.first-setup.sh
script,yarn *
commands can be run safely in parallel withcomposer *
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&
andwait
ing or ensuring the host machine has GNU parallel somehow and running them through it.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.: runningsail 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.: becauseexec
is treated as an unknown command by sail, runningecho 'sail exec redis date; echo $?' | bash
will still yieldthe input device is not a TTY
but runningecho '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:
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 directoryvendor/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 😁)