From owner-cvs-all@FreeBSD.ORG Tue Oct 11 09:36:09 2005 Return-Path: X-Original-To: cvs-all@FreeBSD.org Delivered-To: cvs-all@FreeBSD.org Received: by hub.freebsd.org (Postfix, from userid 1033) id 67D7216A428; Tue, 11 Oct 2005 09:36:09 +0000 (GMT) Date: Tue, 11 Oct 2005 09:36:09 +0000 From: Alexey Dokuchaev To: Edwin Groothuis Message-ID: <20051011093609.GA75258@FreeBSD.org> References: <200510101133.j9ABXWg4000289@repoman.freebsd.org> <20051010125906.GA3640@FreeBSD.org> <20051010234044.GB1239@k7.mavetju> Mime-Version: 1.0 Content-Type: text/plain; charset=koi8-r Content-Disposition: inline In-Reply-To: <20051010234044.GB1239@k7.mavetju> User-Agent: Mutt/1.4.2.1i Cc: cvs-ports@FreeBSD.org, Jean-Marc Zucconi , 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 X-BeenThere: cvs-all@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: CVS commit messages for the entire tree List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 11 Oct 2005 09:36:09 -0000 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 > #ifdef LINUX > /* Linux: We need malloc.h for malloc() and friends */ > #include > #endif > > Now get your s/malloc.h/stdlib.h/ over it: > > #include > #ifdef LINUX > /* Linux: We need stdlib.h for malloc() and friends */ > #include > #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