ocaml-opam / opam-publish

A tool to ease contributions to opam repositories
https://opam.ocaml.org
Other
40 stars 21 forks source link

Allow users to answer questions asked by ssh when git is called #156

Closed kit-ty-kate closed 1 year ago

kit-ty-kate commented 1 year ago

Fixes #155 cc @johnwhitington

kit-ty-kate commented 1 year ago

The issue was that ssh called by git clone asked if you wanted to add github.com’s key as trusted but opam-publish didn’t pass stdin to the git command so ssh defaulted to its default value of fingerprint and basically never added it as trusted and thus failed.

johnwhitington commented 1 year ago

Ok, it's now fixed. Thanks!

One usability improvment. It's not obvious that the question hasn't been auto-answered by opam-publish, because there is some further output on stdout even before I have typed yes and pressed return:

This key is not known by any other names
Are you sure you want to continue connecting (yes/no/[fingerprint])? - Cloning into '/Users/john/.opam/plugins/opam-publish/repos/ocaml%opam-repository'...
yes
kit-ty-kate commented 1 year ago

Actually this doesn't seem to work at all. ssh uses /dev/tty to get what the user inputs and to output to the console.

That's part of the problem and the solution too.

The problem is that the opam library that opam-publish uses wants to be able to distinguish between the output from a subprogram (e.g. git) and the output from the opam-publish itself. To that end it gets the stdout/stderr of every subprograms and reoutputs them in parallel with a yellow coloured - in front. However ssh outputs things to /dev/tty instead of stdout/stderr so both ssh and opam competes for the console resulting in the weird [...]? - Cloning [...] output.

The same happens for the inputs so your problem was that before you didn't realized that it was a question and simply hit [enter] (which defaults to fingerprint instead of yes). So the solution is that it already works just fine, the only thing is that the UI is confusing.

To my knowledge it is not possible to catch outputs from /dev/tty so if we really want to have the UI not be confusing we'll have to make sure we don't catch the stdout/stderr and that seems to require a change in the opam library if we still want to use the opam library (otherwise we'll have to rewrite it and use Unix instead.