Prevent git from hanging a process #6140
Conversation
* Use of a timeout when IPC::Run invokes * Enhance error message and notify user for likely solution I tried to catch the prompt but I assume that the process is not running in a tty and IPC::Run could not get the output. Signed-off-by: Ioannis Bonatakis <ybonatakis@suse.com>
| return_code => undef, | ||
| stderr => 'an internal error occurred', | ||
| stdout => '', | ||
| my $additional_info = "If you use http for CASEDIR or NEEDLES_DIR is adviced to |
There was a problem hiding this comment.
run_cmd_with_log_return_error is a general purpose function not only used for git commands.
It doesn't make sense to add a message about git here.
Also, we shouldn't add lengthy directions into the error message, but link to the documentation, as it was suggested in the ticket
| my $additional_info = "If you use http for CASEDIR or NEEDLES_DIR is adviced to | ||
| configure git with the insteadof option in the .gitconfig file. | ||
| Run the following command to: | ||
| git config [--global] url.\"git://github.com\".insteadOf https://github.com"; |
There was a problem hiding this comment.
url.insteadOf is not what we want. it is url.pushInsteadOf, which is an important difference.
We still want the fetch to happen with http.
| my @cmd; | ||
| push @cmd, 'env', 'GIT_SSH_COMMAND=ssh -oBatchMode=yes' if $ssh_batchmode; | ||
| push @cmd, 'env', 'GIT_SSH_COMMAND=ssh -oBatchMode=yes' if $batchmode; |
There was a problem hiding this comment.
Why did you not use GIT_ASKPASS and GIT_TERMINAL_PROMPT like suggested in the ticket?
Waiting for a timing out command seems way more complicated than just telling git to not prompt in the first place.
There was a problem hiding this comment.
I was thinking that with the timeout we gonna have a general solution when the _run_cmd (or more specifically run_cmd_with_log_return_error) encounter similar issue.
GIT_ASKPASS and GIT_TERMINAL_PROMPT make the logic more complicated IMO but also provide a better error as I didnt find a way to catch the prompt in the $stdout somehow.
Maybe both can be applied at the end.
There was a problem hiding this comment.
I'm not sure what's complicated about using two env vars. It's as simple as it gets?
I didnt notice and I didnt know about this. Again two ppl work on the same thing |
This is not merged tho. what do you mean? |
|
closed in favor of #6141 |
I tried to catch the prompt but I assume that the process is not running in a tty and IPC::Run could not get the output.
https://progress.opensuse.org/issues/174592