Gentoo Archives: gentoo-portage-dev

From: Brian Dolbec <dolsen@g.o>
To: gentoo-portage-dev@l.g.o
Subject: Re: [gentoo-portage-dev] [PATCH] repoman: new QA error: slot operator under '||' alternative
Date: Sat, 18 Jun 2016 20:39:11
Message-Id: 20160618133813.2964fff9.dolsen@gentoo.org
1 On Sat, 18 Jun 2016 20:37:06 +0100
2 Sergei Trofimovich <slyich@×××××.com> wrote:
3
4 > On Sat, 18 Jun 2016 11:02:53 -0700
5 > Brian Dolbec <dolsen@g.o> wrote:
6 >
7 > > > + if runtime:
8 > > > + try:
9 > > > + # to find use of ':=' in '||' we
10 > > > preserve
11 > > > + # tree structure of dependencies
12 > > > + hier_atoms =
13 > > > portage.dep.use_reduce(
14 > > > + mydepstr,
15 > > > + flat=False,
16 > > > + matchall=1,
17 > > > +
18 > > > is_valid_flag=pkg.iuse.is_valid_flag,
19 > > > + opconvert=True,
20 > > > + token_class=token_class)
21 > > > + except
22 > > > portage.exception.InvalidDependString as e:
23 > > > + hier_atoms = None
24 > > > + badsyntax.append(str(e))
25 > > > + check_slotop(hier_atoms, mytype,
26 > > > qatracker, ebuild.relative_path)
27 > >
28 > >
29 > > Is there a special reason this code can not be part of the
30 > > check_slotop()? All it has is the one statement calling it's
31 > > internal recursive function. Then all this code would would be
32 > > replaced here by the one check_slotop() which adds to badsyntax.
33 >
34 > I don't have special preference. I've followed example of what a
35 > check few lines before does to call check_missingslot(). I've sent a
36 > V2 patch w/o pulling it off to a separate helper.
37 >
38 > If you think it should be better moved completely to a separate
39 > helper I'll move it there. As you see I have a very weak python fu.
40 >
41 > If there is examples of better ways to pass that large context of
42 > variables please point me to it :)
43 >
44
45 OK, V2 is better, but not quite what I had thought of.
46
47 The original check_sloptop() had 2 items, the _traverse_tree() and the
48 one staement calling it. So, yes it is better that in that it doesn't
49 have the needless sub function. But the _depend_checks() is getting
50 quite long already. The new code you add in there makes it even longer
51 and adds even more branching. It can lead to things being more
52 complicated for future changes. What I meant was to move most of that
53 additional code into the pretty unused check_sloptop() and have it call
54 the _traverse_tree() from there. That would simplify the
55 depends_check() and move the check_sloptop specific code to that
56 function. Then the _traverse_tree sub-function would be utilized with
57 the check_slotop() better utilized.
58
59 so:
60
61 + if runtime:
62 + check_slotop(...)
63
64 would be all that was added to _depends_check() directly.
65
66 Zac, what do you think?
67 --
68 Brian Dolbec <dolsen>

Replies