I've been trying to update a pkgsrc module and I've run into a situation
where the best way to enable an option seems to be to optionally patch
the source before building, using a specific patch selected from a set
of multiple optional patches (included in say ${FILESDIR}), based on
some setting in mk/bsd.prefs.mk and/or /etc/mk.conf.

Note that in this case I'm changing, or more accurately "localising",
the default behaviour of the package to match user-selectable features
set in mk/bsd.prefs.mk and/or /etc/mk.conf.  There's no sane way with
this particular patch to hide run-time configuration of these features
from the user without actually changing a distributed file directly.  It
is also _highly_ desirable to change the default features to match local
preferences since many users will rely on this behaviour being correctly
selected by default (i.e. I cannot rely on local users to do their own
runtime configuration of this feature -- it really is a necessary
localisation that the users will need).

I can almost make this work with $PATCHFILES

	.if exits(${FILESDIR}/patch-${FEATURE})
	PATCHFILES+=	${FILESDIR}/patch-${FEATURE}
	.endif
	
but unfortunately the distinfo checksumming feature interferes since
$PATCHFILES are treated like $DISTFILES.  I can hack around that with a
custom target to assist maintainers in checksumming these files:

	do-all-makesums:
		${MAKE} PATCHFILES=${FILESDIR}/patch-* makesum

but that doesn't really work portably as it causes my working directory
to be included in the pathname to the files -- normally they'd be in
$DISTDIR.  There's no fix for this because just using "files/patch-*"
makes the system think there are files that need downloading since it
prefixes $DISTDIR to every relative name given in $PATCHFILES.

My quick hack to avoid the above problem is to add an expansion of
"${LOCAL_PATCHFILES}" in mk/bsd.pkg.mk to the loop where $PATCHFILES are
now iterated.

***************
*** xxxx,yyyy ****
  
  .if !target(do-patch)
  do-patch: uptodate-digest
! .  if defined(PATCHFILES)
  	@${ECHO_MSG} "${_PKGSRC_IN}> Applying distribution patches for ${PKGNAME}"
  	${_PKG_SILENT}${_PKG_DEBUG}cd ${_DISTDIR}; \
! 	  for i in ${PATCHFILES}; do \
  		if [ ${PATCH_DEBUG_TMP} = yes ]; then \
  			${ECHO_MSG} "${_PKGSRC_IN}> Applying distribution patch $$i" ; \
  		fi; \
--- XXXX,YYYY ----
  
  .if !target(do-patch)
  do-patch: uptodate-digest
! .  if defined(PATCHFILES) || defined(LOCAL_PATCHFILES)
  	@${ECHO_MSG} "${_PKGSRC_IN}> Applying distribution patches for ${PKGNAME}"
  	${_PKG_SILENT}${_PKG_DEBUG}cd ${_DISTDIR}; \
! 	  for i in ${PATCHFILES} ${LOCAL_PATCHFILES}; do \
  		if [ ${PATCH_DEBUG_TMP} = yes ]; then \
  			${ECHO_MSG} "${_PKGSRC_IN}> Applying distribution patch $$i" ; \
  		fi; \


then of course I just have to do

	.if exits(files/patch-${FEATURE})
	PATCHFILES+=	${FILESDIR}/patch-${FEATURE}
	.endif

(I don't use FILESDIR in the exists() because I would like to do this
after the include of "../../bsd.prefs.mk", but before the include of
"../../bsd.pkg.mk", and of course FILESDIR ise defined in the latter.)


I also considered doing something like this:

	pre-patch:
		cp ${FILESDIR}/${SELECTED_OPTIONAL_PATCHFILE} \
			${PATCHDIR}/patch-local-${SELECTED_OPTIONAL_PATCHFILE}

	post-patch:
		rm -f ${PATCHDIR}/patch-local-${SELECTED_OPTIONAL_PATCHFILE}

but I'd rather not interfere with patches/patch-local-* files....


I suppose there are other ways this could be done, perhaps even using
'sed' in this one case, though in general I think patch is the best tool
here, and I think my hack to add LOCAL_PATCHFILES is close to ideal,
though better might be to rename PATCHFILES to DIST_PATCHFILES and just
call the new variable PATCHFILES....

Should I send-pr some variant of the above, and if so which?

Any better suggestions?

-- 
								Greg A. Woods

+1 416 218-0098;  <gwoods@acm.org>;  <g.a.woods@ieee.org>;  <woods@robohack.ca>
Planix, Inc. <woods@planix.com>; VE3TCP; Secrets of the Weird <woods@weird.com>