cvs commit: ports/games/doom Makefile ports/games/doom/files patch-ag patch-sndserv__soundsrv.c patch-sndserv__wadread.c

[Adam Weinberger]

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
> 
> 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.
> 
> 
> So don't worry about the inodes, worry about the quality.
> 
> 
> Edwin

But OTOH, using a REINPLACE saves you from having to regenerate patches 
for every single update. It might make initial patching a bit trickier, 
but it can prevent unexpected problem from popping up in the future.

In my eyes, saying that people shouldn't use pre-patch instead of patch 
files to prevent difficult sequential patching is akin to saying that 
people shouldn't own cars because they might slam their fingers in the 
door ::P

# Adam


-- 
Adam Weinberger
adamw at magnesium.net || adamw at FreeBSD.org
adamw at vectors.cx    ||   adamw at gnome.org
http://www.vectors.cx