Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 11 Oct 2005 09:36:09 +0000
From:      Alexey Dokuchaev <danfe@FreeBSD.org>
To:        Edwin Groothuis <edwin@mavetju.org>
Cc:        cvs-ports@FreeBSD.org, Jean-Marc Zucconi <jmz@FreeBSD.org>, cvs-all@FreeBSD.org, ports-committers@FreeBSD.org
Subject:   Re: cvs commit: ports/games/doom Makefile ports/games/doom/files patch-ag patch-sndserv__soundsrv.c patch-sndserv__wadread.c
Message-ID:  <20051011093609.GA75258@FreeBSD.org>
In-Reply-To: <20051010234044.GB1239@k7.mavetju>
References:  <200510101133.j9ABXWg4000289@repoman.freebsd.org> <20051010125906.GA3640@FreeBSD.org> <20051010234044.GB1239@k7.mavetju>

next in thread | previous in thread | raw e-mail | index | archive | help
On Tue, Oct 11, 2005 at 09:40:44AM +1000, Edwin Groothuis wrote:
> On Mon, Oct 10, 2005 at 12:59:06PM +0000, Alexey Dokuchaev wrote:
> > On Mon, Oct 10, 2005 at 11:33:31AM +0000, Jean-Marc Zucconi wrote:
> > > jmz         2005-10-10 11:33:30 UTC
> > > 
> > >   FreeBSD ports repository
> > > 
> > >   Modified files:
> > >     games/doom           Makefile 
> > >     games/doom/files     patch-ag 
> > >   Added files:
> > >     games/doom/files     patch-sndserv__soundsrv.c 
> > >                          patch-sndserv__wadread.c 
> > >   Log:
> > 
> > 	...
> > 
> > >   Replace post-patch with real patch files.
> > 
> > I've always been under impression that we try to avoid creating trivial
> > patches when desired functionality can be implemented using some
> > inplace-editing tools.  Could you elaborate on what you've done here?
> 
> Inplace patches used for other things besides replacing FreeBSD
> specific variables (X11BASE, PREFIX etc) are a bad habbit because
> they obscure what is actually being replaced:
> 
>     #include <stdlib.h>
>     #ifdef LINUX
>     /* Linux: We need malloc.h for malloc() and friends */
>     #include <malloc.h>
>     #endif
> 
> Now get your s/malloc.h/stdlib.h/ over it:
> 
>     #include <stdlib.h>
>     #ifdef LINUX
>     /* Linux: We need stdlib.h for malloc() and friends */
>     #include <stdlib.h>
>     #endif

When I use inplace tools, I always check (with diff(1)) that I change
correct occurrence(s) of match pattern.  So it's actually a matter of
porter's accuracy and diligence.

> 
> I admit, it doesn't matter, but when you are looking through the
> code (*waves at the maintainer who has to fix a problem*) this piece
> of code actually looks silly. Hello FreeBSD ports collection!
> 
> 
> Second reason: Using pre-patch inplace patches and patch-files on
> the same file is a recipe for disaster for the maintainer when you
> do the inplace first and the patch-files next (imagine having a
> line within the patches file comparing range changed) and using
> post-patch inplace patches leaves you with an invalid .orig file
> to compare the patched file to.

My experience tells me that managing (maintaining, updating, viewing
changes, etc) patch files are actually harder than carefully written and
documented inplace regexps: patches are simply generated result, and
inplace commands actually reveal the exact meaning of required changes.

I must also confess and I hate reading diffs to diffs, and inplace
commands tend to be more stable, largely because patches are rather
context-specific.

./danfe



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20051011093609.GA75258>