nix-community / poetry2nix

Convert poetry projects to nix automagically [maintainer=@adisbladis,@cpcloud]
MIT License
775 stars 402 forks source link

Separation of build & runtime dependencies? #1616

Open Ma27 opened 2 months ago

Ma27 commented 2 months ago

Describe the issue

Consider the following scenario:

The problem is IMHO that build-dependencies (a.k.a nativeBuildInputs - such as hatchling & setuptools) are modified by the overlay that defines the runtime-inputs (a.k.a propagatedBuildInputs).

My first question is, is there any particular reason for this? I'm in a rather early stage of checking if poetry2nix is useful for $project, so I'm not sure if I'm missing something here or if this was just sufficient for the use-cases you've had before.

I hacked together a small patch that appears to solve the issue for the reproducer below and the test-setup I've been experimenting with:

diff --git a/default.nix b/default.nix
index bc038b8..15fae5b 100644
--- a/default.nix
+++ b/default.nix
@@ -246,6 +246,10 @@ lib.makeScope pkgs.newScope (self: {
             (
               self: _super:
                 {
+                  # Use buildtools in nativeBuildInputs without modifications from
+                  # poetry.lock.
+                  __unpatched = pkgs.${self.python.pythonAttr}.pkgs;
+
                   mkPoetryDep = self.callPackage ./mk-poetry-dep.nix {
                     inherit lib python poetryLib pep508Env pyVersion;
                     inherit pyproject-nix;
diff --git a/overrides/default.nix b/overrides/default.nix
index c0f6dab..21166db 100644
--- a/overrides/default.nix
+++ b/overrides/default.nix
@@ -11,6 +11,12 @@ let
     }:
     let
       buildSystem =
+        let
+          buildSystemPackage = attr:
+            if attr == "cython"
+              then (self.python.pythonOnBuildForHost or self.python.pythonForBuild).pkgs.cython
+              else self.__unpatched.${attr};
+        in
         if builtins.isAttrs attr then
           let
             fromIsValid =
@@ -24,14 +30,11 @@ let
               else
                 true;
             intendedBuildSystem =
-              if attr.buildSystem == "cython" then
-                (self.python.pythonOnBuildForHost or self.python.pythonForBuild).pkgs.cython
-              else
-                self.${attr.buildSystem};
+              buildSystemPackage attr.buildSystem;
           in
           if fromIsValid && untilIsValid then intendedBuildSystem else null
         else
-          if attr == "cython" then (self.python.pythonOnBuildForHost or self.python.pythonForBuild).pkgs.cython else self.${attr};
+          buildSystemPackage attr;
     in
     if (attr == "flit-core" || attr == "flit" || attr == "hatchling") && !self.isPy3k then drv
     else if drv == null then null

I think it's a little hacky to do this via pkgs.${python.pythonAttr}, but I didn't manage to construct __unpatched with just super because (1) I got an infinite recursion when using all of super and (2) when just copying build-tools into __unpatched, it wasn't sufficient, because the packages take their dependencies (pluggy in this case) from the fixpoint and not from super, so this was essentially a no-op.

Will need to find a few more test-cases for this, but I'm curious what your stance is on the entire topic.

cc @Mic92 @adisbladis

Additional context

Minimal reproducer: https://gist.github.com/Ma27/3ba79121c92458f4dbd1eb58cb967cea Please don't forget to mkdir minimal_reproducer && touch minimal_reproducer/__init__.py, gists don't allow folders apparently :(

Mic92 commented 2 months ago

Sorry. I am not maintaining this package and it feels like @adisbladis has dropped the ball. I think, I will give out the commit bit to someone else, if we don't hear soon back from him.