On Mon, Jan 20, 2025 at 04:45:07PM +0000, Stuart Henderson wrote:
> Currently, if a diff appears to be reversed, patch asks whether to apply
> it with -R, with the default answer 'yes'. If there's no tty (as is the
> case in a dpb build), the default is accepted automatically. This is a
> problem as it means the patch is silently reversed. Building might not
> fail but have unwanted results (consider the case of a security fix
> which was applied via a patch, later committed upstream, and cvs rm of
> the patch was missed).
>
> Previously we used --forward unless PATCH_DEBUG was used. PATCH_DEBUG
> was later made the default (which generally makes sense, as we do want
> it to be easy to see if a patch needs regenerating) and eventually
> removed, changing the default PATCH_*ARGS to those from PATCH_DEBUG.
>
> I think we should probably reinstate --forward.
>
> It does make it a little harder for a ports developer to apply a patch
> where some hunks were applied upstream and some were not (which you
> could do by having patch(1) ask the question and answering 'n' to it),
> but patch(1) doesn't seem to have an option which just does that;
> closest is -s but that also "skips patches for which a file to patch
> can't be found" which I don't think we want either.
>
> The only recent time the warning message from patch shows up in dpb logs
> was with x11/gnome/color-manager where there was a genuine problem with
> this, so I think it's safe to do this without a full bulk.
I also think it is fine to commit this without a bulk.
Apart from being annoying, I think there is a genuine risk in the
current behavior, for example if someone forgets to cvs rm a backported
security fix.
Patch is doing a lot of unpredictable things but this one we can really
live without.
> OK?
Yes please
ok
>
>
> Index: bsd.port.mk
> ===================================================================
> RCS file: /cvs/ports/infrastructure/mk/bsd.port.mk,v
> diff -u -p -r1.1642 bsd.port.mk
> --- bsd.port.mk 14 Jan 2025 15:06:49 -0000 1.1642
> +++ bsd.port.mk 20 Jan 2025 16:34:11 -0000
> @@ -770,8 +770,8 @@ PATCHORIG ?= .orig.port
> PATCH_STRIP ?= -p0
> PATCH_DIST_STRIP ?= -p0
>
> -PATCH_ARGS ?= -d ${WRKDIST} -z ${PATCHORIG} -E ${PATCH_STRIP}
> -PATCH_DIST_ARGS ?= -z ${DISTORIG} -d ${WRKDIST} -E ${PATCH_DIST_STRIP}
> +PATCH_ARGS ?= -d ${WRKDIST} -z ${PATCHORIG} -E ${PATCH_STRIP} --forward
> +PATCH_DIST_ARGS ?= -z ${DISTORIG} -d ${WRKDIST} -E ${PATCH_DIST_STRIP}
> --forward
While you're touching this, would you mind changing the order of -z and -d
so that the two lines match?
PATCH_DIST_ARGS ?= -d ${WRKDIST} -z ${DISTORIG} -E ${PATCH_DIST_STRIP} --forward
>
> .if ${PATCH_CHECK_ONLY:L} == "yes"
> PATCH_ARGS += -C
