Re: [PATCH] compat/terminal: mark parameter of git_terminal_prompt() UNUSED

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 29/08/2024 23:26, Jeff King wrote:
> On Thu, Aug 29, 2024 at 10:57:50PM +0100, Ramsay Jones wrote:
> 
>> The 'seen' branch fails to compile on cygwin (but its fine on Linux), due
>> to an unused parameter. I haven't looked too hard at the code (at first
>> blush, it seemed to me that it should not even be trying to compile that
>> code, but ...), I simply added an UNUSED to fix the build. ;)
>>
>> So, this may not be the correct 'fix' for this, but I thought I should
>> report it here, since I don't have time to look into this now. sorry! :(
> 
> Thanks, this is definitely the right fix. I have to rely on CI for
> catching cases outside of what I build locally, and it looks like we
> don't trigger this fallback code at all in CI (we hit HAVE_DEV_TTY for
> Linux and macOS, and then GIT_WINDOWS_NATIVE for Windows).
> 
> Here's a potential commit message for the patch:
> 
>   If neither HAVE_DEV_TTY nor GIT_WINDOWS_NATIVE is set, the fallback
>   code calls the system getpass(). This unfortunately ignores the "echo"
>   boolean parameter, as we have no way to implement that functionality.
>   But we still have to keep the unused parameter, since our interface
>   has to match the other implementations.

Yes, this reads well. Do you want to send an updated patch or shall I?

> As an aside, I wonder if cygwin could be using either /dev/tty or the
> Windows variant. But that's obviously a separate patch, and either way
> we'd want to fix this fallback code in the meantime.

Yes, this is what I meant by '... it should not even be trying to compile
that code ...' ;) ie I was expecting HAVE_DEV_TTY to be set on cygwin (which
does have /dev/tty).

However, there may be reasons for it not being set - I haven't had time to
look into it yet.

ATB,
Ramsay Jones






[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux