Re: 04/05: gnu: swig: Patch for Octave 4.4.

[Kei Kebreau]

Mark H Weaver <address@hidden> writes:

> Hi Kei,
>
> Kei Kebreau <address@hidden> writes:
>
>> Mark H Weaver <address@hidden> 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 <address@hidden>
>> 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

Other than Shogun's Python/SWIG-related build failure (attached), this
patch seems to work fairly well. There appears to be an upstream issue
related to the invalid conversion mentioned in the build failure. I'm
keeping an eye on it for any new developments.

0001-gnu-shogun-Use-a-patched-swig-for-Octave-4.4.patch
Description: Text document

shogun-build-failure.txt
Description: Text document

signature.asc
Description: PGP signature