On Nov 7, 2008, at 12:11 PM, Arkadiusz Miskiewicz wrote:

> On Friday 07 of November 2008, Jeff Johnson wrote:
>> On Nov 7, 2008, at 10:58 AM, Jason Corley wrote:
>>> On Fri, Nov 7, 2008 at 9:35 AM, Jeff Johnson <n3npq@mac.com> wrote:
>>>> But there are rpmbuild behavior changes, now reported as issues
>>>> #40 and #41, and "show-stoppers" for PLD using rpm-5.x.
>>>
>>> For some reason I read this as bugs in PLD's bug tracker, not PLD  
>>> put
>>> bugs in rpm5's bug tracker...  Seems like both tickets are of the
>>> "should fail on non-existence" type and while I'm not sure I agree
>>> that those are bugs in rpm I have to ask: How hard would it be to  
>>> add
>>> a test/exit condition to the macro?
>>
>> There are two levels of exit, one for expansion, the other for the
>> execution
>> of the expansion.
>>
>> It __IS__ one line of shell once the expansion is complete, and "||
>> exit 1"
>> __COULD__ be added to the pipeline trivially:
>
> It doesn't fail with exit 1 - tested. See below.
>
> %prep
> %setup -q -n %{name}
> %patch0 -p1
>
> Source0: exists
> Patch1: points_to_notexisting_file
>
> [arekm@jbj SPECS]$ rpmbuild -bb alien.spec
> Fetching(Patch0): alien-DESTDIR.patchxx
> error: Fetching Patch0 failed: Unknown or unexpected error
> error: Missing Patch0: alien-DESTDIR.patchxx: No such file or  
> directory
> Executing(%prep): /bin/sh -e /home/users/arekm/tmp/rpm-tmp.33473
> + umask 022
> + cd /home/users/arekm/rpm/BUILD
> + exit 0
>

We differ in the definition of "it".

The non-existence of a patch is trigger to attempt a fetch. At one
point in time, there was a disabler in place for each of these
macros
	%_sourcedir             %{_topdir}/SOURCES
	%_patchdir              %{_sourcedir}
	%_icondir               %{_sourcedir}
so that the fetch was attempted iff %_Lpatchdir and %_Rpatchdir were
defined (L == local path, R == remote path usually a URI).

But the discussion of
	Should rpm be a network client? With internal or external?
has led to a lightweight macro expansion fetch on non-existent
patch using an external wget/curl helper.

Then we get to the question whether %prep should fail immediately, or  
continue
"best effort" so that all failures of %prep, not just the first,
can be displayed before %prep exits.

You want instant failure on patch non-existence as "it". Meanwhile there
is additional behavior coupled onto the non-existent patch state, i.e.  
two
forms of fetching. ANd I would claim that continuing "best effort" to  
end
of %prep (as dictated by -bp) is more useful because all, not the  
next, errors
will be seen. That's my definition for "it".

> rpm-5.2a1
>
> - Source is not unpacked, no idea why since %prep is run
>

Why does it matter whether source is unpacked for a command that we
both agree needs to fail and exit?

> - patch doesn't exist but since it's supposed to be "just macro"  
> then why
> there is no expansion here and why it doesn't try to apply it?
>

Why is the non-existence of an expansion for a failed command of  
interest? Dunno
the answer, but its likely coupled to obscure error handling pathways  
through
rpmbuild headed for exit.

> - error: Missing Patch0: alien-DESTDIR.patchxx: No such file or  
> directory - is
> that from patch expansion? Doesn't look so - why it doesn't fail  
> here then?
>

That error is likely from build/files.c:772:
         if (errno != ENOENT) {
             ec++;
             rpmlog(RPMLOG_ERR, _("Missing %s%d %s: %s\n"),
                 ((sp->flags & RPMFILE_SOURCE) ? "Source" : "Patch"),
                 sp->num, sp->source, strerror(ENOENT));
             goto bottom;
         }

>
and "it" doesn't fail "here" because "goto bottom" ends up doing
	return ec
and
	ec++
is not being done in the snippet above.

But the real flaw(s) have nothing to do with whether ec == 0 or not.  
Driving
fetching and existence testing as a side effect of parsing/expanding is
not the right or best implementation.

73 de Jeff