Re: Automatic testing of patches in commit fest

On Wed, Sep 13, 2017 at 2:34 AM, Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org> wrote:
> Tom Lane wrote:
>> Aleksander Alekseev <a(dot)alekseev(at)postgrespro(dot)ru> writes:
>> > I've ended up with this script [1]. It just generates a list of patches
>> > that are in "Needs Review" status but don't apply or don't compile. Here
>> > is the current list:
>>
>> > === Apply Failed: 29 ===
>> > https://commitfest.postgresql.org/14/1235/ (Support arrays over domain types)
>>
>> Can you clarify what went wrong for you on that one? I went to rebase it,
>> but I end up with the identical patch except for a few line-numbering
>> variations.
>
> I think "git apply" refuses to apply a patch if it doesn't apply
> exactly. So you could use "git apply -3" (which merges) or just plain
> old "patch" and the patch would work fine.
>
> If the criteria is that strict, I think we should relax it a bit to
> avoid punting patches for pointless reasons. IOW I think we should at
> least try "git apply -3".

The cfbot is not using git apply, it's using plain old GNU patch
invoked with "patch -p1". From http://commitfest.cputube.org/ if you
click the "apply|failing" badge you can see the log from the failed
apply attempt. It says:

== Fetched patches from message ID 3881.1502471872%40sss.pgh.pa.us
== Applying on top of commit 2d4a614e1ec34a746aca43d6a02aa3344dcf5fd4
== Applying patch 01-rationalize-coercion-APIs.patch...
== Applying patch 02-reimplement-ArrayCoerceExpr.patch...
1 out of 1 hunk FAILED -- saving rejects to file
src/pl/plpgsql/src/pl_exec.c.rej

It seems to be a legitimate complaint. The rejected hunk is trying to
replace this line:

! return exec_simple_check_node((Node *) ((ArrayCoerceExpr
*) node)->arg);

But you removed exec_simple_check_node in
00418c61244138bd8ac2de58076a1d0dd4f539f3, so this 02 patch needs to be
rebased.

> Also, at this point this should surely be just an experiment.

+1

--
Thomas Munro
http://www.enterprisedb.com