Re: [PATCH v3 3/3] credential: handle `credential.<partial-URL>.<key>` again
- From
Johannes Schindelin <johannes.schindelin@gmx.de>
- Date
- Apr 29, 2020, 14:58 UTC
- Message-ID
- <nycvar.QRO.7.76.6.2004291657220.18039@tvgsbejvaqbjf.bet>
- In-Reply-To
- <xmqqr1w7yzjc.fsf@gitster.c.googlers.com>
Hi Junio,
On Tue, 28 Apr 2020, Junio C Hamano wrote:
Show 55 quoted lines
> 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