Merge pull request #287957 from DavHau/python

pythonCatchConflictsHook: scan $out, not sys.path (2)

doc/languages-frameworks/python.section.md

@@ -469,7 +469,7 @@ are used in [`buildPythonPackage`](#buildpythonpackage-function).
   be added as `nativeBuildInput`.
 - `pipInstallHook` to install wheels.
 - `pytestCheckHook` to run tests with `pytest`. See [example usage](#using-pytestcheckhook).
-- `pythonCatchConflictsHook` to check whether a Python package is not already existing.
+- `pythonCatchConflictsHook` to fail if the package depends on two different versions of the same dependency.
 - `pythonImportsCheckHook` to check whether importing the listed modules works.
 - `pythonRelaxDepsHook` will relax Python dependencies restrictions for the package.
   See [example usage](#using-pythonrelaxdepshook).

pkgs/development/interpreters/python/catch_conflicts/catch_conflicts.py

@@ -2,28 +2,77 @@ from importlib.metadata import PathDistribution
 from pathlib import Path
 import collections
 import sys
+import os
+from typing import Dict, List, Tuple
+do_abort: bool = False
+packages: Dict[str, Dict[str, List[Dict[str, List[str]]]]] = collections.defaultdict(list)
+out_path: Path = Path(os.getenv("out"))
+version: Tuple[int, int] = sys.version_info
+site_packages_path: str = f'lib/python{version[0]}.{version[1]}/site-packages'
 
 
-do_abort = False
-packages = collections.defaultdict(list)
+def get_name(dist: PathDistribution) -> str:
+    return dist.metadata['name'].lower().replace('-', '_')
 
 
-for path in sys.path:
-    for dist_info in Path(path).glob("*.dist-info"):
-        dist = PathDistribution(dist_info)
-
-        packages[dist._normalized_name].append(
-            f"{dist._normalized_name} {dist.version} ({dist._path})"
-        )
+# pretty print a package
+def describe_package(dist: PathDistribution) -> str:
+    return f"{get_name(dist)} {dist.version} ({dist._path})"
 
 
-for name, duplicates in packages.items():
-    if len(duplicates) > 1:
+# pretty print a list of parents (dependency chain)
+def describe_parents(parents: List[str]) -> str:
+    if not parents:
+        return ""
+    return \
+        f"    dependency chain:\n      " \
+        + str(f"\n      ...depending on: ".join(parents))
+
+
+# inserts an entry into 'packages'
+def add_entry(name: str, version: str, store_path: str, parents: List[str]) -> None:
+    if name not in packages:
+        packages[name] = {}
+    if store_path not in packages[name]:
+        packages[name][store_path] = []
+    packages[name][store_path].append(dict(
+        version=version,
+        parents=parents,
+    ))
+
+
+# transitively discover python dependencies and store them in 'packages'
+def find_packages(store_path: Path, site_packages_path: str, parents: List[str]) -> None:
+    site_packages: Path = (store_path / site_packages_path)
+    propagated_build_inputs: Path = (store_path / "nix-support/propagated-build-inputs")
+
+    # add the current package to the list
+    if site_packages.exists():
+        for dist_info in site_packages.glob("*.dist-info"):
+            dist: PathDistribution = PathDistribution(dist_info)
+            add_entry(get_name(dist), dist.version, store_path, parents)
+
+    # recursively add dependencies
+    if propagated_build_inputs.exists():
+        with open(propagated_build_inputs, "r") as f:
+            build_inputs: List[str] = f.read().strip().split(" ")
+            for build_input in build_inputs:
+                find_packages(Path(build_input), site_packages_path, parents + [build_input])
+
+
+find_packages(out_path, site_packages_path, [f"this derivation: {out_path}"])
+
+# print all duplicates
+for name, store_paths in packages.items():
+    if len(store_paths) > 1:
         do_abort = True
         print("Found duplicated packages in closure for dependency '{}': ".format(name))
-        for duplicate in duplicates:
-            print(f"\t{duplicate}")
+        for store_path, candidates in store_paths.items():
+            for candidate in candidates:
+                print(f"  {name} {candidate['version']} ({store_path})")
+                print(describe_parents(candidate['parents']))
 
+# fail if duplicates were found
 if do_abort:
     print("")
     print(

pkgs/development/interpreters/python/hooks/default.nix

@@ -108,7 +108,7 @@ in {
     makePythonHook {
       name = "python-catch-conflicts-hook";
       substitutions = let
-        useLegacyHook = lib.versionOlder python.pythonVersion "3.10";
+        useLegacyHook = lib.versionOlder python.pythonVersion "3";
       in {
         inherit pythonInterpreter pythonSitePackages;
         catchConflicts = if useLegacyHook then
@@ -118,6 +118,10 @@ in {
       } // lib.optionalAttrs useLegacyHook {
         inherit setuptools;
       };
+      passthru.tests = import ./python-catch-conflicts-hook-tests.nix {
+        inherit pythonOnBuildForHost runCommand;
+        inherit (pkgs) coreutils gnugrep writeShellScript;
+      };
     } ./python-catch-conflicts-hook.sh) {};
 
   pythonImportsCheckHook = callPackage ({ makePythonHook }:

pkgs/development/interpreters/python/hooks/python-catch-conflicts-hook-tests.nix

@@ -0,0 +1,137 @@
+{ pythonOnBuildForHost, runCommand, writeShellScript, coreutils, gnugrep }: let
+
+  pythonPkgs = pythonOnBuildForHost.pkgs;
+
+  ### UTILITIES
+
+  # customize a package so that its store paths differs
+  customize = pkg: pkg.overrideAttrs { some_modification = true; };
+
+  # generates minimal pyproject.toml
+  pyprojectToml = pname: builtins.toFile "pyproject.toml" ''
+    [project]
+    name = "${pname}"
+    version = "1.0.0"
+  '';
+
+  # generates source for a python project
+  projectSource = pname: runCommand "my-project-source" {} ''
+    mkdir -p $out/src
+    cp ${pyprojectToml pname} $out/pyproject.toml
+    touch $out/src/__init__.py
+  '';
+
+  # helper to reduce boilerplate
+  generatePythonPackage = args: pythonPkgs.buildPythonPackage (
+    {
+      version = "1.0.0";
+      src = runCommand "my-project-source" {} ''
+        mkdir -p $out/src
+        cp ${pyprojectToml args.pname} $out/pyproject.toml
+        touch $out/src/__init__.py
+      '';
+      pyproject = true;
+      catchConflicts = true;
+      buildInputs = [ pythonPkgs.setuptools ];
+    }
+    // args
+  );
+
+  # in order to test for a failing build, wrap it in a shell script
+  expectFailure = build: errorMsg: build.overrideDerivation (old: {
+    builder = writeShellScript "test-for-failure" ''
+      export PATH=${coreutils}/bin:${gnugrep}/bin:$PATH
+      ${old.builder} "$@" > ./log 2>&1
+      status=$?
+      cat ./log
+      if [ $status -eq 0 ] || ! grep -q "${errorMsg}" ./log; then
+        echo "The build should have failed with '${errorMsg}', but it didn't"
+        exit 1
+      else
+        echo "The build failed as expected with: ${errorMsg}"
+        mkdir -p $out
+      fi
+    '';
+  });
+in {
+
+  ### TEST CASES
+
+  # Test case which must not trigger any conflicts.
+  # This derivation has runtime dependencies on custom versions of multiple build tools.
+  # This scenario is relevant for lang2nix tools which do not override the nixpkgs fix-point.
+  # see https://github.com/NixOS/nixpkgs/issues/283695
+  ignores-build-time-deps =
+    generatePythonPackage {
+      pname = "ignores-build-time-deps";
+      buildInputs = [
+        pythonPkgs.build
+        pythonPkgs.packaging
+        pythonPkgs.setuptools
+        pythonPkgs.wheel
+      ];
+      propagatedBuildInputs = [
+        # Add customized versions of build tools as runtime deps
+        (customize pythonPkgs.packaging)
+        (customize pythonPkgs.setuptools)
+        (customize pythonPkgs.wheel)
+      ];
+    };
+
+  # Simplest test case that should trigger a conflict
+  catches-simple-conflict = let
+    # this build must fail due to conflicts
+    package = pythonPkgs.buildPythonPackage rec {
+      pname = "catches-simple-conflict";
+      version = "0.0.0";
+      src = projectSource pname;
+      pyproject = true;
+      catchConflicts = true;
+      buildInputs = [
+        pythonPkgs.setuptools
+      ];
+      # depend on two different versions of packaging
+      # (an actual runtime dependency conflict)
+      propagatedBuildInputs = [
+        pythonPkgs.packaging
+        (customize pythonPkgs.packaging)
+      ];
+    };
+  in
+    expectFailure package "Found duplicated packages in closure for dependency 'packaging'";
+
+
+  /*
+    More complex test case with a transitive conflict
+
+    Test sets up this dependency tree:
+
+      toplevel
+      ├── dep1
+      │   └── leaf
+      └── dep2
+          └── leaf (customized version -> conflicting)
+  */
+  catches-transitive-conflict = let
+    # package depending on both dependency1 and dependency2
+    toplevel = generatePythonPackage {
+      pname = "catches-transitive-conflict";
+      propagatedBuildInputs = [ dep1 dep2 ];
+    };
+    # dep1 package depending on leaf
+    dep1 = generatePythonPackage {
+      pname = "dependency1";
+      propagatedBuildInputs = [ leaf ];
+    };
+    # dep2 package depending on conflicting version of leaf
+    dep2 = generatePythonPackage {
+      pname = "dependency2";
+      propagatedBuildInputs = [ (customize leaf) ];
+    };
+    # some leaf package
+    leaf = generatePythonPackage {
+      pname = "leaf";
+    };
+  in
+    expectFailure toplevel "Found duplicated packages in closure for dependency 'leaf'";
+}