Re: [PATCH v3 3/3] credential: handle `credential.<partial-URL>.<key>` again
- From
Johannes Schindelin <johannes.schindelin@gmx.de>
- Date
- Apr 25, 2020, 10:43 UTC
- Message-ID
- <nycvar.QRO.7.76.6.2004251238590.18039@tvgsbejvaqbjf.bet>
- In-Reply-To
- <20200424152307.GA85188@Carlos-MBP>
Hi Carlo,
On Fri, 24 Apr 2020, Carlo Marcelo Arenas Belón wrote:
Show 19 quoted lines
> On Fri, Apr 24, 2020 at 11:49:52AM +0000, Johannes Schindelin via GitGitGadget wrote:
> > diff --git a/credential.c b/credential.c
> > index 7dbbf26f174..c1a9ca4e485 100644
> > --- a/credential.c
> > +++ b/credential.c
> > @@ -35,6 +35,10 @@ int credential_match(const struct credential *want,
> > #undef CHECK
> > }
> >
> > +
> > +static int credential_from_potentially_partial_url(struct credential *c,
> > + const char *url);
> > +
> > static int credential_config_callback(const char *var, const char *value,
> > void *data)
> > {
>
> something like credential_from_url_partial might had been more grep friendly
> but this would work as well.It might be more grep'able, but it sounds really awkward to me, that's why I did not use that name (it was my first candidate).
Show 19 quoted lines
> > diff --git a/t/t0300-credentials.sh b/t/t0300-credentials.sh > > index efed3ea2955..f796bbfd48b 100755 > > --- a/t/t0300-credentials.sh > > +++ b/t/t0300-credentials.sh > > @@ -448,4 +448,43 @@ test_expect_success 'credential system refuses to work with missing protocol' ' > > test_i18ncmp expect stderr > > ' > > > > +test_expect_success 'credential config with partial URLs' ' > > + echo "echo password=yep" | write_script git-credential-yep && > > + test_write_lines url=https://user@example.com/repo.git >stdin && > > + for partial in \ > > + example.com \ > > + user@example.com \ > > even if it works, configurations with a username might not be worth the > trouble to support IMHO > > maybe better not to include them in the tests then either?
Let me counter this:
- It would take extra code _to prevent_ the username from being used, and
- There is precedent where the user name _does_ matter: it is relatively normal to access different orgs' repositories at https://dev.azure.com/<org>/<repo>/_git via different accounts.
Together, those points convince me that special-casing the username _and explicitly ignoring it_ would not make sense.
> other than that, like the previous version (which is functionally equivalent) > should be IMHO good to go.
Thank you for reviewing it!
Ciao, Dscho