[Rpm-maint] [PATCH 5/5] Add a generic plugin for use by simple Collections

[Steve Lawrence]

On Fri, 2010-06-18 at 17:00 +0300, Panu Matilainen wrote:
> On Thu, 17 Jun 2010, Steve Lawrence wrote:
> 
> > This patch adds a generic plugin, exec.so, that should be sufficient for the
> > majority of Collection actions. After all packages in a Collection have been
> > installed/removed, this plugin executes the arguments by calling system(3),
> > allowing for a very generic and powerful method to perform many actions.
> >
> > This also adds two sample macros as examples of the format, using the exec.so
> > plugin.
> > ---
> > Makefile.am              |    4 ++--
> > collections/Makefile.am  |   19 +++++++++++++++++++
> > collections/collection.h |   12 ++++++++++++
> > collections/exec.c       |   31 +++++++++++++++++++++++++++++++
> 
> Care to change the directory to "plugins" instead of "collections"? These 
> are plugins afterall, and there will be more than just collection plugins 
> sooner or later, might as well lump them to a common spot.
> 

Not a problem.

> > +rpmRC COLLHOOK_POST_ANY_FUNC(rpmts ts, const char * collname, const char * options)
> > +{
> > +	int rc = RPMRC_FAIL;
> > +
> > +	if (rpmChrootSet(rpmtsRootDir(ts)) || rpmChrootIn()) {
>              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> [snip]
> > +	if (rpmChrootOut() || rpmChrootSet(NULL)) {
>                             ^^^^^^^^^^^^^^^^^^^^^
> 
> These look highly suspect, rpmChrootSet() is done already from 
> rpmtsSetup() and rpmtsFinish(), everything between those shouldn't need 
> anything more than just rpmChrootIn() and rpmChrootOut(). Calling 
> rpmChrootSet(NULL) makes it forget the cwd so further rpmChrootIn/Out 
> calls will fail.
> 

Yep, I'll fix this.

> > +%__collection_font		%{__collectiondir}/exec.so /usr/bin/fc-cache
> > +%__collection_java		%{__collectiondir}/exec.so /usr/bin/rebuild-gcj-db
> 
> For demonstration purposes this is fine, but I'd rather see these go into 
> the relevant packages, eg whatever magic adding/removing fonts needs is 
> not rpm's business, it's fontconfig's business. If the command changed 
> from fc-cache to, say, fc-cache-update, it should be a matter of changing 
> the fontconfig package, not rpm.
> 
> Of course that brings in another complication: the "collection owner" 
> needs to be tracked somehow. Which makes me think it might make sense for 
> the collection tracking to be done simply with requires & provides. Eg 
> fontconfig package could provide "collection(fonts)" and packages wanting 
> to belong to the collection would simply require that collection. I guess 
> it'd need a new dependency flag, eg RPMSENSE_COLLECTION to be able to 
> distinguish them from regular deps, possibly with syntactic sugar .. or 
> perhaps store them in a new kind of dependency set (somewhat like 
> RPMTAG_COLLECTIONS, but with optional versions and flags) ... or 
> something. I need to ponder on this a bit, just food for thought for now.

The main issue we have with this is that rather than being controlled by
RPM, any package can now become a "collection owner" and do whatever it
wants when the collection action is triggered. With that we would have
to deal with the possibility of multiple collection owners and malicious
packages defining their own collection actions and doing bad things. But
I guess they could just do their bad things in %post scripts, so maybe
that point is moot.

- Steve