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 AE4781382BD for ; Sat, 18 Jun 2016 18:03:41 +0000 (UTC) Received: from pigeon.gentoo.org (localhost [127.0.0.1]) by pigeon.gentoo.org (Postfix) with SMTP id 5D1A9E0BB0; Sat, 18 Jun 2016 18:03:40 +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 A648FE0B49 for ; Sat, 18 Jun 2016 18:03:39 +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 53C7033D3CE for ; Sat, 18 Jun 2016 18:03:38 +0000 (UTC) Date: Sat, 18 Jun 2016 11:02:53 -0700 From: Brian Dolbec To: gentoo-portage-dev@lists.gentoo.org Subject: Re: [gentoo-portage-dev] [PATCH] repoman: new QA error: slot operator under '||' alternative Message-ID: <20160618110253.4aac27c6.dolsen@gentoo.org> In-Reply-To: <20160618170111.21592-3-slyfox@gentoo.org> References: <20160618170111.21592-1-slyfox@gentoo.org> <20160618170111.21592-3-slyfox@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: 9a4edbea-6c09-4cdc-b439-726112e08a7e X-Archives-Hash: d19cad99c53bed27ccd5a00f0463e57f On Sat, 18 Jun 2016 18:01:11 +0100 Sergei Trofimovich wrote: > From: Sergei Trofimovich > > A few links discussing what can go wrong with slot operator under > '||': > https://archives.gentoo.org/gentoo-dev/message/81a4e1a1f5767e20009628ecd33da8d4 > https://archives.gentoo.org/gentoo-dev/message/66ff016a055e57b6027abcd36734e0e3 > > The main problem here is how vdb gets updated when you have a > dependency of style: RDEPEND="|| ( foo:= bar:= )" > depending on what you have installed in system: only 'foo'/'bar' or > both 'foo' and 'bar'. > > I'm about to add actual test for some examples to gen-b0rk. > > New repoman complains on them as follows: > > RDEPEND="|| ( =not-broken/pkg1-subslot-0:= > =not-broken/pkg1-subslot-1:0= )" > > Yields: > > dependency.badslotop [fatal] 2 > ebuild-test/RDEPEND-any-of-slotop/RDEPEND-any-of-slotop-0.ebuild: > RDEPEND: '=not-broken/pkg1-subslot-0:=' uses ':=' slot operator under > '||' dep clause. > ebuild-test/RDEPEND-any-of-slotop/RDEPEND-any-of-slotop-0.ebuild: > RDEPEND: '=not-broken/pkg1-subslot-1:0=' uses ':=' slot operator > under '||' dep clause. > > CC: qa@gentoo.org > CC: mgorny@gentoo.org > Signed-off-by: Sergei Trofimovich > --- > repoman/pym/repoman/check_slotop.py | 32 > ++++++++++++++++++++++ .../repoman/modules/scan/depend/_depend_checks.py > | 17 ++++++++++++ repoman/pym/repoman/qa_data.py > | 2 ++ 3 files changed, 51 insertions(+) > create mode 100644 repoman/pym/repoman/check_slotop.py > Hmm, this file should be in the module namespace, not in the base repoman namespace. It is not used anywhere else, so should be in the module like _depend_checks.py Another alternative is just add it to _depend_checks.py, it isn't a very large file and is the only place this code is used. > diff --git a/repoman/pym/repoman/check_slotop.py > b/repoman/pym/repoman/check_slotop.py new file mode 100644 > index 0000000..3c3aec1 > --- /dev/null > +++ b/repoman/pym/repoman/check_slotop.py > @@ -0,0 +1,32 @@ > +# -*- coding:utf-8 -*- > +# repoman: missing slot check > +# Copyright 2016 Gentoo Foundation > +# Distributed under the terms of the GNU General Public License v2 > + > +"""This module contains the check used to find ':=' slot operator > +uses in '||' style dependencies.""" > + > +from portage.dep import Atom > + > +def check_slotop(dep_tree, mytype, qatracker, relative_path): > + def _traverse_tree(dep_tree, is_under_any_of): > + # leaf > + if isinstance(dep_tree, Atom): > + atom = dep_tree > + if is_under_any_of and atom.slot_operator: > + > qatracker.add_error("dependency.badslotop", relative_path + > + ": %s: '%s' uses ':=' slot > operator under '||' dep clause." % > + (mytype, atom)) > + > + # branches > + if isinstance(dep_tree, list): > + if len(dep_tree) == 0: > + return > + # any-of > + if dep_tree[0] == '||': > + _traverse_tree(dep_tree[1:], True) > + else: > + for branch in dep_tree: > + _traverse_tree(branch, > is_under_any_of) + > + _traverse_tree(dep_tree, False) > diff --git > a/repoman/pym/repoman/modules/scan/depend/_depend_checks.py > b/repoman/pym/repoman/modules/scan/depend/_depend_checks.py index > 4e1d216..1fd69d4 100644 --- > a/repoman/pym/repoman/modules/scan/depend/_depend_checks.py +++ > b/repoman/pym/repoman/modules/scan/depend/_depend_checks.py @@ -4,6 > +4,7 @@ from _emerge.Package import Package > from repoman.check_missingslot import check_missingslot > +from repoman.check_slotop import check_slotop > # import our initialized portage instance > from repoman._portage import portage > from repoman.qa_data import suspect_virtual, suspect_rdepend > @@ -117,6 +118,22 @@ def _depend_checks(ebuild, pkg, portdb, > qatracker, repo_metadata): > type_list.extend([mytype] * (len(badsyntax) - > len(type_list))) > + if runtime: > + try: > + # to find use of ':=' in '||' we > preserve > + # tree structure of dependencies > + hier_atoms = portage.dep.use_reduce( > + mydepstr, > + flat=False, > + matchall=1, > + > is_valid_flag=pkg.iuse.is_valid_flag, > + opconvert=True, > + token_class=token_class) > + except portage.exception.InvalidDependString > as e: > + hier_atoms = None > + badsyntax.append(str(e)) > + check_slotop(hier_atoms, mytype, qatracker, > ebuild.relative_path) Is there a special reason this code can not be part of the check_slotop()? All it has is the one statement calling it's internal recursive function. Then all this code would would be replaced here by the one check_slotop() which adds to badsyntax. > for m, b in zip(type_list, badsyntax): > if m.endswith("DEPEND"): > qacat = "dependency.syntax" > diff --git a/repoman/pym/repoman/qa_data.py > b/repoman/pym/repoman/qa_data.py index b9475e8..48ab389 100644 > --- a/repoman/pym/repoman/qa_data.py > +++ b/repoman/pym/repoman/qa_data.py > @@ -58,6 +58,8 @@ qahelp = { > "Ebuild has a dependency that refers to an unknown > package" " (which may be valid if it is a blocker for a > renamed/removed package," " or is an alternative choice provided by > an overlay)"), > + "dependency.badslotop": ( > + "RDEPEND contains ':=' slot operator under '||' > dependency."), "file.executable": ( > "Ebuilds, digests, metadata.xml, Manifest, and > ChangeLog do not need" " the executable bit"), -- Brian Dolbec