RFC patchconfig.mak.uname: add HAVE_DEV_TTY to cygwin config section

Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
---

Hi,

I found a few minutes to try building with HAVE_DEV_TTY on cygwin tonight and had a quick test:

  $ printf "%s\n" protocol=https host=example.com path=git | ./git credential fill
  Username for 'https://example.com': user
  Password for 'https://user@example.com': 
  protocol=https
  host=example.com
  username=user
  password=pass
  $ 

So, that seems to work. Actually, when I first tried, it didn't work at all, because I typed 'git credential ...' not './git credential ...'! :)

Also, somewhat surprising, is that job control also works:

  $ printf "%s\n" protocol=https host=example.com path=git | ./git credential fill
  Username for 'https://example.com': 
  [1]+  Stopped                 printf "%s\n" protocol=https host=example.com path=git | ./git credential fill
  $ echo hhh
  hhh
  $ fg
  printf "%s\n" protocol=https host=example.com path=git | ./git credential fill
  user
  Password for 'https://user@example.com': 
  protocol=https
  host=example.com
  username=user
  password=pass
  $ 

The only difference between Linux and cygwin seems to be that cygwin does not echo ^Z when back-grounding.

Still, I need to do a full test suite run, just to check for any regressions. (Unfortunately, that takes about 6 hours to run, so I can't get to that soon).

Also, I should check other uses of these routines:

  $ git grep -n read_key_without_echo
  add-patch.c:1226:               int res = read_key_without_echo(&s->answer);
  compat/terminal.c:535:int read_key_without_echo(struct strbuf *buf)
  compat/terminal.c:602:int read_key_without_echo(struct strbuf *buf)
  compat/terminal.h:25:int read_key_without_echo(struct strbuf *buf);
  $ 

  $ git grep -n git_terminal_prompt
  compat/terminal.c:428:char *git_terminal_prompt(const char *prompt, int echo)
  compat/terminal.c:597:char *git_terminal_prompt(const char *prompt, int echo UNUSED)
  compat/terminal.h:22:char *git_terminal_prompt(const char *prompt, int echo);
  prompt.c:65:                    r = git_terminal_prompt(prompt, flags & PROMPT_ECHO);
  $ 

  $ git grep -n git_prompt
  builtin/bisect.c:401:           yesno = git_prompt(_("Are you sure [Y/n]? "), PROMPT_ECHO);
  builtin/bisect.c:913:   yesno = git_prompt(_("Do you want me to do it for you "
  credential.c:247:       r = git_prompt(prompt.buf, flags);
  help.c:712:                     answer = git_prompt(msg.buf, PROMPT_ECHO);
  prompt.c:45:char *git_prompt(const char *prompt, int flags)
  prompt.h:7:char *git_prompt(const char *prompt, int flags);
  $ 
  
So, 'add-patch', bisect and help (in addition to git-credential).

[Note: save_term() and restore_term() are not called outside of that TU, so they could be marked static!]

Anyhow, just a quick heads up.

ATB, Ramsay Jones

 config.mak.uname | 1 +
 1 file changed, 1 insertion(+)

On Mon, Sep 02, 2024 at 11:15:35PM +0100, Ramsay Jones wrote:

I'm not sure if we cover this at all in the test suite, since it implies access to an actual terminal. All of the auth tests rely on the askpass interface to simulate the user typing a password.

We do have test_terminal, which simulates a pty, but I don't think it would work for this case. For one thing, I don't know that it counts as the controlling terminal, so opening /dev/tty wouldn't find it. And two, the stdin handling had race problems that caused us to remove it recently. So it's really only good for checking isatty() for stdout/stderr.

On the plus side, if it works for you in a single manual test then I suspect that's enough. The key thing is really "can we get something tty-ish that responds to termios", and it sounds like you can.

Certainly doesn't hurt to test the single-key mode of "add -p", etc, though.

Thanks for looking into it. :)

-Peff

On 05/09/2024 11:43, Jeff King wrote:

Heh, yes I suspect you are correct - it's just that I have been tripped up too many times thinking "well, these changes can't possibly affect the test-suite ..." :)

Anyway, I confirmed tonight that there are no regressions noticed by the test-suite. (Which is not to say there aren't any ;) ).

Yeah, I still need to check the 'other uses' out. Hmm, I have never used 'add -p' (never felt the need), so I guess I will have to read up on how to use it ...

ATB, Ramsay Jones

On Sat, Sep 07, 2024 at 12:53:11AM +0100, Ramsay Jones wrote:

Try:

  git init
  echo foo >file
  git add file
  echo bar >file
  git -c interactive.singleKey=true add -p

Hitting 'y' or 'n' should immediately be accepted (and exit the program either with the hunk staged or not), whereas I suspect it would not without HAVE_DEV_TTY.

-Peff

On 07/09/2024 07:21, Jeff King wrote:

Oh, that is much better! Everything works exactly as described and is a much better user experience than:

    "warning: reading single keystrokes not supported ..."

:)

Looking good.

Thanks.

ATB, Ramsay Jones