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 A16DF59CA3 for ; Tue, 15 Mar 2016 00:48:10 +0000 (UTC) Received: from pigeon.gentoo.org (localhost [127.0.0.1]) by pigeon.gentoo.org (Postfix) with SMTP id A4B7221C015; Tue, 15 Mar 2016 00:48:07 +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 09FBB21C00D for ; Tue, 15 Mar 2016 00:48:06 +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 B6C35340C44 for ; Tue, 15 Mar 2016 00:48:05 +0000 (UTC) Date: Mon, 14 Mar 2016 17:47:10 -0700 From: Brian Dolbec To: gentoo-portage-dev@lists.gentoo.org Subject: Re: [gentoo-portage-dev] [Patch] Repoman rewrite stage2 modularization conversion complete Message-ID: <20160314174710.4b6ebd1e.dolsen@gentoo.org> In-Reply-To: <56E754D3.5080308@gentoo.org> References: <20160110134008.4fce78c0.dolsen@gentoo.org> <5693CCAF.7020102@gentoo.org> <20160305133705.42b54258.dolsen@gentoo.org> <56E6F167.4050806@gentoo.org> <56E6F343.80005@gentoo.org> <20160314105256.37fb8e90.dolsen@gentoo.org> <56E754D3.5080308@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=US-ASCII Content-Transfer-Encoding: 7bit X-Archives-Salt: 90ab408b-37bc-4bd5-81f5-858fc804ca89 X-Archives-Hash: ebfd3739aae02d5c9a8609b20235d6ac On Mon, 14 Mar 2016 17:18:27 -0700 Zac Medico wrote: > On 03/14/2016 10:52 AM, Brian Dolbec wrote: > > On Mon, 14 Mar 2016 10:22:11 -0700 > > Zac Medico wrote: > > > >> On 03/14/2016 10:14 AM, Zac Medico wrote: > >>> On 03/05/2016 01:37 PM, Brian Dolbec wrote: > >>> > >>>> Zac, I'm done with code changes in the rewrite. Ready for a last > >>>> look before a merge. Can you have a look again? I did some > >>>> changes/fixes and rebased them in. Floppym hasn't reported any > >>>> more bugs, so I think it's ready for broader testing in a > >>>> release. Then we can work on moving all the test data to a > >>>> separate file in the tree or downloaded... > >>> > >>> The dynamic_data stuff in Scanner is a little hard to follow. Then > >>> it calls dynamic_data.update(rdata), is there any chance that the > >>> update operation might clobber something that shouldn't have been > >>> clobbered? > >> > >> To clarify my question, suppose that one function returns {'foo': > >> True} and another one returns {'foo', False}, so now there first > >> {'foo': True} setting is forgotten. Is that going to be a > >> problem? > > > > No, as stated in my other reply. There are only a few things that > > are modified. Mostly as I made a new module, following the original > > order the checks were run. As data was discovered missing it was > > added to dynamic_data from the previous check that supplied it to > > the Scanner class. So, only data needed later was passed back to > > update the dynamic_data. > > > > Also all those checks originally ran in one huge 1k LOC loop with > > another slightly smaller ebuild loop nested inside it. So all those > > variables were subject to change already by previous code run. In > > the stage1 rewrite, I/we did the same thing in creating the > > separated checks classes. After the check was done, only the data > > required was brought back into the primary loop. > > > > I've found what may be a real instance of the kind of problem I was > worried about. The 'allvalid' key is set in both > pym/repoman/modules/scan/ebuild/isebuild.py and > pym/repoman/modules/scan/ebuild/ebuild.py, and its non-trivial for me > to determine whether this is a real problem or not. It is not a problem. allvalid is initialized in the isEbuild class at the start of the pkg level checks to True, then there are several things that if found reset it to False... Then when it gets to the ebuild level checks the ebuild check there just sets it False if the pkg.invalid variable is True. In some cases it might be a double False, but never will it trounce the previous setting. The only consumer for that allvalid variable is the metadata UnusedCheck class. So the allvalid variable is True until found False by whichever checks along the way find it to be False. Like a fuse, it's good until it's blown, then it can never be good again. I don't think this particular variable justifies a special class that more fully mimics a fuse. Impossible to reset it like a breaker. To be honest I did not look into the pkg.invalid variable's need to be setting it False in the Ebuild class. It may in fact be a dupe of a setting in the isEbuild class or it might be moved there. That original spaghetti code could in fact have had duplicates since it was so hard to figure out where to embed something. -- Brian Dolbec