cpython: refactor & clean up

- group dependencies logically
- remove `? null` fallback value
- remove redundant isPy3k conditions
- remove overly broad `with lib` import
- more assertion messages
- move python310 out of sources attrset

2 files changed, 137 insertions, 83 deletions

diff --git a/pkgs/development/interpreters/python/cpython/default.nix b/pkgs/development/interpreters/python/cpython/default.nix
index ec5d21416dabf..ed507e3baf9c2 100644
--- a/pkgs/development/interpreters/python/cpython/default.nix
+++ b/pkgs/development/interpreters/python/cpython/default.nix
@@ -1,39 +1,54 @@
-{ lib, stdenv, fetchurl, fetchpatch, fetchgit
+{ lib
+, stdenv
+, fetchurl
+, fetchpatch
+, fetchgit
+
+# build dependencies
+, autoconf-archive
+, autoreconfHook
+, nukeReferences
+, pkg-config
+, python-setup-hook
+
+# runtime dependencies
 , bzip2
 , expat
 , libffi
-, gdbm
-, xz
-, mailcap, mimetypesSupport ? true
+, libxcrypt
 , ncurses
 , openssl
-, readline
 , sqlite
-, tcl ? null, tk ? null, tix ? null, libX11 ? null, xorgproto ? null, x11Support ? false
-, bluez ? null, bluezSupport ? false
+, xz
 , zlib
-, tzdata ? null
-, libxcrypt
-, self
+
+# platform-specific dependencies
+, bash
 , configd
 , darwin
 , windows
-, autoreconfHook
-, autoconf-archive
-, pkg-config
-, python-setup-hook
-, nukeReferences
-# For the Python package set
-, packageOverrides ? (self: super: {})
+
+# optional dependencies
+, bluezSupport ? false, bluez
+, mimetypesSupport ? true, mailcap
+, tzdata
+, withGdbm ? !stdenv.hostPlatform.isWindows, gdbm
+, withReadline ? !stdenv.hostPlatform.isWindows, readline
+, x11Support ? false, tcl, tk, tix, libX11, xorgproto
+
+# splicing/cross
+, pythonAttr ? "python${sourceVersion.major}${sourceVersion.minor}"
+, self
 , pkgsBuildBuild
 , pkgsBuildHost
 , pkgsBuildTarget
 , pkgsHostHost
 , pkgsTargetTarget
+
+# build customization
 , sourceVersion
 , hash
 , passthruFun
-, bash
 , stripConfig ? false
 , stripIdlelib ? false
 , stripTests ? false
@@ -43,21 +58,28 @@
 , includeSiteCustomize ? true
 , static ? stdenv.hostPlatform.isStatic
 , enableFramework ? false
+, noldconfigPatch ? ./. + "/${sourceVersion.major}.${sourceVersion.minor}/no-ldconfig.patch"
+
+# pgo (not reproducible) + -fno-semantic-interposition
+# https://docs.python.org/3/using/configure.html#cmdoption-enable-optimizations
 , enableOptimizations ? false
-# these dont build for windows
-, withGdbm ? !stdenv.hostPlatform.isWindows
-, withReadline ? !stdenv.hostPlatform.isWindows
-# enableNoSemanticInterposition is a subset of the enableOptimizations flag that doesn't harm reproducibility.
-# clang starts supporting `-fno-sematic-interposition` with version 10
-, enableNoSemanticInterposition ? (!stdenv.cc.isClang || (stdenv.cc.isClang && lib.versionAtLeast stdenv.cc.version "10"))
-# enableLTO is a subset of the enableOptimizations flag that doesn't harm reproducibility.
+
+# improves performance, but remains reproducible
+, enableNoSemanticInterposition ? true
+
 # enabling LTO on 32bit arch causes downstream packages to fail when linking
 # enabling LTO on *-darwin causes python3 to fail when linking.
 , enableLTO ? stdenv.is64bit && stdenv.isLinux
+
+# enable asserts to ensure the build remains reproducible
 , reproducibleBuild ? false
-, pythonAttr ? "python${sourceVersion.major}${sourceVersion.minor}"
-, noldconfigPatch ? ./. + "/${sourceVersion.major}.${sourceVersion.minor}/no-ldconfig.patch"
+
+# for the Python package set
+, packageOverrides ? (self: super: {})
+
+# tests
 , testers
+
 } @ inputs:
 
 # Note: this package is used for bootstrapping fetchurl, and thus
@@ -72,7 +94,11 @@ assert x11Support -> tcl != null
 
 assert bluezSupport -> bluez != null;
 
-assert enableFramework -> stdenv.isDarwin;
+assert lib.assertMsg (bluezSupport -> stdenv.isLinux)
+  "Bluez support is only available on Linux.";
+
+assert lib.assertMsg (enableFramework -> stdenv.isDarwin)
+  "Framework builds are only supported on Darwin.";
 
 assert lib.assertMsg (reproducibleBuild -> stripBytecode)
   "Deterministic builds require stripping bytecode.";
@@ -83,14 +109,21 @@ assert lib.assertMsg (reproducibleBuild -> (!enableOptimizations))
 assert lib.assertMsg (reproducibleBuild -> (!rebuildBytecode))
   "Deterministic builds are not achieved when (default unoptimized) bytecode is created.";
 
-with lib;
-
 let
+  inherit (lib)
+    concatMapStringsSep
+    concatStringsSep
+    getDev
+    getLib
+    optionals
+    optionalString
+    replaceStrings
+    versionOlder
+  ;
+
   buildPackages = pkgsBuildHost;
   inherit (passthru) pythonOnBuildForHost;
 
-  inherit (darwin.apple_sdk.frameworks) Cocoa;
-
   tzdataSupport = tzdata != null && passthru.pythonAtLeast "3.9";
 
   passthru = let
@@ -114,12 +147,12 @@ let
 
   version = with sourceVersion; "${major}.${minor}.${patch}${suffix}";
 
-  nativeBuildInputs = optionals (!stdenv.isDarwin) [
+  nativeBuildInputs = [
+    nukeReferences
+  ] ++ optionals (!stdenv.isDarwin) [
+    autoconf-archive # needed for AX_CHECK_COMPILE_FLAG
     autoreconfHook
     pkg-config
-    autoconf-archive # needed for AX_CHECK_COMPILE_FLAG
-  ] ++ [
-    nukeReferences
   ] ++ optionals (stdenv.hostPlatform != stdenv.buildPlatform) [
     buildPackages.stdenv.cc
     pythonOnBuildForHost
@@ -127,19 +160,37 @@ let
     stdenv.cc.cc.libllvm.out
   ];
 
-  buildInputs = filter (p: p != null) ([
-    zlib bzip2 expat xz libffi libxcrypt ]
-    ++ optional withGdbm gdbm
-    ++ [ sqlite ]
-    ++ optional withReadline readline
-    ++ [ ncurses openssl ]
-    ++ optionals x11Support [ tcl tk libX11 xorgproto ]
-    ++ optionals (bluezSupport && stdenv.isLinux) [ bluez ]
-    ++ optionals stdenv.isDarwin [ configd ])
-
-    ++ optionals enableFramework [ Cocoa ]
-    ++ optionals stdenv.hostPlatform.isMinGW [ windows.mingw_w64_pthreads windows.dlfcn ]
-    ++ optionals tzdataSupport [ tzdata ];  # `zoneinfo` module
+  buildInputs = lib.filter (p: p != null) ([
+    bzip2
+    expat
+    libffi
+    libxcrypt
+    ncurses
+    openssl
+    sqlite
+    xz
+    zlib
+  ] ++ optionals bluezSupport [
+    bluez
+  ] ++ optionals enableFramework [
+    darwin.apple_sdk.frameworks.Cocoa
+  ] ++ optionals stdenv.hostPlatform.isMinGW [
+    windows.dlfcn
+    windows.mingw_w64_pthreads
+  ] ++ optionals stdenv.isDarwin [
+    configd
+  ] ++ optionals tzdataSupport [
+    tzdata
+  ] ++ optionals withGdbm [
+    gdbm
+  ] ++ optionals withReadline [
+    readline
+  ] ++ optionals x11Support [
+    libX11
+    tcl
+    tk
+    xorgproto
+  ]);
 
   hasDistutilsCxxPatch = !(stdenv.cc.isGNU or false);
 
@@ -202,7 +253,7 @@ let
       pythonAbi = nixpkgsPythonAbiMappings.${parsed.abi.name} or parsed.abi.name;
     in
       # Python <3.11 doesn't distinguish musl and glibc and always prefixes with "gnu"
-      if lib.versionOlder version "3.11" then
+      if versionOlder version "3.11" then
         replaceStrings [ "musl" ] [ "gnu" ] pythonAbi
       else
         pythonAbi;
@@ -233,8 +284,9 @@ in with passthru; stdenv.mkDerivation (finalAttrs: {
   inherit src version;
 
   inherit nativeBuildInputs;
-  buildInputs = lib.optionals (!stdenv.hostPlatform.isWindows) [ bash ] ++ buildInputs; # bash is only used for patchShebangs
-
+  buildInputs = lib.optionals (!stdenv.hostPlatform.isWindows) [
+    bash # only required for patchShebangs
+  ] ++ buildInputs;
 
   prePatch = optionalString stdenv.isDarwin ''
     substituteInPlace configure --replace-fail '`/usr/bin/arch`' '"i386"'
@@ -265,7 +317,7 @@ in with passthru; stdenv.mkDerivation (finalAttrs: {
   ] ++ optionals (pythonAtLeast "3.9" && pythonOlder "3.11" && stdenv.isDarwin) [
     # Stop checking for TCL/TK in global macOS locations
     ./3.9/darwin-tcl-tk.patch
-  ] ++ optionals (isPy3k && hasDistutilsCxxPatch && pythonOlder "3.12") [
+  ] ++ optionals (hasDistutilsCxxPatch && pythonOlder "3.12") [
     # Fix for http://bugs.python.org/issue1222585
     # Upstream distutils is calling C compiler to compile C++ code, which
     # only works for GCC and Apple Clang. This makes distutils to call C++
@@ -311,7 +363,9 @@ in with passthru; stdenv.mkDerivation (finalAttrs: {
     substituteInPlace Lib/mimetypes.py \
       --replace-fail "@mime-types@" "${mailcap}"
   '' + optionalString (pythonOlder "3.13" && x11Support && (tix != null)) ''
-    substituteInPlace "Lib/tkinter/tix.py" --replace-fail "os.environ.get('TIX_LIBRARY')" "os.environ.get('TIX_LIBRARY') or '${tix}/lib'"
+    substituteInPlace "Lib/tkinter/tix.py" --replace-fail \
+      "os.environ.get('TIX_LIBRARY')" \
+      "os.environ.get('TIX_LIBRARY') or '${tix}/lib'"
   '';
 
   env = {
@@ -326,21 +380,26 @@ in with passthru; stdenv.mkDerivation (finalAttrs: {
     PYTHONHASHSEED=0;
   };
 
+  # https://docs.python.org/3/using/configure.html
   configureFlags = [
     "--without-ensurepip"
     "--with-system-expat"
+  ] ++ optionals (openssl != null) [
+    "--with-openssl=${openssl.dev}"
+  ] ++ optionals tzdataSupport [
+    "--with-tzpath=${tzdata}/share/zoneinfo"
+  ] ++ optionals (execSuffix != "") [
+    "--with-suffix=${execSuffix}"
+  ] ++ optionals enableLTO [
+    "--with-lto"
   ] ++ optionals (!static && !enableFramework) [
     "--enable-shared"
   ] ++ optionals enableFramework [
     "--enable-framework=${placeholder "out"}/Library/Frameworks"
   ] ++ optionals enableOptimizations [
     "--enable-optimizations"
-  ] ++ optionals enableLTO [
-    "--with-lto"
-  ] ++ optionals (sqlite != null && isPy3k) [
+  ] ++ optionals (sqlite != null) [
     "--enable-loadable-sqlite-extensions"
-  ] ++ optionals (openssl != null) [
-    "--with-openssl=${openssl.dev}"
   ] ++ optionals (libxcrypt != null) [
     "CFLAGS=-I${libxcrypt}/include"
     "LIBS=-L${libxcrypt}/lib"
@@ -372,10 +431,9 @@ in with passthru; stdenv.mkDerivation (finalAttrs: {
     # Never even try to use lchmod on linux,
     # don't rely on detecting glibc-isms.
     "ac_cv_func_lchmod=no"
-  ] ++ optionals tzdataSupport [
-    "--with-tzpath=${tzdata}/share/zoneinfo"
-  ] ++ optional static "LDFLAGS=-static"
-  ++ optional (execSuffix != "") "--with-suffix=${execSuffix}";
+  ] ++ optionals static [
+    "LDFLAGS=-static"
+  ];
 
   preConfigure = optionalString (pythonOlder "3.12") ''
     # Improve purity
@@ -471,7 +529,6 @@ in with passthru; stdenv.mkDerivation (finalAttrs: {
     '' + optionalString includeSiteCustomize ''
     # Include a sitecustomize.py file
     cp ${../sitecustomize.py} $out/${sitePackages}/sitecustomize.py
-
     '' + optionalString stripBytecode ''
     # Determinism: deterministic bytecode
     # First we delete all old bytecode.
@@ -529,9 +586,9 @@ in with passthru; stdenv.mkDerivation (finalAttrs: {
 
   # Enforce that we don't have references to the OpenSSL -dev package, which we
   # explicitly specify in our configure flags above.
-  disallowedReferences =
-    lib.optionals (openssl != null && !static && !enableFramework) [ openssl.dev ]
-    ++ lib.optionals (stdenv.hostPlatform != stdenv.buildPlatform) [
+  disallowedReferences = lib.optionals (openssl != null && !static && !enableFramework) [
+    openssl.dev
+  ] ++ lib.optionals (stdenv.hostPlatform != stdenv.buildPlatform) [
     # Ensure we don't have references to build-time packages.
     # These typically end up in shebangs.
     pythonOnBuildForHost buildPackages.bash
@@ -564,11 +621,11 @@ in with passthru; stdenv.mkDerivation (finalAttrs: {
 
   enableParallelBuilding = true;
 
-  meta = {
+  meta = with lib; {
     homepage = "https://www.python.org";
     changelog = let
-      majorMinor = lib.versions.majorMinor version;
-      dashedVersion = lib.replaceStrings [ "." "a" ] [ "-" "-alpha-" ] version;
+      majorMinor = versions.majorMinor version;
+      dashedVersion = replaceStrings [ "." "a" ] [ "-" "-alpha-" ] version;
     in
       if sourceVersion.suffix == "" then
         "https://docs.python.org/release/${version}/whatsnew/changelog.html"
diff --git a/pkgs/development/interpreters/python/default.nix b/pkgs/development/interpreters/python/default.nix
index 7f60ba036c5ff..1322673c1bc6b 100644
--- a/pkgs/development/interpreters/python/default.nix
+++ b/pkgs/development/interpreters/python/default.nix
@@ -16,16 +16,6 @@
   passthruFun = import ./passthrufun.nix args;
 
   sources = {
-    python310 = {
-      sourceVersion = {
-        major = "3";
-        minor = "10";
-        patch = "13";
-        suffix = "";
-      };
-      hash = "sha256-XIiEhmhkDT4VKzW0U27xwjsspL0slX7x7LsFP1cd0/Y=";
-    };
-
     python311 = {
       sourceVersion = {
         major = "3";
@@ -78,11 +68,18 @@ in {
     inherit passthruFun;
   };
 
-  python310 = callPackage ./cpython ({
+  python310 = callPackage ./cpython {
     self = __splicedPackages.python310;
+    sourceVersion = {
+      major = "3";
+      minor = "10";
+      patch = "13";
+      suffix = "";
+    };
+    hash = "sha256-XIiEhmhkDT4VKzW0U27xwjsspL0slX7x7LsFP1cd0/Y=";
     inherit (darwin) configd;
     inherit passthruFun;
-  } // sources.python310);
+  };
 
   python311 = callPackage ./cpython ({
     self = __splicedPackages.python311;
@@ -125,8 +122,8 @@ in {
     readline = null;
     ncurses = null;
     gdbm = null;
-    sqlite = null;
     configd = null;
+    sqlite = null;
     tzdata = null;
     libffi = libffiBoot; # without test suite
     stripConfig = true;