Bug 250698 - Mk/Scripts/do-patch.sh: not failing on missing patch file
Summary: Mk/Scripts/do-patch.sh: not failing on missing patch file
Status: Closed Not Accepted
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Many People
Assignee: freebsd-ports-bugs (Nobody)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-10-28 17:43 UTC by John Hein
Modified: 2020-11-05 19:54 UTC (History)
2 users (show)

See Also:


Attachments
[patch] fix undetected cat_file failure (610 bytes, patch)
2020-10-28 17:43 UTC, John Hein
jcfyecrayz: maintainer-approval? (portmgr)
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
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 2020-10-29 13:22:54 UTC
In which case is this supposed to happen?
Comment 3 Mathieu Arnold freebsd_committer 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 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.