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

[Ben Woodcroft]

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