Hi Junio & Peff, On Wed, 30 Sep 2020, Junio C Hamano wrote: > Jeff King <peff@xxxxxxxx> writes: > > > On Mon, Sep 28, 2020 at 01:58:03PM -0700, Junio C Hamano wrote: > > > >> "Nikita Leonov via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes: > >> > >> > From: Nikita Leonov <nykyta.leonov@xxxxxxxxx> > >> > > >> > This commit makes reading process regarding credentials compatible with > >> > 'CR/LF' line ending. It makes using git more convenient on systems like > >> > Windows. > >> > >> I can see why this is a good thing for "store" and the two updated > >> pieces of the test script demonstrate it very well. > >> > >> But it is unclear why and how cache-daemon benefits from this change. > >> That needs to be justified. > > > > I suspect it doesn't need touched, because it is internal to git-daemon. > > But it does handle CRLF for some lines, because the first patch > > modified credential_read(), which the daemon builds on (and which _is_ > > user-facing via git-credential). So there's perhaps an argument that > > these calls should just be made consistent, even though the only one who > > would write them is our matching client. If that is the argument to be > > made, I think it would make sense to do so in a separate patch, since > > there's no functional change. > > > > (I'm also slightly puzzled that anybody on Windows would care about > > credential-cache, since it require unix sockets. But maybe in a world of > > WSL people are actually able to mix the two. I confess I haven't kept up > > with the state of things in Windows). > > True, so some, if not all, parts of these changes start to look more > and more like "I change LF to CRLF purely for political correctness, > even I know nobody sends CRLF in these cases (or "even I do not know > if anybody sends CRLF in these cases", which essentially amounts to > the same thing but may be worse)", needless code churns. Indeed. I did not make the connection that `credential-cache--daemon` uses Unix sockets internally (and hence if you try to use it on Windows, you are greeted with the message "credential-cache--daemon unavailable; no unix socket support"). Will drop this part from the next iteration. Ciao, Dscho
