[pacman-dev] [PATCH 3/3] Added tests for -Q --check (both fast(files) and full(mtree)).

[Andrew Gregory]

On 10/04/13 at 01:18pm, Jeremy Heiner wrote:
> On Fri, Oct 4, 2013 at 11:22 AM, Andrew Gregory
> <andrew.gregory.8 at gmail.com> wrote:
> > There is A LOT going on in this patch.  Please try to break your
> > patches up into smaller changes.  At least mtree and hook support
> > definitely should have been individual patches.
> 
> Hi, Andrew,
> Thanks for the code review!
> Sorry about the size of this patch. It's really due to the fact that
> these things grew organically. If I had been known before I started
> exactly what I needed from the mtree and hook support in order to meet
> my goals, well, then I would have won several lotteries by now. I'd be
> happy to reverse-engineer some additional commits to make it look like
> I was more prescient :).
> 
> > Before I get into specifics, pacman already has "hooks" of a sort:
> > install scripts.  I think I would prefer to see support added for
> > install scripts in local db packages than the ability to run arbitrary
> > python code.  You could then modify files as needed from
> > post_install().  This would match our existing use of install scripts
> > as hooks and could be run within fakechroot.  Anybody else have any
> > thoughts on this?
> 
> I really like that idea. The reason I didn't go down that path was
> that I saw what pmpkg.install_package contained, or rather did not
> contain, and was scared off.
> 
> > The only reason to refactor this code into functions seems to be so
> > that you can run pacman from genhook, but you only do that to install
> > packages, which should actually be done by adding them to the local
> > db, so I'm not really sure what the point of this is.  Furthermore,
> > I'm not sure how wise it is to use pacman to set up its own test
> > environment.
> 
> Testing -Qk(k) requires /var/lib/pacman/local/* to be populated, which
> is not something pmpkg does. So how much more does pmpkg need to do?
> The more it duplicates what pacman does, the higher the ongoing code
> maintenance cost. So where's the sweet spot?

pmdb is responsible for populating /var/lib/pacman/local/.  In fact,
you'll need to update pmdb so it will create the mtree file, which
I seem to have neglected to mention before.

> I would argue that pmpkg should do the minimum (i.e. only what it
> currently does). And that there is no danger in using pacman to set up
> its own test environment because there are extensive unit tests for
> the installation features. And if those pass, then where is the
> danger? It might be nice to add "this test should fail without even
> running unless that test has succeeded" to the specifications, but
> that seems a bit overkill.
> 
> One more subtle thing to keep in mind is the timing of the snapshot.
> Now I didn't really dig deeply into how all the tests make use of the
> snapshot, but for some tests you want it taken between the install and
> the mangling. There is no way to achieve this effect using local
> pmpkgs.

The snapshot is used to determine if the file was modified after being
installed.  None of your tests needed a post-snapshot hook, and
I can't think of any that would.  Of course, if such a need does
arise, we can revisit the idea of post-snapshot hooks at that time.

> But I want to say again that I really like the idea. There's no
> question it is superior to my hooks mechanism in the complexity of the
> testing framework and even the generality. I like it so much that I'm
> going to delay responding to the other points from your code review to
> provide an opportunity for feedback. I think it should be possible to
> find workarounds for the issues I mentioned above, and if that's the
> case then I would happily abandon my hooks idea in favor of install
> scripts.
> Thanks again,
> Jeremy