From d88a7735d29cf7aeab1753ba4c2efb4654201620 Mon Sep 17 00:00:00 2001 From: adisbladis Date: Thu, 12 Mar 2020 23:29:09 +0000 Subject: [PATCH 1/4] Python: introduce NIX_PYTHONPREFIX in order to set site.PREFIXES This is needed in case of `python.buildEnv` to make sure site.PREFIXES does not only point to the unwrapped executable prefix. -------------------------------------------------------------------------------- This PR is a story where your valiant hero sets out on a very simple adventure but ends up having to slay dragons, starts questioning his own sanity and finally manages to gain enough knowledge to slay the evil dragon and finally win the proverbial price. It all started out on sunny spring day with trying to tackle the Nixops plugin infrastructure and make that nice enough to work with. Our story begins in the shanty town of [NixOps-AWS](https://github.com/nixos/nixops-aws) where [mypy](http://mypy-lang.org/) type checking has not yet been seen. As our deuteragonist (@grahamc) has made great strides in the capital city of [NixOps](https://github.com/nixos/nixops) our hero wanted to bring this out into the land and let the people rejoice in reliability and a wonderful development experience. The plugin work itself was straight forward and our hero quickly slayed the first small dragon, at this point things felt good and our hero thought he was going to reach the town of NixOps-AWS very quickly. But alas! Mypy did not want to go, it said: `Cannot find implementation or library stub for module named 'nixops'` Our hero felt a small sliver of life escape from his body. Things were not going to be so easy. After some frustration our hero discovered there was a [rule of the land of Python](https://www.python.org/dev/peps/pep-0561/) that governed the import of types into the kingdom, more specificaly a very special document (file) called `py.typed`. Things were looking good. But no, what the law said did not seem to match reality. How could things be so? After some frustrating debugging our valiant hero thought to himself "Hmm, I wonder if this is simply a Nix idiosyncrasy", and it turns out indeed it was. Things that were working in the blessed way of the land of Python (inside a `virtualenv`) were not working the way they were from his home town of Nix (`nix-shell` + `python.withPackages`). After even more frustrating attempts at reading the mypy documentation and trying to understand how things were supposed to work our hero started questioning his sanity. This is where things started to get truly interesting. Our hero started to use a number of powerful weapons, both forged in the land of Python (pdb) & by the mages of UNIX (printf-style-debugging & strace). After first trying to slay the dragon simply by `strace` and a keen eye our hero did not spot any weak points. Time to break out a more powerful sword (`pdb`) which also did not divulge any secrets about what was wrong. Our hero went back to the `strace` output and after a fair bit of thought and analysis a pattern started to emerge. Mypy was looking in the wrong place (i.e. not in in the environment created by `python.withPackages` but in the interpreter store path) and our princess was in another castle! Our hero went to the pub full of old grumpy men giving out the inner workings of the open source universe (Github) and acquired a copy of Mypy. He littered the code with print statements & break points. After a fierce battle full of blood, sweat & tears he ended up in https://github.com/python/mypy/blob/20f7f2dd71c21bde4d3d99f9ab69bf6670c7fa03/mypy/sitepkgs.py and realised that everything came down to the Python `site` module and more specifically https://docs.python.org/3.7/library/site.html#site.getsitepackages which in turn relies on https://docs.python.org/3.7/library/site.html#site.PREFIXES . Our hero created a copy of the environment created by `python.withPackages` and manually modified it to confirm his findings, and it turned out it was indeed the case. Our hero had damaged the dragon and it was time for a celebration. He went out and acquired some mead which he ingested while he typed up his story and waited for the dragon to finally die (the commit caused a mass-rebuild, I had to wait for my repro). In the end all was good in [NixOps-AWS](https://github.com/nixos/nixops-aws)-town and type checks could run. (PR for that incoming tomorrow). --- pkgs/development/interpreters/python/sitecustomize.py | 4 ++++ pkgs/development/interpreters/python/wrapper.nix | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/pkgs/development/interpreters/python/sitecustomize.py b/pkgs/development/interpreters/python/sitecustomize.py index e03b244dbc0..e59d6070cb6 100644 --- a/pkgs/development/interpreters/python/sitecustomize.py +++ b/pkgs/development/interpreters/python/sitecustomize.py @@ -21,6 +21,10 @@ paths = os.environ.pop('NIX_PYTHONPATH', None) if paths: functools.reduce(lambda k, p: site.addsitedir(p, k), paths.split(':'), site._init_pathinfo()) +prefixes = os.environ.pop('NIX_PYTHONPREFIX', None) +if prefixes: + site.PREFIXES.extend(prefixes.split(':')) + executable = os.environ.pop('NIX_PYTHONEXECUTABLE', None) if 'PYTHONEXECUTABLE' not in os.environ and executable: sys.executable = executable diff --git a/pkgs/development/interpreters/python/wrapper.nix b/pkgs/development/interpreters/python/wrapper.nix index b437584024f..dffad6b98f5 100644 --- a/pkgs/development/interpreters/python/wrapper.nix +++ b/pkgs/development/interpreters/python/wrapper.nix @@ -37,7 +37,7 @@ let if [ -f "$prg" ]; then rm -f "$out/bin/$prg" if [ -x "$prg" ]; then - makeWrapper "$path/bin/$prg" "$out/bin/$prg" --set NIX_PYTHONEXECUTABLE ${pythonExecutable} --set NIX_PYTHONPATH ${pythonPath} ${if permitUserSite then "" else ''--set PYTHONNOUSERSITE "true"''} ${stdenv.lib.concatStringsSep " " makeWrapperArgs} + makeWrapper "$path/bin/$prg" "$out/bin/$prg" --set NIX_PYTHONPREFIX "$out" --set NIX_PYTHONEXECUTABLE ${pythonExecutable} --set NIX_PYTHONPATH ${pythonPath} ${if permitUserSite then "" else ''--set PYTHONNOUSERSITE "true"''} ${stdenv.lib.concatStringsSep " " makeWrapperArgs} fi fi done From 05571d3059deeecf5a10d2b85ec6a8f929d3e413 Mon Sep 17 00:00:00 2001 From: adisbladis Date: Sat, 14 Mar 2020 15:59:42 +0000 Subject: [PATCH 2/4] Python Add test for NIX_PYTHONPREFIX --- pkgs/development/interpreters/python/tests.nix | 1 + pkgs/development/interpreters/python/tests/test_python.py | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/pkgs/development/interpreters/python/tests.nix b/pkgs/development/interpreters/python/tests.nix index 37fbe670114..a0f41610723 100644 --- a/pkgs/development/interpreters/python/tests.nix +++ b/pkgs/development/interpreters/python/tests.nix @@ -36,6 +36,7 @@ let is_venv = "True"; is_nixenv = "False"; }; + # Venv built using Python Nix environment (python.buildEnv) # TODO: Cannot create venv from a nix env # Error: Command '['/nix/store/ddc8nqx73pda86ibvhzdmvdsqmwnbjf7-python3-3.7.6-venv/bin/python3.7', '-Im', 'ensurepip', '--upgrade', '--default-pip']' returned non-zero exit status 1. diff --git a/pkgs/development/interpreters/python/tests/test_python.py b/pkgs/development/interpreters/python/tests/test_python.py index f631a172ccc..93aa2bb2b9e 100644 --- a/pkgs/development/interpreters/python/tests/test_python.py +++ b/pkgs/development/interpreters/python/tests/test_python.py @@ -35,7 +35,7 @@ class TestCasePython(unittest.TestCase): def test_site_prefix(self): self.assertTrue(sys.prefix in site.PREFIXES) - @unittest.skipIf(sys.version_info.major==2, "Python 2 does not have base_prefix") + @unittest.skipIf(IS_PYPY or sys.version_info.major==2, "Python 2 does not have base_prefix") def test_base_prefix(self): if IS_VENV: self.assertNotEqual(sys.prefix, sys.base_prefix) From 753122388d0080c8566872cb977f5841b22f1b4e Mon Sep 17 00:00:00 2001 From: adisbladis Date: Sat, 14 Mar 2020 16:16:20 +0000 Subject: [PATCH 3/4] Python: Add integration test verifying NIX_PYTHONPATH with Mypy --- .../development/interpreters/python/tests.nix | 11 +++++++- .../tests/test_nix_pythonprefix/default.nix | 25 +++++++++++++++++++ .../typeddep/default.nix | 11 ++++++++ .../test_nix_pythonprefix/typeddep/setup.py | 18 +++++++++++++ .../typeddep/typeddep/__init__.py | 0 .../typeddep/typeddep/py.typed | 0 .../typeddep/typeddep/util.py | 2 ++ 7 files changed, 66 insertions(+), 1 deletion(-) create mode 100644 pkgs/development/interpreters/python/tests/test_nix_pythonprefix/default.nix create mode 100644 pkgs/development/interpreters/python/tests/test_nix_pythonprefix/typeddep/default.nix create mode 100644 pkgs/development/interpreters/python/tests/test_nix_pythonprefix/typeddep/setup.py create mode 100644 pkgs/development/interpreters/python/tests/test_nix_pythonprefix/typeddep/typeddep/__init__.py create mode 100644 pkgs/development/interpreters/python/tests/test_nix_pythonprefix/typeddep/typeddep/py.typed create mode 100644 pkgs/development/interpreters/python/tests/test_nix_pythonprefix/typeddep/typeddep/util.py diff --git a/pkgs/development/interpreters/python/tests.nix b/pkgs/development/interpreters/python/tests.nix index a0f41610723..55065c45d57 100644 --- a/pkgs/development/interpreters/python/tests.nix +++ b/pkgs/development/interpreters/python/tests.nix @@ -2,6 +2,7 @@ , runCommand , substituteAll , lib +, callPackage }: let @@ -50,6 +51,14 @@ let # }; }; + # All PyPy package builds are broken at the moment + integrationTests = lib.optionalAttrs (python.isPy3k && (!python.isPyPy)) rec { + # Before the addition of NIX_PYTHONPREFIX mypy was broken with typed packages + nix-pythonprefix-mypy = callPackage ./tests/test_nix_pythonprefix { + interpreter = python; + }; + }; + testfun = name: attrs: runCommand "${python.name}-tests-${name}" ({ inherit (python) pythonVersion; } // attrs) '' @@ -61,4 +70,4 @@ let touch $out/success ''; -in lib.mapAttrs testfun envs \ No newline at end of file +in lib.mapAttrs testfun envs // integrationTests diff --git a/pkgs/development/interpreters/python/tests/test_nix_pythonprefix/default.nix b/pkgs/development/interpreters/python/tests/test_nix_pythonprefix/default.nix new file mode 100644 index 00000000000..05798cbaf1b --- /dev/null +++ b/pkgs/development/interpreters/python/tests/test_nix_pythonprefix/default.nix @@ -0,0 +1,25 @@ +{ interpreter, writeText, runCommandNoCC }: + +let + + python = let + packageOverrides = self: super: { + typeddep = super.callPackage ./typeddep {}; + }; + in interpreter.override {inherit packageOverrides; self = python;}; + + pythonEnv = python.withPackages(ps: [ + ps.typeddep + ps.mypy + ]); + + pythonScript = writeText "myscript.py" '' + from typeddep import util + s: str = util.echo("hello") + print(s) + ''; + +in runCommandNoCC "${interpreter.name}-site-prefix-mypy-test" {} '' + ${pythonEnv}/bin/mypy ${pythonScript} + touch $out +'' diff --git a/pkgs/development/interpreters/python/tests/test_nix_pythonprefix/typeddep/default.nix b/pkgs/development/interpreters/python/tests/test_nix_pythonprefix/typeddep/default.nix new file mode 100644 index 00000000000..06219a69fca --- /dev/null +++ b/pkgs/development/interpreters/python/tests/test_nix_pythonprefix/typeddep/default.nix @@ -0,0 +1,11 @@ +{ buildPythonPackage }: + + +buildPythonPackage { + + pname = "typeddep"; + version = "1.3.3.7"; + + src = ./.; + +} diff --git a/pkgs/development/interpreters/python/tests/test_nix_pythonprefix/typeddep/setup.py b/pkgs/development/interpreters/python/tests/test_nix_pythonprefix/typeddep/setup.py new file mode 100644 index 00000000000..25bac69ea09 --- /dev/null +++ b/pkgs/development/interpreters/python/tests/test_nix_pythonprefix/typeddep/setup.py @@ -0,0 +1,18 @@ +from setuptools import setup + +setup(**{ + 'name': 'typeddep', + 'version': '1.3.3.7', + 'description': 'Minimal repro to test mypy and site prefixes with Nix', + 'long_description': None, + 'author': 'adisbladis', + 'author_email': 'adisbladis@gmail.com', + 'maintainer': None, + 'maintainer_email': None, + 'url': None, + 'packages': ['typeddep'], + 'package_data': {'': ['*']}, + 'install_requires': [], + 'entry_points': {}, + 'python_requires': '>=3.7,<4.0', +}) diff --git a/pkgs/development/interpreters/python/tests/test_nix_pythonprefix/typeddep/typeddep/__init__.py b/pkgs/development/interpreters/python/tests/test_nix_pythonprefix/typeddep/typeddep/__init__.py new file mode 100644 index 00000000000..e69de29bb2d diff --git a/pkgs/development/interpreters/python/tests/test_nix_pythonprefix/typeddep/typeddep/py.typed b/pkgs/development/interpreters/python/tests/test_nix_pythonprefix/typeddep/typeddep/py.typed new file mode 100644 index 00000000000..e69de29bb2d diff --git a/pkgs/development/interpreters/python/tests/test_nix_pythonprefix/typeddep/typeddep/util.py b/pkgs/development/interpreters/python/tests/test_nix_pythonprefix/typeddep/typeddep/util.py new file mode 100644 index 00000000000..c1c3ffe7477 --- /dev/null +++ b/pkgs/development/interpreters/python/tests/test_nix_pythonprefix/typeddep/typeddep/util.py @@ -0,0 +1,2 @@ +def echo(s: str) -> str: + return s From 7447fff95a4e091dc9ebe961d46cb4fbc32e69d1 Mon Sep 17 00:00:00 2001 From: Frederik Rietdijk Date: Sat, 14 Mar 2020 16:22:36 +0100 Subject: [PATCH 4/4] Fix sys.prefix in case of a Nix env The prefix will now be correct in case of Nix env. Note, however, that creating a venv from a Nix env still does not function. This does not seem to be possible with the current approach either, because venv will copy or symlink our Python wrapper. In case it symlinks (the default) it won't see a pyvenv.cfg. If it is copied I think it should function but it does not... --- .../interpreters/python/sitecustomize.py | 21 +++++++++++++------ .../interpreters/python/tests/test_python.py | 4 ++-- 2 files changed, 17 insertions(+), 8 deletions(-) diff --git a/pkgs/development/interpreters/python/sitecustomize.py b/pkgs/development/interpreters/python/sitecustomize.py index e59d6070cb6..72ce951328f 100644 --- a/pkgs/development/interpreters/python/sitecustomize.py +++ b/pkgs/development/interpreters/python/sitecustomize.py @@ -21,10 +21,19 @@ paths = os.environ.pop('NIX_PYTHONPATH', None) if paths: functools.reduce(lambda k, p: site.addsitedir(p, k), paths.split(':'), site._init_pathinfo()) -prefixes = os.environ.pop('NIX_PYTHONPREFIX', None) -if prefixes: - site.PREFIXES.extend(prefixes.split(':')) +# Check whether we are in a venv. +# Note Python 2 does not support base_prefix so we assume we are not in a venv. +in_venv = sys.version_info.major == 3 and sys.prefix != sys.base_prefix -executable = os.environ.pop('NIX_PYTHONEXECUTABLE', None) -if 'PYTHONEXECUTABLE' not in os.environ and executable: - sys.executable = executable +if not in_venv: + executable = os.environ.pop('NIX_PYTHONEXECUTABLE', None) + prefix = os.environ.pop('NIX_PYTHONPREFIX', None) + + if 'PYTHONEXECUTABLE' not in os.environ and executable is not None: + sys.executable = executable + if prefix is not None: + # Because we cannot check with Python 2 whether we are in a venv, + # creating a venv from a Nix env won't work as well with Python 2. + # Also, note that sysconfig does not like it when sys.prefix is set to None + sys.prefix = sys.exec_prefix = prefix + site.PREFIXES.insert(0, prefix) diff --git a/pkgs/development/interpreters/python/tests/test_python.py b/pkgs/development/interpreters/python/tests/test_python.py index 93aa2bb2b9e..011978c6254 100644 --- a/pkgs/development/interpreters/python/tests/test_python.py +++ b/pkgs/development/interpreters/python/tests/test_python.py @@ -27,7 +27,7 @@ class TestCasePython(unittest.TestCase): def test_interpreter(self): self.assertEqual(sys.executable, INTERPRETER) - @unittest.skipIf(IS_NIXENV or IS_PYPY, "Prefix is incorrect and needs to be fixed.") + @unittest.skipIf(IS_PYPY, "Prefix is incorrect and needs to be fixed.") def test_prefix(self): self.assertEqual(sys.prefix, ENV) self.assertEqual(sys.prefix, sys.exec_prefix) @@ -37,7 +37,7 @@ class TestCasePython(unittest.TestCase): @unittest.skipIf(IS_PYPY or sys.version_info.major==2, "Python 2 does not have base_prefix") def test_base_prefix(self): - if IS_VENV: + if IS_VENV or IS_NIXENV: self.assertNotEqual(sys.prefix, sys.base_prefix) else: self.assertEqual(sys.prefix, sys.base_prefix)