On Thu, Apr 28, 2011 at 12:22:22AM +0200, Tshepang Lekhonkhobe wrote: > Package: python-apt > Version: 0.8.0~exp4 > Severity: wishlist > Tags: patch > > I found it surprising that missing_deps and required_changes required > a call to check() first. > # Bazaar merge directive format 2 (Bazaar 0.90) > # revision_id: tshepang@gmail.com-20110427221153-fb7azvn3hkqyix2h > # target_branch: http://bzr.debian.org/apt/python-apt/debian-\ > # experimental/ > # testament_sha1: cac4924b099b4007a2eee52ca0a22ea887bd9107 > # timestamp: 2011-04-28 00:14:14 +0200 > # base_revision_id: jak@debian.org-20110427110820-qmhxr9c0rux0nn6r > # > # Begin patch > === modified file 'apt/debfile.py' > --- apt/debfile.py 2011-04-06 09:15:47 +0000 > +++ apt/debfile.py 2011-04-27 22:11:53 +0000 > @@ -472,7 +472,7 @@ > def missing_deps(self): > """Return missing dependencies.""" > self._dbg(1, "Installing: %s" % self._need_pkgs) > - if self._need_pkgs is None: > + if not self._need_pkgs: > self.check() > return self._need_pkgs > > @@ -485,6 +485,8 @@ > install = [] > remove = [] > unauthenticated = [] > + if not self._cache: > + self.check() > for pkg in self._cache: > if pkg.marked_install or pkg.marked_upgrade: > install.append(pkg.name) For future reference, from the IRC log on that day: Apr 28 16:32:06 <juliank> mvo: What's your opinion about Bug#624379? Should DebPackage.missing_deps and required changes work without a call to check or should we throw an error there? tshepang wants an implicit check call, but it could get dirty if you change something on the cache, as the result is not true anymore Apr 28 16:32:35 <juliank> tshepang: "+ if not self._cache: Apr 28 16:32:35 <juliank> + self.check()" is never executed Apr 28 16:32:50 <tshepang> oh Apr 28 16:33:12 <juliank> mvo: s/true/correct/ Apr 28 16:43:27 * tshepang has quit (Ping timeout: 480 seconds) Apr 28 16:43:51 * tshepang (~wena@41-132-33-179.dsl.mweb.co.za) has joined #debian-apt Apr 28 16:44:44 * tshepang had network trouble Apr 28 16:44:48 <tshepang> here's my test code http://paste.debian.net/115356/ Apr 28 16:45:44 <tshepang> it proves that self.check (for required_changes) is executed Apr 28 16:50:56 <tshepang> juliank, u still there? Apr 28 16:54:12 <mvo> juliank: I quite like the idea of doing this implicitely, but you have a valid concern Apr 28 16:54:36 * mvo looks at the code again Apr 28 16:56:25 <mvo> we would need to check if the cache has chnaged since we last calculated it Apr 28 16:56:44 <mvo> but just raising "CallCheckPlease()" is probably ok as well Apr 28 16:56:58 <mvo> also I like the implict check as its more ellegant Apr 28 17:00:59 <tshepang> u mentioned concerns regarding calling check explicitly Apr 28 17:01:11 <tshepang> what's the disadvantages again (in plain english :) Apr 28 17:02:19 <tshepang> *I meant implicitly Apr 28 17:03:33 <tshepang> "it could get dirty if you change something on the cache" Apr 28 17:03:38 <tshepang> what that means Apr 28 17:03:47 <mvo> the concern that juliank has (please correct me if I misquote) is that you call missing_deps then maniplulate the cache in some way and on the next call it would still have the "old" information Apr 28 17:03:59 <mvo> because its "cached" in _need_pkgs Apr 28 17:04:24 <mvo> but that information is outdated if e.g. you have marked_install() something already Apr 28 17:05:11 <tshepang> oh, I kinda get it now Apr 28 17:06:30 <tshepang> so how does one check if cache has changed? Apr 28 17:14:50 <mvo> you could use cache.get_changes() and store that with the _need_pkgs I guess Apr 28 17:15:07 <mvo> I haven't look at the code closely in a while though, so please take it with a grain of salt :) Apr 28 17:22:08 <tshepang> I dont understand how cache works, so can you guys improve the docs for these two methods Apr 28 17:22:43 <tshepang> does the test suite even cover them? Apr 28 17:37:00 <juliank> mvo: In order to have performance be acceptable, I guess I just introduce a counter in the depcache that gets increased on every change Apr 28 17:37:26 <mvo> yeah, that is probably useful in other situations as well, I like that Apr 28 17:39:15 <juliank> Once we have this we only need to deal with reopening in apt.Cache Apr 28 17:40:13 <juliank> let's say we temporarily store the old count and set on the newdepcache.changes_count = olddepcache.changes_count + 1 Apr 28 17:40:23 <juliank> OK. Problem solvable. Apr 28 17:42:25 <mvo> nice work on the pkgOrderList stuff btw Apr 28 17:43:50 <tshepang> yeah, it was quite a big patch Apr 28 17:44:34 <juliank> mvo: tshepang: It was not much to think about, mostly just documentation paper work. Apr 28 17:57:02 <juliank> mvo: There's another problem if you change something unrelated with this approach, you get empty changes back as all changes have been marked already Apr 28 17:57:13 <juliank> So both cases do not really work that well Apr 28 17:57:55 <juliank> debFile is only expected to work alone, one DebPackage per cache with no further changes Apr 28 18:04:12 * mvo nods Apr 28 18:04:27 <mvo> I will run out for dinner in a little bit Apr 28 18:08:17 <tshepang> juliank, r u aware that "[] == None" always returns False Apr 28 18:09:20 <tshepang> *always as in only "None == None" can ever return True Apr 28 18:11:34 <juliank> tshepang: Everyone knows that. That's what I told you yesterday. Apr 28 18:11:49 <tshepang> oh Apr 28 18:11:56 <tshepang> ok Apr 28 18:12:18 <tshepang> me didnt understand Apr 28 18:24:04 * tshepang has quit (Ping timeout: 480 seconds) -- Julian Andres Klode - Debian Developer, Ubuntu Member See http://wiki.debian.org/JulianAndresKlode and http://jak-linux.org/.
Attachment:
pgpBP1IsyLOgP.pgp
Description: PGP signature