From mboxrd@z Thu Jan 1 00:00:00 1970 From: Mark H Weaver Subject: Re: 04/05: gnu: swig: Patch for Octave 4.4. Date: Wed, 30 May 2018 14:53:23 -0400 Message-ID: <877enl0ycc.fsf@netris.org> References: <20180529222049.16826.20591@vcs0.savannah.gnu.org> <20180529222052.5A8DA20537@vcs0.savannah.gnu.org> <87bmcxbqy4.fsf@netris.org> <87d0xdw645.fsf@posteo.net> Mime-Version: 1.0 Content-Type: text/plain Return-path: Received: from eggs.gnu.org ([2001:4830:134:3::10]:45679) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fO6FJ-0005ma-PS for guix-devel@gnu.org; Wed, 30 May 2018 14:54:54 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1fO6FF-0001jk-Gi for guix-devel@gnu.org; Wed, 30 May 2018 14:54:53 -0400 Received: from world.peace.net ([64.112.178.59]:38500) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1fO6FF-0001fY-Cf for guix-devel@gnu.org; Wed, 30 May 2018 14:54:49 -0400 In-Reply-To: <87d0xdw645.fsf@posteo.net> (Kei Kebreau's message of "Wed, 30 May 2018 10:49:46 -0400") 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: Kei Kebreau Cc: guix-devel@gnu.org Hi Kei, Kei Kebreau writes: > Mark H Weaver writes: > >> Instead, I suggest creating a different 'swig' package for octave that >> inherits from the primary one and adds this patch. > > I agree that this will work. Is the attached patch something like what > you're thinking? Yes, although I have some suggestions: > From 475a7698fd4c88ea688cb43499344e65ffaf5b84 Mon Sep 17 00:00:00 2001 > From: Kei Kebreau > Date: Wed, 30 May 2018 08:34:42 -0400 > Subject: [PATCH] gnu: shogun: Use a patched swig for Octave 4.4. > > * gnu/packages/machine-learning.scm (swig-for-octave): New variable > * gnu/packages/machine-learning.scm (shogun)[inputs]: Replace swig with > swig-for-octave. > * gnu/packages/patches/swig-octave-patches.patch: New file. > * gnu/local.mk (dist_patch_DATA): Register patch. [...] > diff --git a/gnu/packages/machine-learning.scm b/gnu/packages/machine-learning.scm > index e135ee0ee..4d54584a7 100644 > --- a/gnu/packages/machine-learning.scm > +++ b/gnu/packages/machine-learning.scm > @@ -365,6 +365,14 @@ value imputation, classifier creation, generalization error estimation and > sample proximities between pairs of cases.") > (license license:gpl3+))) > > +(define swig-for-octave > + (package/inherit swig > + (name (string-append (package-name swig) "-for-octave")) > + (source > + (origin > + (inherit (package-source swig)) > + (patches (search-patches "swig-octave-patches.patch")))))) For the patches field, how about something like this: (patches (append (origin-patches (package-source swig)) (search-patches "swig-octave-patches.patch"))) so if someone adds a patch to swig, it will automatically get picked up by this one too. Please add a comment above the 'swig-for-octave' definition explaining what it's for. I was confused at first that you didn't change the octave definition, but now I guess that it's for other packages that wish to use octave via swig, or something along those lines? It should be good to explain in a comment. Also, I think it would be better to put the 'swig-for-octave' definition immediately after the one for 'swig', so that it's more likely to be noticed by people working on swig. This package has more connection to swig than it does to shogun. It has no direct connection to 'shogun' except that shogun is currently the only user of it. Another question: instead of adding the patch, how about changing the origin to build from a git checkout? The advantage of that is that it requires less trust from users. I look at this big patch and I wonder if I should check to make sure it matches what upstream has (I didn't). If it's a git checkout, there's less trust needed. It would also make later upgrades easier. It might also be better to call the package "swig-git" or something, dunno. What do you think? Thanks for working on it. Mark