From: Jeff King <peff@peff.net>
To: "Carlo Marcelo Arenas Belón" <carenas@gmail.com>
Cc: git@vger.kernel.org, jrnieder@gmail.com
Subject: Re: [RFC PATCH] credential: minor documentation fixes
Date: Mon, 4 May 2020 10:44:36 -0400 [thread overview]
Message-ID: <20200504144436.GA9893@coredump.intra.peff.net> (raw)
In-Reply-To: <20200504074520.GB86805@Carlos-MBP>
On Mon, May 04, 2020 at 12:45:20AM -0700, Carlo Marcelo Arenas Belón wrote:
> On Sun, May 03, 2020 at 02:58:26AM -0400, Jeff King wrote:
> > On Sat, May 02, 2020 at 11:34:23PM -0700, Carlo Marcelo Arenas Belón wrote:
> >
> > > the order of parameters used in credential_match was inconsistent
> > > between credential.c and credential.h as well, so update both to
> > > match the current implementation.
> >
> > Yes, looks like this has been wrong since the beginning in 118250728e
> > (credential: apply helper config, 2011-12-10). I checked the callers to
> > make sure none of them had gotten it backwards, but they all look right
> > (and just the declaration is wrong).
>
> thanks for checking, will include this (and your typo fix) in the
> submission; should I add your "Reviewed-by" then?
Sure, feel free to.
> some things that I think might need clarification (or maybe even code changes
> after agreed on) are :
>
> * the meaning of "exactly" for matching protocol and hostname in the URL
> since 06 are both case insensitive per RFC3986 and we have been
> ambiguous on that, leading to some helpers assuming case or encoding.
Yeah, IIRC we discussed case-sensitivity at the time and went with the
stricter behavior in the name of safety over convenience. And I don't
think anybody has complained since then. So I'm not really _opposed_ to
loosening it to match the URL, but perhaps a maintenance release is not
the best time to do so.
> * the rules for how helper matching are expected to be ordered, specially
> considering the recent adding of wildcard matching and the revival of
> partial matching, and the fact that the order is relevant for both
> discovery of credentials and which helpers are used.
Yes, this could be better documented. I think the current rules are
probably not ideal, especially when you mix credential.*.helper and
credential.helper. So some fixes are possible there, but I think they
might be best added as new feature (e.g., a new config like
credential.*.helperOrder that says whether and how to use
non-url-specific helpers; or something like that).
> * the use of hostname as a key, since the addition of cert:// that has none
> and uses path instead (had emailed the author privately for clarification,
> but hadn't heard yet) and the effect that has on which fields are expected
> and which values are valid.
Yeah, there could be more discussion in gitcredentials(7) there.
> * the role of overrides (ex: the documented example of passing URL and later
> updating it seems useful, eventhough I am not aware if being used)
I'd be OK to see this feature removed. I have used it for various
debugging or experimenting scenarios, but Git in general would never
pass anything except a broken-down set of fields, and only one of each
field.
> * clarification on which fields can be updated by the helper; currently I
> don't think we allow overrides for protocol and host and assume all others
> but the documentation doesn't clarify, and that might be a problem for
> cert:// where file is more relevant.
I think we do allow a helper to transform a credential in any way:
echo url=https://example.com/ |
git \
-c credential.helper='!sed >&2 s/^/one:/; echo host=other.example.com;:' \
-c credential.helper='!sed >&2 s/^/two:/;:' \
credential fill
produces:
one:protocol=https
one:host=example.com
two:protocol=https
two:host=other.example.com
Username for 'https://other.example.com':
So after the first helper runs, subsequent helpers (and the internal
terminal prompt) will consider the modified hostname.
Now whether that's a sane feature or not, I'm not sure.
-Peff
next prev parent reply other threads:[~2020-05-04 14:44 UTC|newest]
Thread overview: 29+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-05-03 6:34 [RFC PATCH] credential: minor documentation fixes Carlo Marcelo Arenas Belón
2020-05-03 6:58 ` Jeff King
2020-05-04 7:45 ` Carlo Marcelo Arenas Belón
2020-05-04 14:44 ` Jeff King [this message]
2020-05-04 15:39 ` Carlo Marcelo Arenas Belón
2020-05-04 16:10 ` Jeff King
2020-05-04 15:58 ` Carlo Marcelo Arenas Belón
2020-05-04 16:13 ` Jeff King
2020-05-05 1:39 ` [PATCH 0/4] credential: documentation updates for maint Carlo Marcelo Arenas Belón
2020-05-05 1:39 ` [PATCH 1/4] credential: update description for credential_from_url_gently Carlo Marcelo Arenas Belón
2020-05-05 1:39 ` [PATCH 2/4] credential: correct order of parameters for credential_match Carlo Marcelo Arenas Belón
2020-05-05 1:39 ` [PATCH 3/4] credential: update gitcredentials documentation Carlo Marcelo Arenas Belón
2020-05-06 16:21 ` Jeff King
2020-05-05 1:39 ` [PATCH 4/4] credential: document protocol updates Carlo Marcelo Arenas Belón
2020-05-06 16:26 ` Jeff King
2020-05-06 16:27 ` [PATCH 0/4] credential: documentation updates for maint Jeff King
2020-05-06 23:28 ` Carlo Marcelo Arenas Belón
2020-05-07 20:59 ` Jeff King
2020-05-07 21:23 ` Carlo Marcelo Arenas Belón
2020-05-07 22:17 ` Jeff King
2020-05-07 23:35 ` Carlo Marcelo Arenas Belón
2020-05-06 21:47 ` [PATCH v2 " Carlo Marcelo Arenas Belón
2020-05-06 21:47 ` [PATCH v2 1/4] credential: update description for credential_from_url_gently Carlo Marcelo Arenas Belón
2020-05-06 21:47 ` [PATCH v2 2/4] credential: correct order of parameters for credential_match Carlo Marcelo Arenas Belón
2020-05-06 21:47 ` [PATCH v2 3/4] credential: update gitcredentials documentation Carlo Marcelo Arenas Belón
2020-05-07 20:54 ` Jeff King
2020-05-07 21:02 ` Junio C Hamano
2020-05-06 21:47 ` [PATCH v2 4/4] credential: document protocol updates Carlo Marcelo Arenas Belón
2020-05-07 20:57 ` Jeff King
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=20200504144436.GA9893@coredump.intra.peff.net \
--to=peff@peff.net \
--cc=carenas@gmail.com \
--cc=git@vger.kernel.org \
--cc=jrnieder@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).