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

[Brian Dolbec]

On Sat, 18 Jun 2016 23:00:21 +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: dol...@gentoo.org
> CC: q...@gentoo.org
> CC: mgo...@gentoo.org
> Signed-off-by: Sergei Trofimovich 
> ---
> V4: forbid only ':=' style slot operator. Zac pointed out ':*' is
> fine here. V3: unbaked tree traversal back to _traverse_tree, moved
> dependency parser out to check_slotop() helper
> V2: made check_slotop function local, baked tree traversal in it
>  .../repoman/modules/scan/depend/_depend_checks.py  | 45
> ++
> repoman/pym/repoman/qa_data.py |  2 + 2 files
> changed, 47 insertions(+)
> 
> diff --git
> a/repoman/pym/repoman/modules/scan/depend/_depend_checks.py
> b/repoman/pym/repoman/modules/scan/depend/_depend_checks.py index
> 4e1d216..3f6c93e 100644 ---
> a/repoman/pym/repoman/modules/scan/depend/_depend_checks.py +++
> b/repoman/pym/repoman/modules/scan/depend/_depend_checks.py @@ -3,11
> +3,52 @@ from _emerge.Package import Package
>  
> +from portage.dep import Atom
> +
>  from repoman.check_missingslot import check_missingslot
>  # import our initialized portage instance
>  from repoman._portage import portage
>  from repoman.qa_data import suspect_virtual, suspect_rdepend
>  
> +def check_slotop(depstr, is_valid_flag, badsyntax, mytype,
> + qatracker, relative_path):
> + '''Checks if RDEPEND uses ':=' slot operator
> + in '||' style dependencies.'''
> +
> + try:
> + # to find use of ':=' in '||' we preserve
> + # tree structure of dependencies
> + my_dep_tree = portage.dep.use_reduce(
> + depstr,
> + flat=False,
> + matchall=1,
> + is_valid_flag=is_valid_flag,
> + opconvert=True,
> + token_class=portage.dep.Atom)
> + except portage.exception.InvalidDependString as e:
> + my_dep_tree = None
> + badsyntax.append(str(e))
> +
> + def _traverse_tree(dep_tree, in_any_of):
> + # leaf
> + if isinstance(dep_tree, Atom):
> + atom = dep_tree
> + if in_any_of and atom.slot_operator == '=':
> +
> qatracker.add_error("dependency.badslotop",
> + "%s: %s: '%s' uses ':=' slot
> operator under '||' dep clause." %
> + (relative_path, mytype,
> atom)) +
> + # branches
> + if isinstance(dep_tree, list):
> + if len(dep_tree) == 0:
> + return
> + # entering any-of
> + if dep_tree[0] == '||':
> + _traverse_tree(dep_tree[1:],
> in_any_of=True)
> + else:
> + for branch in dep_tree:
> + _traverse_tree(branch,
> in_any_of=in_any_of)
> + _traverse_tree(my_dep_tree, False)
>  
>  def _depend_checks(ebuild, pkg, portdb, qatracker, repo_metadata):
>   '''Checks the ebuild dependencies for errors
> @@ -117,6 +158,10 @@ def _depend_checks(ebuild, pkg, portdb,
> qatracker, repo_metadata): 
>   type_list.extend([mytype] * (len(badsyntax) -
> len(type_list))) 
> + if runtime:
> + check_slotop(mydepstr,
> pkg.iuse.is_valid_flag,
> + badsyntax, mytype, qatracker,
> ebuild.relative_path) +
>   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 

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

[Sergei Trofimovich]

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: dol...@gentoo.org
CC: q...@gentoo.org
CC: mgo...@gentoo.org
Signed-off-by: Sergei Trofimovich 
---
V4: forbid only ':=' style slot operator. Zac pointed out ':*' is fine here.
V3: unbaked tree traversal back to _traverse_tree, moved dependency parser out
to check_slotop() helper
V2: made check_slotop function local, baked tree traversal in it
 .../repoman/modules/scan/depend/_depend_checks.py  | 45 ++
 repoman/pym/repoman/qa_data.py |  2 +
 2 files changed, 47 insertions(+)

diff --git a/repoman/pym/repoman/modules/scan/depend/_depend_checks.py 
b/repoman/pym/repoman/modules/scan/depend/_depend_checks.py
index 4e1d216..3f6c93e 100644
--- a/repoman/pym/repoman/modules/scan/depend/_depend_checks.py
+++ b/repoman/pym/repoman/modules/scan/depend/_depend_checks.py
@@ -3,11 +3,52 @@
 
 from _emerge.Package import Package
 
+from portage.dep import Atom
+
 from repoman.check_missingslot import check_missingslot
 # import our initialized portage instance
 from repoman._portage import portage
 from repoman.qa_data import suspect_virtual, suspect_rdepend
 
+def check_slotop(depstr, is_valid_flag, badsyntax, mytype,
+   qatracker, relative_path):
+   '''Checks if RDEPEND uses ':=' slot operator
+   in '||' style dependencies.'''
+
+   try:
+   # to find use of ':=' in '||' we preserve
+   # tree structure of dependencies
+   my_dep_tree = portage.dep.use_reduce(
+   depstr,
+   flat=False,
+   matchall=1,
+   is_valid_flag=is_valid_flag,
+   opconvert=True,
+   token_class=portage.dep.Atom)
+   except portage.exception.InvalidDependString as e:
+   my_dep_tree = None
+   badsyntax.append(str(e))
+
+   def _traverse_tree(dep_tree, in_any_of):
+   # leaf
+   if isinstance(dep_tree, Atom):
+   atom = dep_tree
+   if in_any_of and atom.slot_operator == '=':
+   qatracker.add_error("dependency.badslotop",
+   "%s: %s: '%s' uses ':=' slot operator 
under '||' dep clause." %
+   (relative_path, mytype, atom))
+
+   # branches
+   if isinstance(dep_tree, list):
+   if len(dep_tree) == 0:
+   return
+   # entering any-of
+   if dep_tree[0] == '||':
+   _traverse_tree(dep_tree[1:], in_any_of=True)
+   else:
+   for branch in dep_tree:
+   _traverse_tree(branch, 
in_any_of=in_any_of)
+   _traverse_tree(my_dep_tree, False)
 
 def _depend_checks(ebuild, pkg, portdb, qatracker, repo_metadata):
'''Checks the ebuild dependencies for errors
@@ -117,6 +158,10 @@ def _depend_checks(ebuild, pkg, portdb, qatracker, 
repo_metadata):
 
type_list.extend([mytype] * (len(badsyntax) - len(type_list)))
 
+   if runtime:
+   check_slotop(mydepstr, pkg.iuse.is_valid_flag,
+   badsyntax, mytype, qatracker, 
ebuild.relative_path)
+
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