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

Avoid trying to read from stdin where this is not useful #543

Open
wants to merge 1 commit into
base: devel
Choose a base branch
from

Conversation

karenetheridge
Copy link

The --help, --version, --self-upgrade commands do not take additional
arguments, so it makes no sense to read from stdin for them. Otherwise,
we will block forever trying to read from stdin when doing
cpanm --version inside a remote script (where -t STDIN is false).

To test, yes | cpanm --version no longer hangs.

The --help, --version, --self-upgrade commands do not take additional
arguments, so it makes no sense to read from stdin for them.  Otherwise,
we will block forever trying to read from stdin when doing
'cpanm --version' inside a remote script (where -t STDIN is false).

To test, 'yes | cpanm --version' no longer hangs.
@zakame
Copy link

zakame commented Jun 29, 2017

On a related note, is the !-t STDIN check useful here? I frequently run Emacs eshell (where TERM=dumb and pipes to STDIN opens to a tty) so something like echo Moose | cpanm won't work on such shells.

Removing !-t STDIN would make it work; let me know if its something so I could make a new issue/PR if needed.

Edit: mixed up my checks here...

@miyagawa
Copy link
Owner

miyagawa commented Jul 2, 2017

@zakame removing that check would make cpanm block on input if you run from a shell.

In a hindsight this "read from STDIN" should be on its own option rather than DWIMing it.

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