From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from lists.gentoo.org (pigeon.gentoo.org [208.92.234.80]) by finch.gentoo.org (Postfix) with ESMTP id 4D281138206 for ; Sun, 24 Apr 2016 22:05:03 +0000 (UTC) Received: from pigeon.gentoo.org (localhost [127.0.0.1]) by pigeon.gentoo.org (Postfix) with SMTP id 41A9BE086F; Sun, 24 Apr 2016 22:05:01 +0000 (UTC) Received: from smtp.gentoo.org (smtp.gentoo.org [140.211.166.183]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by pigeon.gentoo.org (Postfix) with ESMTPS id 9DA65E0838 for ; Sun, 24 Apr 2016 22:05:00 +0000 (UTC) Received: from [192.168.0.20] (ip68-5-185-102.oc.oc.cox.net [68.5.185.102]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) (Authenticated sender: zmedico) by smtp.gentoo.org (Postfix) with ESMTPSA id 0E992340AE0 for ; Sun, 24 Apr 2016 22:04:59 +0000 (UTC) Subject: Re: [gentoo-portage-dev] [PATCH] ArchChecks: don't mix arches between ebuilds To: gentoo-portage-dev@lists.gentoo.org References: <1461455841-20194-1-git-send-email-zmedico@gentoo.org> <20160424081556.3f1b70e3.mgorny@gentoo.org> <20160424010028.0ebb302a.dolsen@gentoo.org> <571D3E7D.3070705@gentoo.org> From: Zac Medico Message-ID: <571D4309.5090405@gentoo.org> Date: Sun, 24 Apr 2016 15:04:57 -0700 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.7.0 Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: List-Id: Gentoo Linux mail X-BeenThere: gentoo-portage-dev@lists.gentoo.org Reply-to: gentoo-portage-dev@lists.gentoo.org MIME-Version: 1.0 In-Reply-To: <571D3E7D.3070705@gentoo.org> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Archives-Salt: c69c6c2e-7c5a-41ec-80d7-b6c885f8558a X-Archives-Hash: 14c23c5e718fe5e374c4915482f07d81 On 04/24/2016 02:45 PM, Zac Medico wrote: > On 04/24/2016 01:00 AM, Brian Dolbec wrote: >> On Sun, 24 Apr 2016 08:15:56 +0200 >> Michał Górny wrote: >> >>> On Sat, 23 Apr 2016 16:57:21 -0700 >>> Zac Medico wrote: >>> >>>> Fix ArchChecks to not mix arches of ebuilds together, so that >>>> errors are only reported for those arches that the ebuild has >>>> keywords for. >>>> --- >>>> Applies to the *repoman* branch. >>>> >>>> pym/repoman/modules/scan/arches/arches.py | 2 +- >>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>> >>>> diff --git a/pym/repoman/modules/scan/arches/arches.py >>>> b/pym/repoman/modules/scan/arches/arches.py index 4df25a8..6e1c17d >>>> 100644 --- a/pym/repoman/modules/scan/arches/arches.py >>>> +++ b/pym/repoman/modules/scan/arches/arches.py >>>> @@ -69,7 +69,7 @@ class ArchChecks(ScanBase): >>>> arches.add(('**', '**', ('**',))) >>>> # update the dynamic data >>>> dyn_arches = kwargs.get('arches') >>>> - #dyn_arches.clear() >>>> + dyn_arches.clear() >>>> dyn_arches.update(arches) >>> >>> Is that some crazy way of replacing contents of an existing dict? May >>> be worth a comment since the code looks suspicious, at least. >>> >> >> It was an interim step so that modules didn't pass back >> arbitrary data to be re-used by other modules after it. This way >> maintains the data pointer to the set (not dict) object in the calling >> function. Just assigning it a new set doesn't set the 'arches' pointer >> in the calling function so that it can pass that data along to other >> modules. > > I feel like the plugin structure has lead to decoupling of things that > really should be coupled. For example, the code in the ArchChecks could > be a function that's called by the ProfileDependsChecks plugin, given > that ProfileDependsChecks seems to be the only consumer of this data. We'll have to go through all of the plugins and look for other things that have too aggressively decoupled. The fact Scanner._scan_ebuilds calls plugins in a hardcoded order is troubling. It means that there may be coupling between plugins that makes the order matter, yet this coupling is hidden and non-trivial to discover. If we couple more things together where appropriate (like ArchChecks and ProfileDependsChecks), then it will make the system easier for a person to reason about. -- Thanks, Zac