[Rpm-maint] Strange test in doPatch/doUntar

[Panu Matilainen]

On 01/28/2015 05:08 PM, Jean Delvare wrote:
> Hi all,
>
> While working on the rpmbuild performance patch I send last week, I
> noticed something curious in file build/parsePrep.c, functions doPatch
> and doUntar. These two functions look quite similar in the way they are
> called and what they do. However they have different conditions for
> early exit. doPatch has:
>
>     /* On non-build parse's, file cannot be stat'd or read. */
>      if ((spec->flags & RPMSPEC_FORCE) || rpmFileIsCompressed(fn, &compressed) || checkOwners(fn)) goto exit;
>
> while doUntar has:
>
>      /* XXX On non-build parse's, file cannot be stat'd or read */
>      if (!(spec->flags & RPMSPEC_FORCE) && (rpmFileIsCompressed(fn, &compressed) || checkOwners(fn))) {
> 	goto exit;
>      }
>
> While I will gladly admit my ignorance regarding what a "non-build
> parse" is, I find the difference in logic rather suspect. Can anyone
> more familiar with the code double check if one of these conditions is
> incorrect?

Non-build parse is a spec query basically, such as 'rpmspec -q foo.spec' 
or using the API.

>
> Looking at the git history, I can see that the conditions were similar
> up to this commit which changed the condition in doPatch:
>
> commit 54b027535f95fea9088f2cd12ed765fddb20c5c1
> Author: Panu Matilainen
> Date:   Wed Mar 18 14:53:34 2009 +0200
>
>      Don't try to parse %patch on spec query (rhbz#487855)
>
> So I would bet that the condition in doUntar is incorrect and should be
> fixed the same way?

This is rpm, forget about logic ;)

IIRC the answer is that both cases are behaving incorrectly or at least 
sub-optimally: you'd want patches and tarballs looked at even on 
non-build parse IF they exist locally. RPMSPEC_FORCE presence should 
only cause it to return with non-error if the files are not locally 
present. The current difference comes from patches being handed to 
macros but tar-case being handled in C.

	- Panu -

>
> Thanks,
>