On 3/29/06, Marius Mauch <[EMAIL PROTECTED]> wrote:
> On Wed, 29 Mar 2006 14:42:25 -0800
> "Brian Harring" <[EMAIL PROTECTED]> wrote:
>
> > 2) code isn't root aware.
>
> If root == $ROOT, then I don't see how it would affect this. Even with
> $ROOT != / we're using the tools in / for most things, $ROOT is just
> for deployment.
If ROOT!="/", portage installed at ROOT=/ isn't capable of reading the
tree, the check will invalidly assume the user is trying to update
installed portage at ROOT=/ when in reality it's trying to update
ROOT!="/" .
ROOT=/ should be updated prior to fooling with ROOT!=/ since as you
said, it's just the offset the files get installed to.
And yes, my initial description sucked :)
> > 4) code invalidly assumes that all later version of the pulled atom
> > from vdb will work.
>
> That assumption is only a fallback if not atom mapping for a format can
> be found. You have a better fallback strategy?
You've got 3 fallbacks already, plus this. Frankly, you can make it
only so smart- the simpler the code is, the less likely it gets
screwed up (thus making compatibility an issue since you have to work
around broken compatibility checks).
Telling folks to upgrade to latest portage version when the latest may
not even support that tree format still is bad advice for a user.
> > 6) This breaks _all_ syncing for users who have overlays but lack the
> > format versions file. That's a massive no go, you don't break
> > compatibility introducing compatibility checks (nor do you piss off
> > several thousand overlay using users for questionable gain).
>
> Reread the code, missing format_version raises a warning, nothing else
> (for now).
Bleh, that's what the hell that split was for...
That's a bit fugly, and is going to be extremely obnoxious for
consumers of it (warning for each overlay isn't purdy).
> > 7) even cooler, say you're running max visible portage, and using an
> > overlay that lacks a format_version file. With the vdb portage
> > lookup, it'll tell the user that they need a version later then the
> > max version. Nice way to get people to test package masked portage's,
> > but it still is wrong.
>
> see above.
This case still exists; assume format_version exists and it's unknown
to any of the 3 (!?) mappings, and that it isn't ''- still is
misleading to users.
> > 8) (minor) output of todo is going to be fugly if anyone uses actual
> > atom constructs, boolean ORs fex (the print implicitly assumes it's
> > just a list of atoms without any boolean constructs).
>
> Yeah, if anyone want to take a shot to convert it to feed the list into
> the depresolver feel free.
Comment was in regards to the output; don't have to resolve it to
output it properly.
> > 9) the attempted check to see if a pkg is in the passed in myfiles
> > won't work if myfiles holds atoms; eg,
> > myfiles, pkg = [">=sys-apps/portage-2.0.54"], "sys-apps/portage"
> > assert pkg not in myfiles
>
> reread the code, we're only comparing the keys (still not perfect, but
> covers your case).
Yeah, noticed that one shortly after opening the mouth ('bout the norm).
>
> > 2) It's overengineered. There is _no_ reason to hit up a webserver
> > just to get atoms; that data can be bundled in the tree in a seperate
> > file. As is, this breaks users who sync without a connection without
> > any gain. Realistically, I'd be surprised if any alt package managers
> > go this route (I know I won't be hitting a webserver up for pkgcore).
>
> a) it's a fallback if the system and tree don't have a depmap
> b) it doesn't break
So it's a fallback (vdb lookup) with a fallback (uri lookup) with a
fallback (bundled mapping) with a default (tree), right?
Sounds overengineered to me ;)
Realistically, the cases where the url fallback are going to be useful
are when the tree doesn't provide it and it's an official tree format-
in other words, *very* rarely (if ever) for folks using rsync portdir.
Seriously, it's only useful when their tree is partially corrupted or
someone screws up and bumps the format version of the tree without
updating the tree deps.
For folks _not_ using gentoo provided portdir, the portage project
'default' is pretty much of no use at all- won't cover any of the tree
level deps beyond stating "you need portage xyz". So again, what gain
beyond added complexity?
> > 3) What the portage project thinks a repo tree needs does not map to
> > what my tree may need. Clarifying, format 1 specifies portage xyz and
> > bash-3 (ebuilds in the tree use bash regex). My personal tree needs
> > portage xyz (manifest/layout changes), but requires just bash-2. With
> > the central db approach, portage will assume my tree is valid via the
> > version #, and if the number differs, it'll assume that I require
> > bash3 when in reality, my tree is bash-2 and up. This points to why
> > the format -> depends mapping should be bundled with the tree.
>
> See above.
<from the patch>
+ mylocations = []
+ mylocations.append(os.path.join(os.sep, portage_const.PRIVATE_PATH,
"format2dep.map"))
+ mylocations.append(os.path.join(tree, "portage-format2dep.map"))
+ mylocations.append(portage_const.FORMATMAP_URL)
+
+ formatmap = {}
+ for loc in mylocations:
+ try:
+ f = urlopen(loc)
+ except:
+ continue
<snip>
+ formatmap[l[0]] = l[1:]
+ f.close()
^^^ that looks a helluva lot like a L->R updating of the dict, with
the rightmost overriding any previous key, rightmost in the list being
the URL... so unless I'm on seriously good crack (literally top of the
line), the tree bundled mapping has no say in it if portage project
states it as xyz.
What also sucks a bit about that is that say a user has 6 overlays
(random figure), it forces 6 pulls of that file- and just to make it
_really_ fun, lets say they're stuck behind a pissy firewall that
requires proxying.
Because this code is using urllib instead of the portage supplied
network downloader (fetch func), it has to pull proxy from the env
(which current code doesn't do); if this isn't done, it's 6 timeouts
without being able to fetch the file because it's not using the user
specified fetching command (FETCHCOMMAND).
Additional implementation note, the code above duplicates grabdict and
doesn't handle comments well (format handling in general doesn't
handle comments well). I'd take a look at extending
portage_util.grab* to work with file objects if passed in instead of
reinventing the wheel in another bit of code.
Finally... I'll note you dodged my point about shoving the file into
metadata dir; polluting the root dir (then and now) still is fugly
from where I'm sitting (address that point please) :)
~harring
--
[email protected] mailing list
