-
-
Notifications
You must be signed in to change notification settings - Fork 3k
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
ifps bootstrap rm --all
fails when pipe to stdin is left open
#2805
Comments
Yeah, we are having problems with the I think we shouldn't using heuristic for stdin but require either no argument or |
@Kubuxu I think youre right. Or we can be a little better about checking our input, instead of assuming "not a terminal" means input, we can check to make sure its a file or a pipe (though i'm not sure what all options there are here) |
IMO if there's any sort of auto-detect (be it pty/pipe/file/etc detection) that changes behavior, there ought be an override on the command-line for odd circumstances where you don't want that. This decouples the optimization of the naive case from the specific case and allows you to get the "just works" amazingness, while preserving specificity for those who want it. |
This should be resolved now |
We're running ipfs through a framework that leaves the file-handle open for a little while the process starts and never sends any data on it. We’re attempting to run
ipfs bootstrap rm --all
usingOpen3.popen2e
(see build_execution for more context). When we run it this way,--all
is interpreted as a multiaddr/peerID format. This seems like it might have something to do with theisTerminal
function incommands/cli/parse.go
. I'm not sure what's going on, and I'm not familiar with go.You can reproduce the problem by running
cat | ipfs bootstrap rm --all
with at least one address in the bootstrap list and then hittingCtrl-D
to send EOF.cat | ipfs bootstrap rm --all Error: invalid ipfs address
cc @mappum for
isTerminal
cc @mtauraso
The text was updated successfully, but these errors were encountered: