Because Mike's original runas patch broke fedpkg (and possible other
pyrpkg clients) I asked him to send fix for that. He haven't sent it to
buildsys mailing list, so forwarding it here. Patch is applied and
pushed upstream now.
-------- Forwarded Message --------
Subject: Re: rebased rpkg patches
Date: Wed, 03 Dec 2014 11:42:43 -0800
From: Mike Bonnet <mikeb(a)redhat.com>
To: Mathieu Bridon <mbridon(a)redhat.com>
CC: Pavol Babincak <pbabinca(a)redhat.com>
On 11/12/14 10:19 AM, Mike Bonnet wrote:
> On 11/4/14 5:03 AM, Mike Bonnet wrote:
>> On 11/04/2014 06:08 AM, Mathieu Bridon wrote:
>>> On Mon, 2014-11-03 at 15:56 -0500, Mike Bonnet wrote:
>>>> On 11/03/2014 04:11 AM, Mathieu Bridon wrote:
>>>>> On Mon, 2014-11-03 at 10:10 +0100, Mathieu Bridon wrote:
>>>>>> On Fri, 2014-10-31 at 09:07 -0700, Mike Bonnet wrote:
>>>>>> def __init__(self, path, lookaside, lookasidehash,
>>>>>> lookaside_cgi,
>>>>>> gitbaseurl, anongiturl, branchre, kojiconfig,
>>>>>> - build_client, user=None, dist=None, target=None,
>>>>>> + build_client, user=None, runas=None, dist=None,
>>>>>> target=None,
>>>>>>
>>>>>> This breaks fedpkg.
>>>>>>
>>>>>> To be honest, it is fedpkg that is doing something wrong here:
>>>>>>
>>>>>> super(Commands, self).__init__(path, lookaside,
>>>>>> lookasidehash,
>>>>>> lookaside_cgi, gitbaseurl,
>>>>>> anongiturl,
>>>>>> branchre, kojiconfig,
>>>>>> build_client,
>>>>>> user, dist, target, quiet)
>>>>>>
>>>>>> Keyword arguments passed as positional arguments. :(
>>>>>>
>>>>>> However, it means that applying your patch to rpkg as it is will
>>>>>> break
>>>>>> fedpkg, and potentially other consumers of the pyrpkg API using it in
>>>>>> the same (wrong) way.
>>>>>>
>>>>>> At the very least, we need to fix fedpkg so it correctly uses the
>>>>>> pyrpkg
>>>>>> API (passing kwargs as kwargs).
>>>>>>
>>>>>> Maybe we should also announce it widely that this change is coming,
>>>>>> and
>>>>>> people should check their code to make sure they don't do the same
>>>>>> thing
>>>>>> as fedpkg?
>>>>>
>>>>> Also, it would have been nicer to send these patches to the public
>>>>> mailing-list, as the discussion about how this breaks fedpkg could
>>>>> have
>>>>> served as a starting point for widely announcing that it might break
>>>>> other consumers. :)
>>>>
>>>> I did send a message about the original (pre-rebase) patch set to
>>>> Fedora
>>>> buildsys list.
>>>>
>>>> Would it be better to move runas=None to the end of the keyword
>>>> parameter list of __init__()? This should avoid the breakage of
>>>> fedpkg.
>>>
>>> It should. (and password=None as well)
>>
>> I'll send a fixed-up patch to Fedora buildsys list.
>
> I actually sent a patch to fix fedpkg to buildsys list. Could we get
> that applied and rolled out, then go with the rpkg patches as-is?
The attached patch fixes the fedpkg breakage. It handles the new
command-line options the same way the module_name parameter is handled.
--
Pavol Babincak
Release Engineering, Red Hat