Re: [PATCH] IPython: Use 'python2-variant'.

[Ben Woodcroft <b.woodcroft@uq.edu.au>]

On 19/04/16 05:07, Leo Famulari wrote:
> On Sun, Apr 17, 2016 at 10:54:03AM +1000, Ben Woodcroft wrote:
>> Hi,
>>
>> This hopefully should be a pretty straightforward patch, but I just wanted
>> to check that I was doing the correct thing this first time. Seems to make
>> the code much cleaner.
>>
>> Does it make sense to go through all the python packages and convert them
>> all to the python2-variant style? As I understand the idea is to do this one
>> package per commit, which means traversing python dependency graph? Is
>> anyone on this?
> What do you mean by "all the python packages"? I think that if a plain
> and unadorned package-with-python2 works for a package, then we should
> not change the package to use the python2-variant system.
>
> The python2-variant system is really for packages that require different
> inputs or build arguments for the python-2 and python-3 versions. Some
> of these packages were causing problems throughout the dependency graph
> before introducing this system. For example, see how Ludo untangled some
> knotty code beginning with 00f2bf34. And again in 519e2f4fd.
>
> My plan was to switch remaining "difficult" packages to python2-variant
> when they started causing problems, or when we were going to be changing
> a package anyways.
>
> But, if you want to make the changes and check that all the affected
> packages still work, then I don't see a reason not to do it :)
>
> By the way, here is the bug report on this topic:
> http://bugs.gnu.org/22437

Re-reading that somehow now makes a lot more sense. I'm glad we don't 
have to change everything, apologies for the noise.

>>   (define-public python2-ipython
>> -  (let ((ipython (package-with-python2 python-ipython)))
>> +  (let ((parent (package-with-python2
>> +                 (strip-python2-variant python-ipython))))
> I prefer to the upstream name (ipython) rather than "parent". Some
> people use "base". I'd rather not introduce a 3rd option ;)

Fair point. I'd prefer "base" since then copy and paste works better, 
which is especially useful if the addition of setuptools as a native 
input is the only difference.

> Otherwise it looks okay, assuming you've tested that the referring
> packages still work as expected.

I'll send an updated patch as part of a series I'm preparing.

Thanks for your explanations.
ben