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

[Panu Matilainen]

On Fri, 18 Jun 2010, Panu Matilainen wrote:
>
>> +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.

Oh, another thing wrt chroots: do you have some specific reason to leave 
the chroot handling for the plugins to handle by themselves, instead of 
just doing it in rpmtsRunCollection()?

Entering braindump mode...

And actually looking at rpmtsRunCollection() and how its called etc... 
seems to me the whole thing could be buried inside rpmte.c around 
rpmteProcess() so rpmteLastInCollectionAdd() and friends wouldn't be 
needed even in internal API.

Errors from collection hooks is something to think about too. Currently 
rpm only considers elements failed when %pre, %preun or payload processing 
fails, but does callback notifications for all scriptlet failures (with 
warning/error indicator). The collections patch set makes collection-hook 
failures to show up in rpmtsRun() return code as errors while not going 
through the other mechanisms (marking elements failed, notifying through 
callback). Which makes the already fuzzy semantics even fuzzier :)

 	- Panu -