Proposal: mechanism for local patches

[Wesley Shields]

On Wed, Dec 03, 2008 at 04:12:34PM +0300, Dmitry Marakasov wrote:
> * G. Paul Ziemba (pz-freebsd-ports at ziemba.us) wrote:
> 
> > I, too, am happy with the idea of an administrator-specifiable parallel
> > tree that would have the same structure as /usr/ports. So this approach
> > involves, for the administrator (I am restating Dmitry's comments):
> > 
> >     1. in /etc/make.conf, define USE_LOCALPATCHES=<patch-path>
> >     2. put patches in <patch-path>/<category>/<portname>/patch-*
> > 
> > If I may offer comments on Dmitry's draft patch:
> > 
> > 1. Good that it's at the end of the do-patch target - that way local
> >    patches can happen after the "official" patches
> 
> Not sure if it's good actually.
> 
> On the one hand, you usually have patches against vanilla sources, and
> just want to drop them to some dir and have them applied.
> Also, there's USE_DOS2UNIX that comes before any actual patching, so for
> ports that use USE_DOS2UNIX you'll have to adapt patches by hand.
> 
> On the other hand, this may cause conflicts with patches from ports,
> and those would be hard to resolve.

I'd like to see any local patches applied after everything from the
normal patch process is applied.  It makes sense that local
modifications happen after the normal process.  This means that some
patches may have to be massaged a bit.  By doing local patching early
you run into the same problem where the normal patches may have the be
massaged a bit.  I'd rather local patches be massaged as they will not
be over-written during the next portsnap run.

> > 2. I'm not sure we need the test for *.orig|*.rej|*~|*,v, but it
> >    wouldn't hurt. Maybe it helps admins who are actively developing
> >    local patches. I see that it's in the existing do-patch code above.
> 
> I suppose that check was done to help to detect patching failures, so it
> may be removed.

I imagine this is because some people may have those left over from
updates.  We should continue to not use these files, just like the
normal patching process does.

> > 3. Does ${OPSYS} belong in the echoed messages for local patches?
> 
> Maybe not, I just copypasted the chunk. Yes, while what comes from
> portstree is definitely `FreeBSD', local patches are more just simply
> local & system-independent. I guess even patch- prefix is not required,
> but I'll keep it for now.
> 
> Updated version here:
> http://people.freebsd.org/~amdmi3/local-patchdir.patch
> 
> - Renamed some vars
>   I guess LOCALPATCHDIR name is consistent with other user-settable
>   directories (PORTSDIR, PKGDIR etc.) and internal PATCHDIR_LOCAL is
>   (somewhat) consistent with PATCHDIR
>   I guess s/LOCALPATCHDIR/LOCALPATCHESDIR/ may be even better
> - Added comment in LOCALPATCHDIR
> - Now uses 2 level tree under LOCALPATCHDIR (category/port)
> - Removed some (likely unneeded) checks

A quick review looks good to me.  I would have left the ${OPSYS} piece
in there and gone with "Applying local ${OPSYS}..." to be more
consistent with the description of other patches being applied.  But
that's just a minor nit and no big deal in the grand scheme of things.

This sounds like a good idea to me and would love to see it get
committed.

-- WXS