Re: [gentoo-portage-dev] [PATCH] repoman: new QA error: slot operator under '||' alternative

[Brian Dolbec]

On Sat, 18 Jun 2016 18:01:11 +0100
Sergei Trofimovich <slyfox@g.o> wrote:

> From: Sergei Trofimovich <siarheit@××××××.com>
>
> 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@g.o
> CC: mgorny@g.o
> Signed-off-by: Sergei Trofimovich <siarheit@××××××.com>
> ---
>  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 <dolsen>