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

Can't launch a command line process from Qt #1196

Closed
MichaelMure opened this issue May 4, 2015 · 11 comments
Closed

Can't launch a command line process from Qt #1196

MichaelMure opened this issue May 4, 2015 · 11 comments

Comments

@MichaelMure
Copy link
Contributor

When launched from Qt with a QProcess, the command line tool wrongly detect an extra arguments and output the help text instead of running the command.

Error: Expected 0 arguments, got 1: []

Here is the code used to launch the process: https://github.com/MichaelMure/Arbore-qt/blob/d9d755db95447bc9b618b5d141402dcefc37cde9/src/ipfs/ipfs.cpp#L75-L78

Using strace, the syscall seems to be the same when launching from Qt and when using the command line:

from Qt

execve("/home/michael/go/bin/ipfs", ["ipfs", "daemon", "--init"], [/* 47 vars */]) = 0 `

from bash

execve("/home/michael/go/bin/ipfs", ["ipfs", "daemon", "--init"], [/* 46 vars */]) = 0

The problem seems to be this line: https://github.com/ipfs/go-ipfs/blob/29c6a53e435a9ca71aaac50daa3078b531582c2e/commands/cli/parse.go#L225

When commenting this line, the launch works as expected from Qt.

@jbenet
Copy link
Member

jbenet commented May 4, 2015

This is super annoying. We should make this a higher priority to fix.


Sent from Mailbox

On Mon, May 4, 2015 at 8:16 AM, Michael Muré [email protected]
wrote:

When launched from Qt with a QProcess, the command line tool wrongly detect an extra arguments and output the help text instead of running the command.

Error: Expected 0 arguments, got 1: []
Here is the code used to launch the process: https://github.com/MichaelMure/Arbore-qt/blob/d9d755db95447bc9b618b5d141402dcefc37cde9/src/ipfs/ipfs.cpp#L75-L78
Using strace, the syscall seems to be the same when launching from Qt and when using the command line:
from Qt
execve("/home/michael/go/bin/ipfs", ["ipfs", "daemon", "--init"], [/* 47 vars /]) = 0 `
from bash
execve("/home/michael/go/bin/ipfs", ["ipfs", "daemon", "--init"], [/
46 vars */]) = 0
The problem seems to be this line: https://github.com/ipfs/go-ipfs/blob/29c6a53e435a9ca71aaac50daa3078b531582c2e/commands/cli/parse.go#L225

When commenting this line, the launch works as expected from Qt.

Reply to this email directly or view it on GitHub:
#1196

@chriscool
Copy link
Contributor

Maybe I can have a look at fixing this at the same time as issue #1141 (ipfs cat "multihash too short" error when using stdin).

There is also PR #1193 that is a good first step for fixing both of the above issues.

@MichaelMure
Copy link
Contributor Author

Might be related to #858

@whyrusleeping
Copy link
Member

the issue here is that we count stdin as an argument unless stdin is your terminal, so in any other case, we grab it as the argument. I ran into this when i was working on the pipe command and @travisperson and I decided the best way forward was to split up the argument types into 'file arg' and a new 'stream arg' which is for stdin. This is a hard problem to solve (thus why pipe is not merged/finished yet)

@jbenet
Copy link
Member

jbenet commented May 4, 2015

#1193 is now merged.

decided the best way forward was to split up the argument types into 'file arg' and a new 'stream arg' which is for stdin.

sgtm. semantically, cmds like "ipfs add" can check first for file args, and otherwise default to stream?

@whyrusleeping
Copy link
Member

Yeah, although it still doesnt solve the issue entirely. even with that change, we still run into the issue where we have a stream argument, and stdin is not a terminal, so we assume there is data. There needs to be a way to check whether or not there is actually data coming through stdin.. @travisperson any ideas?

@wking
Copy link
Contributor

wking commented May 4, 2015

On Mon, May 04, 2015 at 04:33:53PM -0700, Jeromy Johnson wrote:

Yeah, although it still doesnt solve the issue entirely. even with
that change, we still run into the issue where we have a stream
argument, and stdin is not a terminal, so we assume there is
data. There needs to be a way to check whether or not there is
actually data coming through stdin.

We shouldn't care if stdin is a terminal (we could have data coming in
on stdin even if it was a terminal). @jbenet's suggestion (“if we
don't have file args, read from stdin”) sounds like the either-or
suggestions in #1141 and is unambiguous. No heuristics needed.

@whyrusleeping
Copy link
Member

that doesnt solve the problem of knowing whether or not there is data coming through stdin, and whether or not you should wait for it/try to read it.

@wking
Copy link
Contributor

wking commented May 5, 2015

On Mon, May 04, 2015 at 05:42:21PM -0700, Jeromy Johnson wrote:

that doesnt solve the problem of knowing whether or not there is
data coming through stdin, and whether or not you should wait for
it/try to read it.

Why not? If there are argv arguments for whatever you might might
read from stdin, then you shouldn't try to read from stdin. For ‘ipfs
daemon’ that looks like “if there are any argv entries at all after
‘daemon’, then don't read from stdin”.

chriscool added a commit that referenced this issue May 17, 2015
This should fix issue #1196 (Can't launch a command line
process from Qt).

The check was bad because it took stdin into account,
but it really shouldn't.

License: MIT
Signed-off-by: Christian Couder <[email protected]>
@chriscool
Copy link
Contributor

This should be fixed as PR #1238 as been merged.

@MichaelMure
Copy link
Contributor Author

I confirm, it works now.

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

No branches or pull requests

5 participants