round two of the patch, still is missing...
On Sun, Feb 18, 2007 at 06:27:59PM +0000, Marius Mauch wrote:
> Author: genone
> Date: 2007-02-18 18:27:59 +0000 (Sun, 18 Feb 2007)
> New Revision: 5993
>
> Modified:
> main/trunk/pym/portage/dbapi/vartree.py
> Log:
> extend check for internal references, should remove all libs that are only
> used internally now
>
> Modified: main/trunk/pym/portage/dbapi/vartree.py
> ===================================================================
> --- main/trunk/pym/portage/dbapi/vartree.py 2007-02-18 15:38:56 UTC (rev
> 5992)
> +++ main/trunk/pym/portage/dbapi/vartree.py 2007-02-18 18:27:59 UTC (rev
> 5993)
> @@ -1266,14 +1266,33 @@
> preserve_libs = old_libs.difference(mylibs)
>
> # ignore any libs that are only internally used by the
> package
> - for lib in preserve_libs.copy():
> - old_contents_without_libs = [x for x in
> old_contents if os.path.basename(x) not in preserve_libs]
> - if
> len(set(libmap[lib]).intersection(old_contents_without_libs)) ==
> len(libmap[lib]):
> - preserve_libs.remove(lib)
> + def has_external_consumers(lib, contents, otherlibs):
> + consumers = set(libmap[lib])
# this should be done *once*.
# further, if libmap where k->set, you'd avoid repeated
# creation of sets all over the damn place.
> + contents_without_libs = [x for x in contents if
> not os.path.basename(x) in otherlibs]
# os.path.basename invocations aren't horribly expensive, but the
> +
> + # just used by objects that will be autocleaned
> + if
> len(consumers.difference(contents_without_libs)) == 0:
if not consumers.difference(contents_without_libs)):...
# no point in doing the len, thus don't.
# standard behaviour in python code to rely on bool protocol, thus do
# so.
> + return False
> + # used by objects that are referenced as well,
> need to check those
> + # recursively to break any reference cycles
> + elif len(consumers.difference(contents)) == 0:
# same. len isn't needed, thus don't use it.
# majority of builtins *do* support boolean, thus use it.
> + otherlibs = set(otherlibs)
# this will explode. you're passing None in below.
> + for ol in
> otherlibs.intersection(consumers):
> + if has_external_consumers(ol,
> contents, otherlibs.copy().remove(lib)):
no way that code is right.
1) you're copying a set, then removing an item from it.
2) remove is a Py_RETURN_NONE, meaning it returns None. you're
literally cloning a set, poping an item from it, and then
throwing away the set, passing None into has_external_consumers
3) algo in general could be replaced by just doing walks over the
target preserve libs, pruning until a walk doesn't prune anything.
Runtime ought to be better, less daft copys, saner to read also.
> + return True
> + return False
> + # used by external objects directly
> + else:
> + return True
> +
> + for lib in list(preserve_libs):
> + if not has_external_consumers(lib,
> old_contents, preserve_libs):
> + preserve_libs.remove(lib)
>
# this is a bit retarded... for handling libs referring to each other,
# code above continually recalculates.
# reverse the mapping, and work from that angle; should avoid having
# to do this insane little dance.
>
> # get the real paths for the libs
> preserve_paths = [x for x in old_contents if
> os.path.basename(x) in preserve_libs]
> -
> + del old_contents, old_libs, mylibs, preserve_libs
> +
> # inject files that should be preserved into our image
> dir
> import shutil
> for x in preserve_paths:
> @@ -1293,7 +1312,7 @@
> preserve_paths.append(linktarget)
> else:
> shutil.copy2(x, os.path.join(srcroot,
> x.lstrip(os.sep)))
> -
> + del preserve_paths
>
> # check for package collisions
> if "collision-protect" in self.settings.features:
>
Aside from my earlier suggestion that you should pop this crap out of
svn and work it out outside of mainline, breaking it out into a func
and doing tests for the func might be wise- very least easier for you
to iron it out so it behaves. Upshot, makes it easier for someone to
refactor it down the line.
Still have the crappy forced double walk of ${D} regression also.
~harring
pgptsRMvFp1xP.pgp
Description: PGP signature
