[Rpm-maint] [Patch] rpmtsAddInstallElement() failure checking in python

Panu Matilainen pmatilai at laiskiainen.org
Thu May 24 19:29:29 UTC 2007


On Thu, 24 May 2007, seth vidal wrote:

> On Tue, 2007-05-22 at 11:26 +0300, Panu Matilainen wrote:
>> Attached patch (mentioned in the payload format patch commentary) makes
>> the python bindings throw an exception when ts.addInstall() fails, whereas
>> previously it would just silently fail.
>
> What things cause the failure?

Incompatible (eg trying to install a delta rpm directly) or corrupted 
packages, most likely in yum localinstall scenarios.

>> pyrpmError might be a better exception to raise, the TypeError is there
>> because in theory, any callers should already be catching TypeError as
>> there are a couple of ways it could raise a TypeError exception as is. But
>> IIRC at least yum doesn't because the other causes are somewhat obscure.
>> OTOH if rpmtsAddInstallElement() fails the install will fail in one way or
>> the other anyway, it's just a matter of failing in a different way.
>>
>> Another alternative could be returning the result from
>> rpmtsAddInstallElement() directly, so that it'd be possible to check but
>> unmodified clients would behave the same as they are now - silently
>> ignoring and failing late.
>>
>> Raising an exception seems like the right thing to do, but do people think
>> it's too disruptive a change for a maintenance release of rpm?
>
> I don't see any harm in adding in the exception, I just have a feeling
> it won't matter a hill of beans. :)

Probably so, but end users end up doing the strangest things. With more 
delta rpms floating around these days (and expecting even more in the 
future) it's only a matter of time :)

I actually already added the exception (rpm.error), it's better to catch 
badness early than for potentially execute scripts from a package that 
can't be installed (eg a delta rpm) and die with ugly "cpio magic" errors 
very late in the process.

 	- Panu -



More information about the Rpm-maint mailing list