Bug 250698

Summary: Mk/Scripts/do-patch.sh: not failing on missing patch file
Product: Ports & Packages Reporter: John Hein <jcfyecrayz>
Component: Individual Port(s)Assignee: freebsd-ports-bugs (Nobody) <ports-bugs>
Status: Closed Not Accepted    
Severity: Affects Many People CC: portmgr, tatsuki_makino
Priority: ---    
Version: Latest   
Hardware: Any   
OS: Any   
See Also: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=250723
Attachments:
Description Flags
[patch] fix undetected cat_file failure jcfyecrayz: maintainer-approval? (portmgr)

Description John Hein 2020-10-28 17:43:07 UTC 
Created attachment 219173 [details]
[patch] fix undetected cat_file failure

If a patch file does not exist, do-patch.sh does not trigger a build failure.  It appears this was broken in r533459 (2020-04-30).

See also bug 250530 comment 10.

The attached patch checks the return value of cat_file first, then the pipeline to do_patch().  This isn't great efficiency-wise, because of the extra cat, but it catches more potential failures than just [ ! -r "$file" || ! ! -f "$file" ] or similar tests - for instance, if the patch is compressed and the decompression fails for some reason.
Comment 1 John Hein 2020-10-28 19:08:03 UTC 
(In reply to John Hein from comment #0)
Correction: I think this was broken in do-patch.sh from day 1.  r533459 just shuffled things around as far as the particular piece that is addressed in this bug.
Comment 2 Mathieu Arnold freebsd_committer freebsd_triage 2020-10-29 13:22:54 UTC 
In which case is this supposed to happen?
Comment 3 Mathieu Arnold freebsd_committer freebsd_triage 2020-10-29 14:49:29 UTC 
If you have a use case, would a simple "set pipefail" at the top of the script not be enough?
Comment 4 Mathieu Arnold freebsd_committer freebsd_triage 2020-10-29 16:00:56 UTC 
I have opened review D27007 and bug #250723 with a more global pipefail patch.
Comment 5 John Hein 2020-10-29 16:51:14 UTC 
(In reply to Mathieu Arnold from comment #3)
See bug 250530 comment 10 for a real failure case (devel/llvm10 before ports r553456).

pipefail seems like it is a good alternative fix, yes.

I have not tested your wider pipefail patch from bug 250723 to see if anything breaks that shouldn't, but conceptually it seems like it is reasonable.
Comment 6 Tatsuki Makino 2020-10-29 23:35:38 UTC 
It's not written in porter's handbook, but we can use make -D PATCH_DEBUG patch to debug the patch.

Why don't you just run it before you ask them to commit? :)
Comment 7 John Hein 2020-11-05 19:54:24 UTC 
Closing this bug in favor of the fix in bug 250723.