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 |