Re: [PATCH v3 3/3] credential: handle `credential.<partial-URL>.<key>` again

[Johannes Schindelin <Johannes.Schindelin@gmx.de>]

Hi Junio,

On Tue, 28 Apr 2020, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
> > Hi Junio,
> >
> > On Fri, 24 Apr 2020, Junio C Hamano wrote:
> >
> >> "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
> >> writes:
> >>
> >> > @@ -53,7 +57,13 @@ static int credential_config_callback(const char *var, const char *value,
> >> >  		char *url = xmemdupz(key, dot - key);
> >> >  		int matched;
> >> >
> >> > -		credential_from_url(&want, url);
> >> > +		if (credential_from_potentially_partial_url(&want, url) < 0) {
> >> > +			warning(_("skipping credential lookup for key: %s"),
> >> > +				var);
> >> > +			credential_clear(&want);
> >> > +			free(url);
> >> > +			return 0;
> >> > +		}
> >> >  		matched = credential_match(&want, c);
> >>
> >> Unfortunately, the whole section of this code that is being patched
> >> here goes away with 46fd7b39 (credential: allow wildcard patterns
> >> when matching config, 2020-02-20), that delegates large part of the
> >> logic to urlmatch.
> >>
> >> Dscho and Brian, how do we want to proceed?  As the conflicting
> >> change has already been in 'next' for more than a month and a half,
> >> we'd need this "partial URL" logic build to work well with it.
> >
> > I prepared a PR: https://github.com/gitgitgadget/git/pull/620. I do not
> > necessarily feel 100% comfortable with that approach yet, but at least it
> > lets the new test case of t0300 pass.
>
> The -2.17 patch series (your [v3 3/3]) ends t0300 like so
>
>     +	done &&
>     +
>     +	git -c credential.$partial.helper=yep \
>     +		-c credential.with%0anewline.username=uh-oh \
>     +		credential fill <stdin >stdout 2>stderr &&
>     +	test_i18ngrep "skipping credential lookup for key" stderr
>     +
>     +'
>     +
>      test_done
>
> while the other branch lacks the extra blank line just before the
> single quote is closed.  I queued 850ef627 (SQUASH??? lose excess
> blank line to match the other side of the eventual merge,
> 2020-04-24) on top so that the "-s ours" merge would be without
> unnecessary evil-merge noise.  You probably would want to squash it
> in.

Thank you for pointing that out. I already did that:
https://github.com/gitgitgadget/git/compare/0535908dd7ea4487b342c0f86182579279c57b34..d59738ecf741a5fddd70f133eaaf69768e245bcc

Do you want me to send another iteration, for completeness' sake?

Ciao,
Dscho