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

Ansible py3 #49764

Closed
wants to merge 2 commits into from
Closed

Ansible py3 #49764

wants to merge 2 commits into from

Conversation

sengaya
Copy link
Contributor

@sengaya sengaya commented Nov 4, 2018

Motivation for this change
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 nox --run "nox-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)
  • Fits CONTRIBUTING.md.

This PR should make #44413 obsolete.
closes #44413

I guess I need to provide an update to the release notes for 19.03, but I would first like to get some feedback it this change has the chance to get merged.

cc @jgeerds @joamaki @c0bw3b

@c0bw3b
Copy link
Contributor

c0bw3b commented Nov 4, 2018

+1 for the removal of ansible_2_4 since it has unpatched CVEs (probably).

@GrahamcOfBorg build ansible_2_7 ansible-lint

@GrahamcOfBorg
Copy link

Success on aarch64-linux (full log)

Attempted: ansible_2_7, ansible-lint

Partial log (click to expand)

wrapping `/nix/store/2shcbr5f4qjpkgblzngihk8j45s76kgv-python3.6-ansible-lint-3.4.23/bin/ansible-lint'...
running install tests
..........................................WARNING: Couldn't open /build/source/test/doesnotexist.yml - No such file or directory
............................................
----------------------------------------------------------------------
Ran 86 tests in 34.807s

OK
/nix/store/dp81rc3xigwfvq4g4i1ai26zx4cb27bx-python3.6-ansible-2.7.1
/nix/store/2shcbr5f4qjpkgblzngihk8j45s76kgv-python3.6-ansible-lint-3.4.23

@GrahamcOfBorg
Copy link

Success on x86_64-linux (full log)

Attempted: ansible_2_7, ansible-lint

Partial log (click to expand)

wrapping `/nix/store/3ls3v2ih5wqn7y72b2w92bafadcchqvy-python3.6-ansible-lint-3.4.23/bin/ansible-lint'...
running install tests
..........................................WARNING: Couldn't open /build/source/test/doesnotexist.yml - No such file or directory
............................................
----------------------------------------------------------------------
Ran 86 tests in 63.424s

OK
/nix/store/baynj3kdq6liw471115ajf62hxg4fmbr-python3.6-ansible-2.7.1
/nix/store/3ls3v2ih5wqn7y72b2w92bafadcchqvy-python3.6-ansible-lint-3.4.23

@c0bw3b
Copy link
Contributor

c0bw3b commented Nov 4, 2018

At least it builds successfully with Python 3.6

@FRidh
Copy link
Member

FRidh commented Nov 5, 2018

Note on staging-next we've switched to python3 = python37.

@@ -4,13 +4,12 @@
, ansible
, pytest
, mock
, isPy3k
, python3Packages
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please never do this for packages called from python-packages.nix!
Where is this package even used?

@@ -1,6 +1,6 @@
{ stdenv, fetchFromGitHub, pythonPackages, ansible }:
{ stdenv, fetchFromGitHub, python3Packages, ansible }:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You might want to replace python3Packages by python3 ...

@@ -11,9 +11,9 @@ pythonPackages.buildPythonPackage rec {
sha256 = "0cnfgxh5m7alzm811hc95jigbca5vc1pf8fjazmsakmhdjyfbpb7";
};

propagatedBuildInputs = with pythonPackages; [ pyyaml six ] ++ [ ansible ];
propagatedBuildInputs = with python3Packages; [ pyyaml six ] ++ [ ansible ];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

... and ansible by ansible.override { python3 = python3; }.
That way, both are using the same set of python packages.

@Mic92
Copy link
Member

Mic92 commented Nov 5, 2018

cc @Lassulus

@dasJ
Copy link
Member

dasJ commented Nov 5, 2018

@Mic92 Shouldn't this go into the release notes? ansible_2_4 was dropped and the change to Python 3 introduced some potential incompatibilites (I see them myself)

@Mic92
Copy link
Member

Mic92 commented Nov 6, 2018

I guess mentioning the python3 switch is worth a changelog line.
Dropping ansible 2.4 is something to be expected since we regularly remove unsupported versions (those that are marked end of life by upstream).

@costrouc
Copy link
Member

costrouc commented Nov 7, 2018

Why are ansible and ansible-list not moved into python-modules and exposed as applications in all-packages for compatibility? Two packages pytest-ansible and ansible-lint already depend on ansible as a library. I am working on two packages ansible-kernel and ansible-runner that both have python 27 and 3.5+ support and depend on ansible as a module. Currently I run into an error because of python 2 -> 3 issues.

@dotlambda
Copy link
Member

Why are ansible and ansible-list not moved into python-modules and exposed as applications in all-packages for compatibility?

Sounds like a sane thing to do.

@costrouc
Copy link
Member

costrouc commented Nov 7, 2018

Added a note here that one of my PRs depend on this one moving to python-modules. There is a jupyter kernel for ansible that has support for python 2.7 and 3+. @sengaya let me know if you need any help!

@dasJ
Copy link
Member

dasJ commented Dec 3, 2018

So what about this? I'd really love to have these features

@c0bw3b
Copy link
Contributor

c0bw3b commented Dec 3, 2018

ansible and ansible-lint are applications, first and foremost.
They should stay visible in all-packages.nix

@dotlambda
Copy link
Member

ansible and ansible-lint are applications, first and foremost.
They should stay visible in all-packages.nix

That's absolutely possible by adding e.g.

anisble = with python3.pkgs; toPythonApplication ansible;

to all-packages.nix.

@dasJ
Copy link
Member

dasJ commented Feb 17, 2019

Bumping this up because I really need it (currently pinning ansible manually to use python 3).

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

8 participants