Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

treewide: Make all usage of python2Packages explicit #61142

Closed
wants to merge 1 commit into from

Conversation

adisbladis
Copy link
Member

@adisbladis adisbladis commented May 8, 2019

Motivation for this change

This is in preparation for Python2 going EOL in 2020.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nix-review --run "nix-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Assured whether relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@grahamc
Copy link
Member

grahamc commented May 8, 2019

I mentioned this on IRC, but just so it isn't lost (and so other people can give opinions too):

I wonder if the expression being callPackaged should change from pythonPackages to python2Packages. I say this remembering back to several PRs now moving overrides from all-packages.nix in to the per-package expression.

@adisbladis suggested using pythonXPackages.callPackage, but I'm not familiar with that.

@ofborg ofborg bot added the 6.topic: xfce The Xfce Desktop Environment label May 8, 2019
@adisbladis
Copy link
Member Author

It may be better to do simple s/pythonPackages/python2Packages/ inside every of the affected some-package.nix (instead of callPackage-overriding at call-sites like all-packages.nix)

I think I'll rebase this on #61139 and use python2Packages.callPackage so we are guaranteed to get the correct python/pythonPackages attributes in every derivation.

@adisbladis
Copy link
Member Author

I meant moving towards removal of pythonPackages treewide (in all-packages.nix and in small .nix-files) keeping only python2Packages and python3Packages.

I'm with you on this one, that's exactly the end goal.

It may be better to do simple s/pythonPackages/python2Packages/ inside every of the affected some-package.nix (instead of callPackage-overriding at call-sites like all-packages.nix)

I disagree, it's too easy to accidentally mismatch python and pythonPackages, that's why I'm considering to change all
somePackage = callPackage ./path { pythonPackages = python2Packages; };
to
somePackage = python2Packages.callPackage ./path { };
so we are always consistent.

@edolstra
Copy link
Member

edolstra commented May 9, 2019

👎 on this approach since it adds a huge amount of clutter to all-packages.nix. Much better to change Nix expressions to take python2Packages as an argument.

@edolstra
Copy link
Member

edolstra commented May 9, 2019

I disagree, it's too easy to accidentally mismatch python and pythonPackages, that's why I'm considering to change all
somePackage = callPackage ./path { pythonPackages = python2Packages; };
to
somePackage = python2Packages.callPackage ./path { };
so we are always consistent.

That seems very ad hoc to me. Just because a package uses python doesn't mean I should have to use pythonPackages.callPackage to call it. I mean, suppose a package uses both Python and Perl, should I use pythonPackages.callPackage or perlPackages.callPackage to instantiate it? This doesn't look like a composable mechanism.

@adisbladis
Copy link
Member Author

Fair enough. I've pushed a a commit with the nix expressions changed instead.

This is in preparation for Python2 going EOL in 2020
@FRidh
Copy link
Member

FRidh commented May 10, 2019

It's good that we're going to be explicit about which version is used, however, I don't think this is the right approach.

In the past many expressions were modified to use python2Packages to indicate they are not compatible with Python 3, with the idea that we may eventually set python = python3;.

The latter never happened and is unlikely to happen as well, however, by making the proposed change we may be taking a step backwards in identifying what packages can use Python 3.

Therefore, I think instead we should set python = python3; (in a temporary branch/job), identify what packages get broken by this, and use python2Packages for those.

@adisbladis
Copy link
Member Author

adisbladis commented Aug 18, 2019

It's good that we're going to be explicit about which version is used, however, I don't think this is the right approach.

In the past many expressions were modified to use python2Packages to indicate they are not compatible with Python 3, with the idea that we may eventually set python = python3;.

The latter never happened and is unlikely to happen as well, however, by making the proposed change we may be taking a step backwards in identifying what packages can use Python 3.

Therefore, I think instead we should set python = python3; (in a temporary branch/job), identify what packages get broken by this, and use python2Packages for those.

You can go ahead and try that, I think it's unlikely that this will happen and even more unlikely that it will see any significant community involvement.
My intent with this PR was to set us up so we can incrementally change 2 -> 3.

As it currently stands we're branching off for 19.09 in ~2 weeks, a release during which Python 2 will go EOL and Python 2 still very much a default.

Since this has bit-rotted so much by now I'm closing it, even though I'm still in favour of the approach taken.

I'm unlikely to do much more work on the Python package sets, it's far too demotivational.

@adisbladis adisbladis closed this Aug 18, 2019
@FRidh
Copy link
Member

FRidh commented Aug 18, 2019

You can go ahead and try that, I think it's unlikely that this will happen and even more unlikely that it will see any significant community involvement.

Which tells you how much the community really cares about it.

Setting all pythonPackages to python3Packages, and then reverting the cases where it is not possible is the way forward. Assuming you've included all the pythonPackages that were there, then doing this effort for 159 packages is really not much work and won't cause much breakage either.

@adisbladis
Copy link
Member Author

@volth I agree.

Having pythonXPackages was a always a dangerous thing and bad design anyway.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants