[Aptitude-devel] Experimental package and package_pool implementation

[Daniel Burrows]

On Wed, Jul 14, 2010 at 12:31 PM, Piotr Galiszewski
<piotr at galiszewski.pl> wrote:
> 2010/7/14 Daniel Burrows <dburrows at google.com>:
>>  So, let me ask my main question more directly: suppose I have
>> two different package tabs with different filters.  How do they end
>> up displaying different lists of packages, if they both just show all
>> the packages in the singleton package_pool?
>>
>
> It is the power of Qt models ;) Model is only asked about what should
> be displayed. Sorting and filtering is done by proxy model, which
> decides is one particular package should be displayed or not. So model
> always contains reference to all objects in view (is not entirely true
> because data could lazy loaded), and by default all packages are
> displayed. But each view will have different proxy model. And every
> proxy model decides which item will be displayed. My sample code from
> later patch:

  Ah, ok.  So this is a model, but it's not the model that the view sees;
indices into the pool are stored in the proxy models, which are what
is actually displayed.  That makes sense.

  Another minor optimization of the pool: do you think you could
store a Package * instead of a PkgIterator in the "package" object?  A
PkgIterator actually just bundles a pointer to the cache with a
Package * object, so if you're copying every package into your pool,
you'll end up with a ton of pointers to the same cache.  Similarly for
your versions.

  I don't do this in most places in aptitude, but since you're building
a big pile of references to packages, it's probably worth it.  There's
some similar code in aptitude_resolver_universe.h if you want
something to compare to.

  Also, I happened to notice this:

    	// This should be a const reference. Unfortunately
ver.PriorityType() discards
	// const qualifier. Due to that it has to be mutable,

  I see you found one workaround.  The normal thing that aptitude
does here is to use const_cast:

  const_cast<pkgCache::VerIterator &>(ver).PriorityType();

  Pretty gross, but perfectly safe since this is really a const method
that's missing its qualifier.  And since you have a core wrapper around
the apt objects, you only have to have the grossness in one place
instead of having it infect your whole codebase...

  (I haven't taken an exhaustive look at the patch yet, these are just
some things I noticed while glancing at it briefly)

  Daniel