* [gentoo-portage-dev] Re: r5993 - main/trunk/pym/portage/dbapi
[not found] <E1HIqlT-0006nz-Uv@lark.gentoo.org>
@ 2007-02-18 19:03 ` Brian Harring
2007-02-18 19:19 ` Brian Harring
0 siblings, 1 reply; 2+ messages in thread
From: Brian Harring @ 2007-02-18 19:03 UTC (permalink / raw
To: gentoo-portage-dev
[-- Attachment #1: Type: text/plain, Size: 4353 bytes --]
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
[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [gentoo-portage-dev] Re: r5993 - main/trunk/pym/portage/dbapi
2007-02-18 19:03 ` [gentoo-portage-dev] Re: r5993 - main/trunk/pym/portage/dbapi Brian Harring
@ 2007-02-18 19:19 ` Brian Harring
0 siblings, 0 replies; 2+ messages in thread
From: Brian Harring @ 2007-02-18 19:19 UTC (permalink / raw
To: gentoo-portage-dev
[-- Attachment #1: Type: text/plain, Size: 1059 bytes --]
Bleh, pardon; left out a bit accidentally-
On Sun, Feb 18, 2007 at 11:03:33AM -0800, Brian Harring wrote:
> On Sun, Feb 18, 2007 at 06:27:59PM +0000, Marius Mauch wrote:
> > - 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
continual calls to it make the code more verbose then needed (Thus
harder to follow), and slower. Consider a mapping instead, see the
first review for old_based...
~harring
[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2007-02-18 19:21 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <E1HIqlT-0006nz-Uv@lark.gentoo.org>
2007-02-18 19:03 ` [gentoo-portage-dev] Re: r5993 - main/trunk/pym/portage/dbapi Brian Harring
2007-02-18 19:19 ` Brian Harring
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox