svn commit: r533459 - head/Mk/Scripts

[Bryan Drewery]

On 5/6/20 7:40 AM, Mathieu Arnold wrote:
> On Mon, May 04, 2020 at 10:49:34AM -0700, Bryan Drewery wrote:
>> On 4/30/2020 7:48 AM, Mathieu Arnold wrote:
>>> Author: mat
>>> Date: Thu Apr 30 14:48:22 2020
>>> New Revision: 533459
>>> URL: https://svnweb.freebsd.org/changeset/ports/533459
>>>
>>> Log:
>>>   When one patch fails, continue with appling patches.
>>>   
>>>   This makes the process of make patch, fix fallout, make makepatch much
>>>   easier as only one iteration is required.
>>>   
>>>   While there, refactor things around, and streamline messages passed
>>>   between functions.
>>>   
>>>   PR:		244626
>>>   Reported by:	cem
>>>
>>
>> Will this fail between stages? If a files/patch-* patch fails it should
>> not move on to extra patches as then makepatch / diffing will be wrong.
>> Even patch-* can lead to being wrong when considering local (not in
>> ports tree) patches.
>>
>> I don't think this is good default behavior.
> 
> No, it will try to apply all patches, and report all the failures.
> 
> I am not sure where you are getting at.
> 
> If you run `make patch; make makepatch` the usage would have been broken
> before and still is.
> 
> If you run `make patch makepatch`, the usage does not change, if
> patching fails, it does not go into makepatch target, and if patching is
> ok, then makepatch runs and updates your patches.
> 

Not all patches are made with makepatch which is why I referenced
'diffing'. By 'stages' I mean exactly everything in 'make patch'.

Multiple files in files/ can and do affect the same files in the work
directory. Just applying 2 broken patches with 'make patch' now would be
incredibly confusing and make it difficult to fix each of those patch files.

A simple example is security/openssh-portable with these 3 files:
  files/patch-servconf.c
  files/extra-patch-hpn
  files/extra-patch-version-addendum

All modify servconf.c. All can fail to apply. Once they do fail it is
impossible to work out the problem and fix any of the patches.

> ===>  Applying extra patch /root/svn/ports/security/openssh-portable/files/extra-patch-hpn with -p2
> 1 out of 6 hunks failed--saving rejects to servconf.c.rej
> ===>  FAILED Applying extra patch /root/svn/ports/security/openssh-portable/files/extra-patch-hpn with -p2
> ===>  Applying extra patch /root/svn/ports/security/openssh-portable/files/extra-patch-version-addendum
> 1 out of 1 hunks failed--saving rejects to servconf.c.rej
> ===>  FAILED Applying extra patch /root/svn/ports/security/openssh-portable/files/extra-patch-version-addendum
> ===>  Applying FreeBSD patches for openssh-portable-8.2.p1_1,1
> 1 out of 4 hunks failed--saving rejects to servconf.c.rej
> ====> FAILED Applying FreeBSD patch patch-servconf.c
> ===> Cleanly applied FreeBSD patch(es)  patch-auth.c patch-auth2.c patch-configure.ac patch-readconf.c patch-regress__test-exec.sh patch-serverloop.c patch-session.c patch-ssh-agent.1 patch-ssh-agent.c patch-ssh.c patch-ssh_config patch-ssh_config.5 patch-sshconnect.c patch-sshd.8 patch-sshd.c patch-sshd_config patch-sshd_config.5
> ===> FAILED to apply cleanly FreeBSD patch(es)  patch-servconf.c
> ==> SOME PATCHES FAILED TO APPLY CLEANLY.

> # ls -al work-default/openssh-8.2p1/servconf.c*
> -rw-r--r--  1 root  wheel  88639 May  8 09:09 work-default/openssh-8.2p1/servconf.c
> -rw-r--r--  1 root  wheel  88498 May  8 09:09 work-default/openssh-8.2p1/servconf.c.orig
> -rw-r--r--  1 root  wheel    294 May  8 09:09 work-default/openssh-8.2p1/servconf.c.rej
> -rw-r--r--  1 root  wheel    130 May  8 09:09 work-default/openssh-8.2p1/servconf.c.rej.orig


> # cat work-default/openssh-8.2p1/servconf.c.rej
> @@ -344,7 +349,7 @@
>         if (options->print_lastlog == -1)
>                 options->print_lastlog = 1;
>         >x11_forwarding == -1)
> -               options->x11_forwarding = 0;
> +               options->x11_forwarding = 1;
>         if (options->x11_display_offset == -1)
>                 options->x11_display_offset = 10;
>         if (options->x11_use_localhost == -1)

Where do I even start to deal with fixing those 3 patches? It's not
clear which patch even owns that .rej file. And now the file is tainted
with chunks from 3 different patches so it is impossible to create
proper patches again.

From what I've heard and seen this cannot be default. Clearly all of the
impacted cases were not understood before it was committed. Please
revert or change the default.

-- 
Regards,
Bryan Drewery

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://lists.freebsd.org/pipermail/svn-ports-head/attachments/20200508/55c41144/attachment.sig>