Re: [PATCH] contrib: add credential helper for libsecret

From: Dennis Kaarsemaker <dennis@kaarsemaker.net>
Date: Oct 11, 2016, 20:32 UTC
Message-ID: <1476217934.3876.12.camel@kaarsemaker.net>
In-Reply-To: <xmqqa8ea8wzy.fsf@gitster.mtv.corp.google.com>

On Tue, 2016-10-11 at 13:13 -0700, Junio C Hamano wrote:

Show 22 quoted lines

> Dennis Kaarsemaker <dennis@kaarsemaker.net> writes:
> 
> > On Tue, 2016-10-11 at 22:48 +0300, Mantas Mikulėnas wrote:
> > > On 2016-10-11 22:36, Junio C Hamano wrote:
> > > > Thanks for a review.  I'll wait until one of (1) a squashable patch
> > > > to address the "we do not want unconditional overwrite" issue, (2) a
> > > > reroll from Mantas to do the same, or (3) a counter-argument from
> > > > somebody to explain why unconditional overwrite is a good idea here
> > > > (but not in the original) appears.
> > > 
> > > 
> > > 
> > > I overlooked that. I can write a patch, but it shouldn't make any
> > > difference in practice – if c->username *was* set, then it would also
> > > get added to the search attribute list, therefore the search couldn't
> > > possibly return any results with a different username anyway.
> > 
> > 
> > Makes sense, so a (3) it is.
> 
> 
> So... does it mean the gnome-keyring one needs a bugfix?

I'd say both behaviours are correct.

When you do a search without a username, both helpers fill in the username returned by the actual credential storage. When you do a search with a username, the gnome-keyring helper won't overwrite the value passed in and the libsecret helper overwrites it with the same value, as the search can only return matches that have the same value.

D.