From mboxrd@z Thu Jan 1 00:00:00 1970 From: Mark H Weaver Subject: Re: [PATCH] gnu: spice: Fix usbredir for 32 bit platforms. Date: Sun, 14 Aug 2016 20:17:53 -0400 Message-ID: <874m6mc2by.fsf@netris.org> References: <87d1llvq1t.fsf@netris.org> <20160808114649.11396-1-david@craven.ch> <877fbke9dy.fsf@netris.org> Mime-Version: 1.0 Content-Type: text/plain Return-path: Received: from eggs.gnu.org ([2001:4830:134:3::10]:50447) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bZ5cX-0001YV-B4 for guix-devel@gnu.org; Sun, 14 Aug 2016 20:19:14 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bZ5cU-00062N-3i for guix-devel@gnu.org; Sun, 14 Aug 2016 20:19:13 -0400 Received: from world.peace.net ([50.252.239.5]:47802) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bZ5cT-00060Z-Vo for guix-devel@gnu.org; Sun, 14 Aug 2016 20:19:10 -0400 In-Reply-To: (David Craven's message of "Sun, 14 Aug 2016 20:50:36 +0200") List-Id: "Development of GNU Guix and the GNU System distribution." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: guix-devel-bounces+gcggd-guix-devel=m.gmane.org@gnu.org Sender: "Guix-devel" To: David Craven Cc: guix-devel David Craven writes: >> Phase procedures are supposed to return a boolean indicating whether >> they succeeded, but 'system*' returns a number: a result code. In >> scheme, all numbers are considered true. Also, you might as well use >> 'lambda' here instead of 'lambda*', so it should look like this: > >> (lambda _ >> (zero? (system* "sh" "autogen.sh"))) > > The reason was that autogen.sh performs some check at the end that fails. > I'm running autoreconf directly now, so that the zero? doesn't cause the > phase to fail. Sounds good, thanks. >> This is no longer version "0.7.1", so the version number needs to be >> updated accordingly. Please see section 7.6.3 (Version Numbers) in the >> manual for our conventions for version numbers of VCS snapshots, and the >> recommended code to generate those version numbers. > > This could have also probably survived until the package gets updated, but > I fixed it. Thank you. I think it's important for the 'version' field to be accurate. If you could push this fix at your earliest convenience, I'd be grateful. (I don't see it in the git repo yet) >> IT might have been better to just use a simple patch to fix the format >> strings than to use a VCS snapshot that might introduce more bugs, but >> I guess we can see how it goes. > > I considered this option also, but in at least one instance I was asked to > use substitute* instead of a patch, so I thought that patches are considered > a last resort. They are certainly not a last resort in general. The preferred method of making changes prior to the build depends on the specific details. However, one important consideration is that patches and snippets change what is considered by Guix to be the "source" of the package as returned by 'package-source' at the Scheme level and "guix build --source" as the command line. With this in mind, I'd say that for bug fixes, patches are generally the preferred method. > I also checked the history before selecting HEAD as the commit, > it looks like there where only a couple of bugfixes, but not much activity > otherwise. So hopefully this does not introduce any new bugs. Oh, okay, that makes sense. Thanks for working on it. Mark