git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Jeff King <peff@peff.net>
To: Junio C Hamano <gitster@pobox.com>
Cc: Nikita Leonov via GitGitGadget <gitgitgadget@gmail.com>,
	git@vger.kernel.org,
	Johannes Schindelin <johannes.schindelin@gmx.de>,
	Nikita Leonov <nykyta.leonov@gmail.com>
Subject: Re: [PATCH v2 2/3] credentials: make line reading Windows compatible
Date: Mon, 28 Sep 2020 20:35:19 -0400	[thread overview]
Message-ID: <20200929003519.GB898702@coredump.intra.peff.net> (raw)
In-Reply-To: <xmqqk0wdliuc.fsf@gitster.c.googlers.com>

On Mon, Sep 28, 2020 at 01:58:03PM -0700, Junio C Hamano wrote:

> "Nikita Leonov via GitGitGadget" <gitgitgadget@gmail.com> writes:
> 
> > From: Nikita Leonov <nykyta.leonov@gmail.com>
> >
> > 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).

-Peff

  reply	other threads:[~2020-09-29  0:35 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-02-14 13:49 [PATCH] credential.c: fix credential reading with regards to CR/LF Johannes Schindelin via GitGitGadget
2020-02-14 17:55 ` Junio C Hamano
2020-02-14 18:32 ` Jeff King
2020-09-28 11:40 ` [PATCH v2 0/3] Prepare git credential to read input with DOS line endings Johannes Schindelin via GitGitGadget
2020-09-28 11:40   ` [PATCH v2 1/3] credential.c: fix credential reading with regards to CR/LF Nikita Leonov via GitGitGadget
2020-09-29  0:42     ` Jeff King
2020-10-02 11:37       ` Johannes Schindelin
2020-10-02 12:01         ` Jeff King
2020-10-02 12:27           ` Johannes Schindelin
2020-10-02 16:32         ` Junio C Hamano
2020-10-03 13:28           ` Johannes Schindelin
2020-09-28 11:40   ` [PATCH v2 2/3] credentials: make line reading Windows compatible Nikita Leonov via GitGitGadget
2020-09-28 20:58     ` Junio C Hamano
2020-09-29  0:35       ` Jeff King [this message]
2020-09-30 22:33         ` Junio C Hamano
2020-10-02  7:53           ` Johannes Schindelin
2020-09-28 23:26     ` Carlo Arenas
2020-09-28 23:41       ` Junio C Hamano
2020-09-29  0:30       ` Jeff King
2020-09-29  0:41         ` Junio C Hamano
2020-09-29  0:44           ` Jeff King
2020-09-29  0:54             ` Junio C Hamano
2020-09-29  3:00               ` Jeff King
2020-09-30 22:25                 ` Junio C Hamano
2020-09-30 22:39                   ` Jeff King
2020-09-30 22:56                     ` Junio C Hamano
2020-10-01 13:54                       ` Jeff King
2020-10-01 15:42                         ` Junio C Hamano
2020-10-02  8:07                           ` Johannes Schindelin
2020-09-28 11:40   ` [PATCH v2 3/3] docs: make notes regarding credential line reading Nikita Leonov via GitGitGadget
2020-09-28 20:31   ` [PATCH v2 0/3] Prepare git credential to read input with DOS line endings Junio C Hamano
2020-10-03 13:29   ` [PATCH v3] credential: treat CR/LF as line endings in the credential protocol Johannes Schindelin via GitGitGadget

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200929003519.GB898702@coredump.intra.peff.net \
    --to=peff@peff.net \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=gitster@pobox.com \
    --cc=johannes.schindelin@gmx.de \
    --cc=nykyta.leonov@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).