[Rpm-maint] Commit fafd8090 (Dead code removal)

[Panu Matilainen]

On Tue, 13 Apr 2010, Michael Schroeder wrote:

> On Tue, Apr 13, 2010 at 09:51:12AM +0300, Panu Matilainen wrote:
>>> Regarding commit fafd80901c129659d4d0c1945f5922858f410ef7, please
>>> test if a refresh of a package which obsoletes itself works.
>>
>> Well, the commit doesn't change any behavior, it just removes dead code
>> which hasn't been active in > 5 years. So if its broken, its been that way
>> for a long long time :)
>
> Yes, I just wanted to bring that topic up again ;-)

Ah, ok then. AFAICT it does behave correctly in rpm 4.6 - 4.8, HEAD is 
currently broken wrt reinstalls in other ways though.

>>> By "refresh" I mean the re-installation of an already installed
>>> package.
>>>
>>> i.e. if package A contains an "Obsoletes: A", try
>>>
>>>   rpm -i A.rpm
>>>   rpm -Uvv --oldpackage A.rpm
>>>
>>> A package refresh shouldn't run the uninstall scriplets (for
>>> whatever reason).
>>
>> Hmm.. with current HEAD you get this:
>> [root at dhcp102 rpm]# ./rpm -Uvh --oldpackage
>> /home/pmatilai/rpmbuild/RPMS/noarch/obstest-self-0.1-1.noarch.rpm
>> error: Failed dependencies:
>> 	obstest-self is obsoleted by (installed) obstest-self-0.1-1.noarch
>
> That's surprising,

Yeah, the above isn't exactly intended behavior.

> my guess was that addObsoleteErasures() doesn't look for self-obsoletes 
> and adds the installed package to the remove list. But in that case 
> rpmtsPrunedIterator() shouldn't have returned the installed package.

That's more or less what happens on upgrade, but reinstall of same NEVR(A) 
is more quirky and is something I hadn't tested wrt the new obsoletes 
handling at all. The reinstall (or refresh if you like) doesn't add an 
erasure element at all, but instead deals with it as a special case inside 
psm and is a bit nasty in various ways, including this.

> Btw, I'm not sure if the installed obsoletes are really just matched
> against the package NEVRs and not the provides. Doesn't
> rpmalSatisfiesDepend() used in unsatisfiedDepend() look at the
> provides?
>
> (And why is checkInstDeps() using unsatisfiedDepend() for conflicts
> and obsoletes, which also looks at other installed packages?
> I don't think it should report problems installed packages have with
> other installed packages. Or am I misreading the code?)

When looking at the installed obsoletes, the iterator is on Obsoletename 
keyed on package name of the added package, so while unsatisfiedDepend() 
does look at the provides, it only runs on headers with obsoletes matching 
the package name. Ditto for conflicts. But I agree its rather obscure and 
suspicious looking. The recent cleanup-round on the dep-checking code was 
more about carving out unused cruft piece by piece while keeping existing 
behavior (whatever it was) than actually making sense out of it :) That's 
worth another clean-up round, no doubt.

 	- Panu -