Gentoo Archives: gentoo-portage-dev

From: Brian Harring <ferringb@×××××.com>
To: gentoo-portage-dev@l.g.o
Subject: Re: [gentoo-portage-dev] tree dependency check
Date: Thu, 30 Mar 2006 03:02:35
Message-Id: 8d4fefb00603291901x3808b15ayc35ee1393ac6ccd5@mail.gmail.com
In Reply to: Re: [gentoo-portage-dev] tree dependency check by Marius Mauch
1 On 3/29/06, Marius Mauch <genone@g.o> wrote:
2 > On Wed, 29 Mar 2006 14:42:25 -0800
3 > "Brian Harring" <ferringb@×××××.com> wrote:
4 >
5 > > 2) code isn't root aware.
6 >
7 > If root == $ROOT, then I don't see how it would affect this. Even with
8 > $ROOT != / we're using the tools in / for most things, $ROOT is just
9 > for deployment.
10
11 If ROOT!="/", portage installed at ROOT=/ isn't capable of reading the
12 tree, the check will invalidly assume the user is trying to update
13 installed portage at ROOT=/ when in reality it's trying to update
14 ROOT!="/" .
15
16 ROOT=/ should be updated prior to fooling with ROOT!=/ since as you
17 said, it's just the offset the files get installed to.
18
19 And yes, my initial description sucked :)
20
21
22 > > 4) code invalidly assumes that all later version of the pulled atom
23 > > from vdb will work.
24 >
25 > That assumption is only a fallback if not atom mapping for a format can
26 > be found. You have a better fallback strategy?
27
28 You've got 3 fallbacks already, plus this. Frankly, you can make it
29 only so smart- the simpler the code is, the less likely it gets
30 screwed up (thus making compatibility an issue since you have to work
31 around broken compatibility checks).
32
33 Telling folks to upgrade to latest portage version when the latest may
34 not even support that tree format still is bad advice for a user.
35
36
37 > > 6) This breaks _all_ syncing for users who have overlays but lack the
38 > > format versions file. That's a massive no go, you don't break
39 > > compatibility introducing compatibility checks (nor do you piss off
40 > > several thousand overlay using users for questionable gain).
41 >
42 > Reread the code, missing format_version raises a warning, nothing else
43 > (for now).
44
45 Bleh, that's what the hell that split was for...
46 That's a bit fugly, and is going to be extremely obnoxious for
47 consumers of it (warning for each overlay isn't purdy).
48
49
50 > > 7) even cooler, say you're running max visible portage, and using an
51 > > overlay that lacks a format_version file. With the vdb portage
52 > > lookup, it'll tell the user that they need a version later then the
53 > > max version. Nice way to get people to test package masked portage's,
54 > > but it still is wrong.
55 >
56 > see above.
57
58 This case still exists; assume format_version exists and it's unknown
59 to any of the 3 (!?) mappings, and that it isn't ''- still is
60 misleading to users.
61
62
63 > > 8) (minor) output of todo is going to be fugly if anyone uses actual
64 > > atom constructs, boolean ORs fex (the print implicitly assumes it's
65 > > just a list of atoms without any boolean constructs).
66 >
67 > Yeah, if anyone want to take a shot to convert it to feed the list into
68 > the depresolver feel free.
69
70 Comment was in regards to the output; don't have to resolve it to
71 output it properly.
72
73
74 > > 9) the attempted check to see if a pkg is in the passed in myfiles
75 > > won't work if myfiles holds atoms; eg,
76 > > myfiles, pkg = [">=sys-apps/portage-2.0.54"], "sys-apps/portage"
77 > > assert pkg not in myfiles
78 >
79 > reread the code, we're only comparing the keys (still not perfect, but
80 > covers your case).
81
82 Yeah, noticed that one shortly after opening the mouth ('bout the norm).
83
84 >
85 > > 2) It's overengineered. There is _no_ reason to hit up a webserver
86 > > just to get atoms; that data can be bundled in the tree in a seperate
87 > > file. As is, this breaks users who sync without a connection without
88 > > any gain. Realistically, I'd be surprised if any alt package managers
89 > > go this route (I know I won't be hitting a webserver up for pkgcore).
90 >
91 > a) it's a fallback if the system and tree don't have a depmap
92 > b) it doesn't break
93
94 So it's a fallback (vdb lookup) with a fallback (uri lookup) with a
95 fallback (bundled mapping) with a default (tree), right?
96
97 Sounds overengineered to me ;)
98
99 Realistically, the cases where the url fallback are going to be useful
100 are when the tree doesn't provide it and it's an official tree format-
101 in other words, *very* rarely (if ever) for folks using rsync portdir.
102 Seriously, it's only useful when their tree is partially corrupted or
103 someone screws up and bumps the format version of the tree without
104 updating the tree deps.
105
106 For folks _not_ using gentoo provided portdir, the portage project
107 'default' is pretty much of no use at all- won't cover any of the tree
108 level deps beyond stating "you need portage xyz". So again, what gain
109 beyond added complexity?
110
111
112 > > 3) What the portage project thinks a repo tree needs does not map to
113 > > what my tree may need. Clarifying, format 1 specifies portage xyz and
114 > > bash-3 (ebuilds in the tree use bash regex). My personal tree needs
115 > > portage xyz (manifest/layout changes), but requires just bash-2. With
116 > > the central db approach, portage will assume my tree is valid via the
117 > > version #, and if the number differs, it'll assume that I require
118 > > bash3 when in reality, my tree is bash-2 and up. This points to why
119 > > the format -> depends mapping should be bundled with the tree.
120 >
121 > See above.
122
123 <from the patch>
124 + mylocations = []
125 + mylocations.append(os.path.join(os.sep, portage_const.PRIVATE_PATH,
126 "format2dep.map"))
127 + mylocations.append(os.path.join(tree, "portage-format2dep.map"))
128 + mylocations.append(portage_const.FORMATMAP_URL)
129 +
130 + formatmap = {}
131 + for loc in mylocations:
132 + try:
133 + f = urlopen(loc)
134 + except:
135 + continue
136 <snip>
137 + formatmap[l[0]] = l[1:]
138 + f.close()
139
140 ^^^ that looks a helluva lot like a L->R updating of the dict, with
141 the rightmost overriding any previous key, rightmost in the list being
142 the URL... so unless I'm on seriously good crack (literally top of the
143 line), the tree bundled mapping has no say in it if portage project
144 states it as xyz.
145
146 What also sucks a bit about that is that say a user has 6 overlays
147 (random figure), it forces 6 pulls of that file- and just to make it
148 _really_ fun, lets say they're stuck behind a pissy firewall that
149 requires proxying.
150
151 Because this code is using urllib instead of the portage supplied
152 network downloader (fetch func), it has to pull proxy from the env
153 (which current code doesn't do); if this isn't done, it's 6 timeouts
154 without being able to fetch the file because it's not using the user
155 specified fetching command (FETCHCOMMAND).
156
157 Additional implementation note, the code above duplicates grabdict and
158 doesn't handle comments well (format handling in general doesn't
159 handle comments well). I'd take a look at extending
160 portage_util.grab* to work with file objects if passed in instead of
161 reinventing the wheel in another bit of code.
162
163 Finally... I'll note you dodged my point about shoving the file into
164 metadata dir; polluting the root dir (then and now) still is fugly
165 from where I'm sitting (address that point please) :)
166 ~harring
167
168 --
169 gentoo-portage-dev@g.o mailing list