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

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 28, 2020, 23:13 UTC
Message-ID: <xmqqr1w7yzjc.fsf@gitster.c.googlers.com>
In-Reply-To: <nycvar.QRO.7.76.6.2004250027550.18039@tvgsbejvaqbjf.bet>

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

Show 33 quoted lines

> 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.

Thanks.