buildRustPackage: refactor to support finalAttrs pattern

pkgs/build-support/rust/build-rust-package/default.nix

@@ -17,182 +17,205 @@
   windows,
 }:
 
-{
-  name ? "${args.pname}-${args.version}",
-
-  # Name for the vendored dependencies tarball
-  cargoDepsName ? name,
-
-  src ? null,
-  srcs ? null,
-  preUnpack ? null,
-  unpackPhase ? null,
-  postUnpack ? null,
-  cargoPatches ? [ ],
-  patches ? [ ],
-  sourceRoot ? null,
-  logLevel ? "",
-  buildInputs ? [ ],
-  nativeBuildInputs ? [ ],
-  cargoUpdateHook ? "",
-  cargoDepsHook ? "",
-  buildType ? "release",
-  meta ? { },
-  cargoLock ? null,
-  cargoVendorDir ? null,
-  checkType ? buildType,
-  buildNoDefaultFeatures ? false,
-  checkNoDefaultFeatures ? buildNoDefaultFeatures,
-  buildFeatures ? [ ],
-  checkFeatures ? buildFeatures,
-  useNextest ? false,
-  auditable ? !cargo-auditable.meta.broken,
-
-  depsExtraArgs ? { },
-
-  # Toggles whether a custom sysroot is created when the target is a .json file.
-  __internal_dontAddSysroot ? false,
-
-  # Needed to `pushd`/`popd` into a subdir of a tarball if this subdir
-  # contains a Cargo.toml, but isn't part of a workspace (which is e.g. the
-  # case for `rustfmt`/etc from the `rust-sources).
-  # Otherwise, everything from the tarball would've been built/tested.
-  buildAndTestSubdir ? null,
-  ...
-}@args:
-
-assert
-  cargoVendorDir == null && cargoLock == null
-  ->
-    !(args ? cargoSha256 && args.cargoSha256 != null) && !(args ? cargoHash && args.cargoHash != null)
-  -> throw "cargoHash, cargoVendorDir, or cargoLock must be set";
+args:
 
 let
-
-  cargoDeps =
-    if cargoVendorDir != null then
-      null
-    else if cargoLock != null then
-      importCargoLock cargoLock
-    else
-      fetchCargoTarball (
-        {
-          inherit
-            src
-            srcs
-            sourceRoot
-            preUnpack
-            unpackPhase
-            postUnpack
-            cargoUpdateHook
-            ;
-          name = cargoDepsName;
-          patches = cargoPatches;
-        }
-        // lib.optionalAttrs (args ? cargoHash) {
-          hash = args.cargoHash;
-        }
-        // lib.optionalAttrs (args ? cargoSha256) {
-          sha256 = lib.warn "cargoSha256 is deprecated. Please use cargoHash with SRI hash instead" args.cargoSha256;
-        }
-        // depsExtraArgs
-      );
-
   target = stdenv.hostPlatform.rust.rustcTargetSpec;
   targetIsJSON = lib.hasSuffix ".json" target;
-  useSysroot = targetIsJSON && !__internal_dontAddSysroot;
 
-  sysroot = callPackage ./sysroot { } {
-    inherit target;
-    shortTarget = stdenv.hostPlatform.rust.cargoShortTarget;
-    RUSTFLAGS = args.RUSTFLAGS or "";
-    originalCargoToml = src + /Cargo.toml; # profile info is later extracted
-  };
+  takeAttrs = names: lib.filterAttrs (n: _: lib.elem n names);
 
-in
-
-# Tests don't currently work for `no_std`, and all custom sysroots are currently built without `std`.
-# See https://os.phil-opp.com/testing/ for more information.
-assert useSysroot -> !(args.doCheck or true);
-
-stdenv.mkDerivation (
-  (removeAttrs args [
+  # these names are processed manually, so don't directly pass them to mkDerivation
+  namesToRemove = [
     "depsExtraArgs"
     "cargoUpdateHook"
     "cargoLock"
-  ])
-  // lib.optionalAttrs useSysroot {
-    RUSTFLAGS = "--sysroot ${sysroot} " + (args.RUSTFLAGS or "");
-  }
-  // lib.optionalAttrs (stdenv.isDarwin && buildType == "debug") {
-    RUSTFLAGS =
-      "-C split-debuginfo=packed "
-      + lib.optionalString useSysroot "--sysroot ${sysroot} "
-      + (args.RUSTFLAGS or "");
-  }
-  // {
-    inherit buildAndTestSubdir cargoDeps;
-
-    cargoBuildType = buildType;
-
-    cargoCheckType = checkType;
-
-    cargoBuildNoDefaultFeatures = buildNoDefaultFeatures;
-
-    cargoCheckNoDefaultFeatures = checkNoDefaultFeatures;
-
-    cargoBuildFeatures = buildFeatures;
-
-    cargoCheckFeatures = checkFeatures;
+    "patches"
+    "postUnpack"
+    "nativeBuildInputs"
+    "buildInputs"
+    "RUSTFLAGS"
+    "env"
+    "passthru"
+    "meta"
+  ];
+in
 
-    patchRegistryDeps = ./patch-registry-deps;
+stdenv.mkDerivation (
+  finalAttrs:
+
+  let
+    useSysroot = targetIsJSON && !finalAttrs.__internal_dontAddSysroot;
+    sysroot = callPackage ./sysroot { } {
+      inherit target;
+      shortTarget = stdenv.hostPlatform.rust.cargoShortTarget;
+      RUSTFLAGS = args.RUSTFLAGS or "";
+      originalCargoToml = finalAttrs.src + /Cargo.toml; # profile info is later extracted
+    };
 
-    nativeBuildInputs =
-      nativeBuildInputs
-      ++ lib.optionals auditable [
-        (buildPackages.cargo-auditable-cargo-wrapper.override {
-          inherit cargo cargo-auditable;
-        })
+    EXTRA_RUSTFLAGS = toString (
+      lib.optionals (stdenv.hostPlatform.isDarwin && finalAttrs.buildType == "debug") [
+        "-C split-debuginfo=packed"
       ]
-      ++ [
-        cargoBuildHook
-        (if useNextest then cargoNextestHook else cargoCheckHook)
-        cargoInstallHook
-        cargoSetupHook
-        rustc
-      ];
-
-    buildInputs =
-      buildInputs
-      ++ lib.optionals stdenv.hostPlatform.isDarwin [ libiconv ]
-      ++ lib.optionals stdenv.hostPlatform.isMinGW [ windows.pthreads ];
-
-    patches = cargoPatches ++ patches;
+      ++ lib.optionals useSysroot [ "--sysroot ${sysroot}" ]
+    );
+  in
 
-    PKG_CONFIG_ALLOW_CROSS = if stdenv.buildPlatform != stdenv.hostPlatform then 1 else 0;
+  # TODO: this assert leads to infinite recursion, since we need to know the finalAttrs to eval the assertion,
+  #       but the finalAttrs can only be calculated after checking the assertion
 
-    postUnpack =
-      ''
-        eval "$cargoDepsHook"
+  # Tests don't currently work for `no_std`, and all custom sysroots are currently built without `std`.
+  # See https://os.phil-opp.com/testing/ for more information.
+  # assert useSysroot -> !finalAttrs.doCheck;
 
-        export RUST_LOG=${logLevel}
-      ''
-      + (args.postUnpack or "");
-
-    configurePhase =
-      args.configurePhase or ''
+  (
+    (
+      # TODO: figure out something better; probably just do it from a shell, since that's much easier
+      if (args ? env && args.env ? RUSTFLAGS) then
+        {
+          env = args.env // {
+            RUSTFLAGS = EXTRA_RUSTFLAGS + " " + args.env.RUSTFLAGS;
+          };
+        }
+      else
+        {
+          RUSTFLAGS = EXTRA_RUSTFLAGS + " " + (toString args.RUSTFLAGS or "");
+          env = args.env or { };
+        }
+    )
+    // {
+      # Toggles whether a custom sysroot is created when the target is a .json file.
+      __internal_dontAddSysroot = false;
+
+      # TODO: this doesn't need to be here, but the documentation is useful
+      #       maybe move into the hooks, since that's where it's used...
+
+      # Needed to `pushd`/`popd` into a subdir of a tarball if this subdir
+      # contains a Cargo.toml, but isn't part of a workspace (which is e.g. the
+      # case for `rustfmt`/etc from the `rust-sources).
+      # Otherwise, everything from the tarball would've been built/tested.
+      buildAndTestSubdir = null;
+
+      cargoPatches = [ ];
+
+      cargoDeps =
+        if (finalAttrs ? cargoVendorDir && finalAttrs.cargoVendorDir != null) then
+          null
+        else if
+          (finalAttrs ? passthru && finalAttrs.passthru ? cargoLock && finalAttrs.passthru.cargoLock != null)
+        then
+          importCargoLock finalAttrs.passthru.cargoLock
+        else if
+          (
+            (finalAttrs ? cargoHash && finalAttrs.cargoHash != null)
+            || (finalAttrs ? cargoSha256 && finalAttrs.cargoSha256 != null)
+          )
+        then
+          fetchCargoTarball (
+            (takeAttrs [
+              "src"
+              "srcs"
+              "sourceRoot"
+              "preUnpack"
+              "unpackPhase"
+              "postUnpack"
+            ] finalAttrs)
+            // {
+              cargoUpdateHook = args.cargoUpdateHook or ""; # TODO: why was this removed with removeAttrs? Should probably keep it for consistency
+              name = finalAttrs.cargoDepsName;
+              patches = finalAttrs.cargoPatches;
+            }
+            // lib.optionalAttrs (args ? cargoHash) {
+              hash = finalAttrs.cargoHash;
+            }
+            // lib.optionalAttrs (args ? cargoSha256) {
+              sha256 = lib.warn "cargoSha256 is deprecated. Please use cargoHash with SRI hash instead" finalAttrs.cargoSha256;
+            }
+            // args.depsExtraArgs or { }
+          )
+        else
+          throw "cargoHash, cargoVendorDir, or cargoLock must be set";
+
+      cargoDepsName = finalAttrs.name or "${finalAttrs.pname}-${finalAttrs.version}";
+
+      # TODO: decide if we should keep these aliases or just always use the longer version
+      buildNoDefaultFeatures = false;
+      checkNoDefaultFeatures = finalAttrs.buildNoDefaultFeatures;
+      buildType = "release";
+      checkType = finalAttrs.buildType;
+      buildFeatures = [ ];
+      checkFeatures = finalAttrs.buildFeatures;
+
+      cargoBuildType = finalAttrs.buildType;
+      cargoCheckType = finalAttrs.checkType;
+      cargoBuildNoDefaultFeatures = finalAttrs.buildNoDefaultFeatures;
+      cargoCheckNoDefaultFeatures = finalAttrs.checkNoDefaultFeatures;
+      cargoBuildFeatures = finalAttrs.buildFeatures;
+      cargoCheckFeatures = finalAttrs.checkFeatures;
+
+      patchRegistryDeps = ./patch-registry-deps;
+
+      auditable = !cargo-auditable.meta.broken;
+
+      useNextest = false;
+
+      nativeBuildInputs =
+        args.nativeBuildInputs or [ ]
+        ++ lib.optionals finalAttrs.auditable [
+          (buildPackages.cargo-auditable-cargo-wrapper.override {
+            inherit cargo cargo-auditable;
+          })
+        ]
+        ++ [
+          cargoBuildHook
+          (if finalAttrs.useNextest then cargoNextestHook else cargoCheckHook)
+          cargoInstallHook
+          cargoSetupHook
+          rustc
+        ];
+
+      buildInputs =
+        args.buildInputs or [ ]
+        ++ lib.optionals stdenv.hostPlatform.isDarwin [ libiconv ]
+        ++ lib.optionals stdenv.hostPlatform.isMinGW [ windows.pthreads ];
+
+      patches = finalAttrs.cargoPatches ++ args.patches or [ ];
+
+      PKG_CONFIG_ALLOW_CROSS = if stdenv.buildPlatform != stdenv.hostPlatform then 1 else 0;
+
+      # TODO: decide if we should always set this, or just use `or ""`
+      #       also, maybe migrate postUnpack into a hook
+      logLevel = "";
+
+      postUnpack =
+        ''
+          eval "$cargoDepsHook"
+
+          export RUST_LOG=${finalAttrs.logLevel}
+        ''
+        + (args.postUnpack or "");
+
+      configurePhase = ''
         runHook preConfigure
         runHook postConfigure
       '';
 
-    doCheck = args.doCheck or true;
+      doCheck = true;
 
-    strictDeps = true;
+      strictDeps = true;
 
-    meta = meta // {
-      badPlatforms = meta.badPlatforms or [ ] ++ rustc.badTargetPlatforms;
-      # default to Rust's platforms
-      platforms = lib.intersectLists meta.platforms or lib.platforms.all rustc.targetPlatforms;
-    };
-  }
+      passthru =
+        lib.optionalAttrs (args ? cargoLock) {
+          cargoLock = args.cargoLock;
+        }
+        // args.passthru or { };
+
+      meta = args.meta or { } // {
+        badPlatforms = args.meta.badPlatforms or [ ] ++ rustc.badTargetPlatforms;
+        # default to Rust's platforms
+        platforms = lib.intersectLists (args.meta.platforms or lib.platforms.all) rustc.targetPlatforms;
+      };
+    }
+    // lib.removeAttrs args namesToRemove
+  )
 )