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

From: Dennis Kaarsemaker <[email-masked]>
Date: Tue, 11 Oct 2016 22:32:14 +0200

On Tue, 2016-10-11 at 13:13 -0700, Junio C Hamano wrote:
> Dennis Kaarsemaker <[email-masked]> 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.