On Tue, Jun 16, 2020 at 03:05:14PM +0200, Marc Espie wrote:
> I was under the impression that you need if test under set -e.
Ah, you don't.
> Please do
>
> if ... fi anyway
>
> I definitely prefer doing it that way.
Sure.
> > > I would just display failed patches if $$error, the "can't find patch
> > > matching $$i" happens so seldom that it won't be confusing.
> > Why have inconsistent output in those seldom cases if we can do it
> > properly for relevant cases only?
>
> Because it makes for simpler logic. If you want to keep the test,
> it should definitely be under if $$error and not outside of it.
Makes sense, I'll do that.
> -u is not an option... we have a few things that are intentionally not
> in the environment, like _DEPENDS_CACHE, that break -u entirely.
I don't want to add -u, my point simply is setting the variable up front
never hurts.
Is this OK with you?
Index: infrastructure/mk/bsd.port.mk
===================================================================
RCS file: /cvs/ports/infrastructure/mk/bsd.port.mk,v
retrieving revision 1.1540
diff -u -p -r1.1540 bsd.port.mk
--- infrastructure/mk/bsd.port.mk 9 Jun 2020 11:01:08 -0000 1.1540
+++ infrastructure/mk/bsd.port.mk 16 Jun 2020 13:23:00 -0000
@@ -2758,6 +2758,7 @@ ${_PATCH_COOKIE}: ${_EXTRACT_COOKIE}
@${_MAKE} _internal-distpatch
. endif
@if cd ${PATCHDIR} 2>/dev/null || [ x"${PATCH_LIST:M/*}" != x"" ]; then
\
+ failed_patches=''; \
error=false; \
for i in ${PATCH_LIST}; do \
case $$i in \
@@ -2773,6 +2774,7 @@ ${_PATCH_COOKIE}: ${_EXTRACT_COOKIE}
if [ -s $$i ]; then \
${_PBUILD} ${PATCH}
${PATCH_ARGS} < $$i || \
{ echo "***>
$$i did not apply cleanly"; \
+
failed_patches="$$failed_patches\n $$i"; \
error=true; }; \
else \
${ECHO_MSG} "===>
Ignoring empty patchfile $$i"; \
@@ -2786,7 +2788,12 @@ ${_PATCH_COOKIE}: ${_EXTRACT_COOKIE}
;; \
esac; \
done;\
- if $$error; then exit 1; fi; \
+ if $$error; then \
+ if [ -n "$$failed_patches" ]; then \
+ echo "===> Failed patches:
$$failed_patches\n"; \
+ fi; \
+ exit 1; \
+ fi; \
fi
# End of PATCH.
.endif
