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 14AD9138206 for ; Mon, 25 Apr 2016 03:49:13 +0000 (UTC) Received: from pigeon.gentoo.org (localhost [127.0.0.1]) by pigeon.gentoo.org (Postfix) with SMTP id E4E95E0858; Mon, 25 Apr 2016 03:49:10 +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 E246EE080C for ; Mon, 25 Apr 2016 03:49:09 +0000 (UTC) Received: from professor-x (S010634bdfa9ecf80.vc.shawcable.net [96.49.31.57]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) (Authenticated sender: dolsen) by smtp.gentoo.org (Postfix) with ESMTPSA id EDD8F340B10 for ; Mon, 25 Apr 2016 03:49:07 +0000 (UTC) Date: Sun, 24 Apr 2016 20:48:15 -0700 From: Brian Dolbec To: gentoo-portage-dev@lists.gentoo.org Subject: Re: [gentoo-portage-dev] [PATCH] ArchChecks: don't mix arches between ebuilds Message-ID: <20160424204815.1f20d8f8.dolsen@gentoo.org> In-Reply-To: <571D4309.5090405@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> <571D4309.5090405@gentoo.org> Organization: Gentoo 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 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable X-Archives-Salt: dc846486-c933-42c1-8a7d-2a5facd7080b X-Archives-Hash: 1eeb37b2abbbfd6752ef318bcf876937 On Sun, 24 Apr 2016 15:04:57 -0700 Zac Medico wrote: > On 04/24/2016 02:45 PM, Zac Medico wrote: > > On 04/24/2016 01:00 AM, Brian Dolbec wrote: =20 > >> On Sun, 24 Apr 2016 08:15:56 +0200 > >> Micha=C5=82 G=C3=B3rny wrote: > >> =20 > >>> On Sat, 23 Apr 2016 16:57:21 -0700 > >>> Zac Medico wrote: > >>> =20 > >>>> 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 =3D kwargs.get('arches') > >>>> - #dyn_arches.clear() > >>>> + dyn_arches.clear() > >>>> dyn_arches.update(arches) =20 > >>> > >>> Is that some crazy way of replacing contents of an existing dict? > >>> May be worth a comment since the code looks suspicious, at least. > >>> =20 > >> > >> 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. =20 > >=20 > > 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. =20 >=20 > 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. Yes, I agree, but the split out process was a long tedious one. And in making this module system, I did them in the order they were run originally. The code was split up all over with more and more things added to the spaghetti code that ended up at 1000+ lines of code for the one pkg & ebuild loop. It's no wonder things got split badly. The code now can be given decent scrutiny to optimize and refactor where appropriate. Like re-coupling some split up code. --=20 Brian Dolbec