Skip to content

fetchFromGitHub: pass owner, repo, rev, tag to mkDerivation and make rev and tag overridable with <pkg>.overrideAttrs#456751

Merged
MattSturgeon merged 14 commits intoNixOS:masterfrom
ShamrockLee:fetchgithub-derivationArgs
Nov 5, 2025
Merged

fetchFromGitHub: pass owner, repo, rev, tag to mkDerivation and make rev and tag overridable with <pkg>.overrideAttrs#456751
MattSturgeon merged 14 commits intoNixOS:masterfrom
ShamrockLee:fetchgithub-derivationArgs

Conversation

@ShamrockLee
Copy link
Contributor

@ShamrockLee ShamrockLee commented Oct 29, 2025

This PR adds the derivationArgs argument to fetchers like fetchurl, fetchzip, and fetchgit for derived fetchers to pass down default arguments to stdenvNoCC.mkDerivation.

Contrasting to previous works to preserve those attributes after <pkg>.overrideAttrs using passtru, this PR passes them directly down stdenv.mkDerivation and make them overridable as, e.g., tag instead of passthru.tag. Even with a larger diff, it could better address #444503 and #370418 from the overriding perspective.

This PR surpasses:

Detail:

  • fetchurl, fetchzip, and fetchgit: Add argument derivationArgs
    It takes derivationArgs from extended build helpers and pass them all the way down stdenvNoCC.mkDerivation.
    Their precedence are lower than any other arguments specified by the current build helper or by the user.
  • fetchzip: Pass arguments extension and stripRoot down mkDerivation via derivationArgs, and make them overridable with <pkg>.overrideAttrs.
  • fetchgit: Make tag and rev overridable with <pkg>.overrideAttrs as tag and revCustom.
    • Pass tag directly to mkDerivation instead of via passthru.tag.
    • Pass rev to mkDerivation as revCustom (since revWithHash has been passed as rev).
    • Make other arguments observe the tag and revCustom overridden via <pkg>.overrideAttrs, i.e., finalAttrs.tag and finalAttrs.revCustom.
  • fetchgit: Abstract, simplify, and expose getRevWithHash
    • Abstract the revWithTag construction as getRevWithHash.
    • Simplify the conditional expression. The number of branch condition stays the same when either one of tag and rev is specified, and only increase when both unspecified (which is a rare edge-case).
    • Expose the helper function getRevWithTag as fetchgit.getRevWithTag so that other fetchers can form this value the same way.
  • Fix how fetchFromGitHub handle its owner, repo, rev, tag, and meta arguments, making them equally overridable whether the backend fetcher is fetchzip or fetchgit.
    • Pass meta properly via fetcherArgs.
    • Add finalAttrs to fetcherArgs to utilize fetchurl and fetchgit's fixed-point arguments support, as a workaround before fetchFromGitHub itself are reconstructed with lib.extendMkDerivation.
    • Pass owner and repo to mkDerivation via derivationArgs. We can start fixing their overriding after lib.extendMkDerivation reconstruction.
    • Make tag and rev pass the same way as fetchzip does when using the fetchzip backend, with the help of derivationArgs:
      finalAttrs:
      let
        revWithTag = finalAttrs.rev;
      in
      {
        derivationArgs = {
          inherit tag;
          revCustom = rev;
          rev = fetchgit.getRevWithTag {
            inherit (finalAttrs) tag;
            rev = finalAttrs.revCustom;
          };
        };
        # Other fetcher arguments using `revWithTag`
      }
  • Move name specification temporarily down the fetcherArg so as to reference from finalAttrs. We can move it back during reconstruction with lib.extendMkDerivation.
  • Pass githubBase with derivationArgs to prevent rebuilds when we make it overridable in the future.

Things done

  • Built on platform:
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • Tested, as applicable:
  • Ran nixpkgs-review on this PR. See nixpkgs-review usage.
  • Tested basic functionality of all binary files, usually in ./result/bin/.
  • Nixpkgs Release Notes
    • Package update: when the change is major or breaking.
  • NixOS Release Notes
    • Module addition: when adding a new NixOS module.
    • Module update: when the change is significant.
  • Fits CONTRIBUTING.md, pkgs/README.md, maintainers/README.md and other READMEs.

Add a 👍 reaction to pull requests you find important.

@nixpkgs-ci nixpkgs-ci bot added 10.rebuild-linux: 11-100 This PR causes between 11 and 100 packages to rebuild on Linux. 10.rebuild-darwin: 11-100 This PR causes between 11 and 100 packages to rebuild on Darwin. 6.topic: fetch Fetchers (e.g. fetchgit, fetchsvn, ...) labels Oct 29, 2025
@ShamrockLee ShamrockLee force-pushed the fetchgithub-derivationArgs branch from ab42386 to 27c7b8f Compare October 29, 2025 15:38
@ShamrockLee
Copy link
Contributor Author

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 456751
Commit: 27c7b8f6c286dabb62f86bda4f9ec5bde45781e7


x86_64-linux

❌ 1 package failed to build:
  • tests.fetchurl.header
✅ 48 packages built:
  • tests.fetchFirefoxAddon.simple (tests.fetchFirefoxAddon.overridden-source)
  • tests.fetchFromBitbucket.withEncodedWhitespace
  • tests.fetchFromBitbucket.withEncodedWhitespaceGit
  • tests.fetchFromBitbucket.withoutWhitespace
  • tests.fetchFromGitHub.dumb-http-signed-tag
  • tests.fetchFromGitHub.fetchTags
  • tests.fetchFromGitHub.leave-git
  • tests.fetchFromGitHub.rootDir
  • tests.fetchFromGitHub.simple
  • tests.fetchFromGitHub.sparseCheckout
  • tests.fetchFromGitHub.sparseCheckoutNonConeMode
  • tests.fetchFromGitHub.submodule-deep
  • tests.fetchFromGitHub.submodule-leave-git
  • tests.fetchFromGitHub.submodule-leave-git-deep
  • tests.fetchFromGitHub.submodule-simple
  • tests.fetchPypiLegacy.fetchSimple
  • tests.fetchgit.dumb-http-signed-tag
  • tests.fetchgit.fetchTags
  • tests.fetchgit.leave-git
  • tests.fetchgit.prefetch-git-no-add-path
  • tests.fetchgit.rootDir
  • tests.fetchgit.simple
  • tests.fetchgit.sparseCheckout
  • tests.fetchgit.sparseCheckoutNonConeMode
  • tests.fetchgit.submodule-deep
  • tests.fetchgit.submodule-leave-git
  • tests.fetchgit.submodule-leave-git-deep
  • tests.fetchgit.submodule-simple
  • tests.fetchgit.withGitConfig
  • tests.fetchpatch2.decode
  • tests.fetchpatch2.fileWithApostrophe
  • tests.fetchpatch2.fileWithSpace
  • tests.fetchpatch2.full
  • tests.fetchpatch2.hunks
  • tests.fetchpatch2.relative
  • tests.fetchpatch2.simple
  • tests.fetchtorrent.http-link (tests.fetchtorrent.http-link-transmission)
  • tests.fetchtorrent.http-link-rqbit (tests.fetchtorrent.http-link-rqbit-flattened)
  • tests.fetchtorrent.http-link-rqbit-unflattened
  • tests.fetchtorrent.magnet-link (tests.fetchtorrent.magnet-link-transmission)
  • tests.fetchtorrent.magnet-link-rqbit (tests.fetchtorrent.magnet-link-rqbit-flattened)
  • tests.fetchtorrent.magnet-link-rqbit-unflattened
  • tests.fetchzip.hiddenDir
  • tests.fetchzip.postFetch
  • tests.fetchzip.simple
  • tests.haskell.cabalSdist.assumptionLocalHasDirectReference
  • tests.haskell.cabalSdist.localHasNoDirectReference
  • tests.testers.runCommand.bork

Error logs: `x86_64-linux`
tests.fetchurl.header
  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0
  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0
  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0
curl: (22) The requested URL returned error: 503
Warning: Problem : HTTP error. Will retry in 1 second. 3 retries left.

0 0 0 0 0 0 0 0 --:--:-- --:--:-- --:--:-- 0
0 0 0 0 0 0 0 0 --:--:-- --:--:-- --:--:-- 0
curl: (22) The requested URL returned error: 503
Warning: Problem : HTTP error. Will retry in 1 second. 2 retries left.

0 0 0 0 0 0 0 0 --:--:-- --:--:-- --:--:-- 0
0 0 0 0 0 0 0 0 --:--:-- --:--:-- --:--:-- 0
curl: (22) The requested URL returned error: 503
Warning: Problem : HTTP error. Will retry in 1 second. 1 retry left.

0 0 0 0 0 0 0 0 --:--:-- --:--:-- --:--:-- 0
0 0 0 0 0 0 0 0 --:--:-- --:--:-- --:--:-- 0
curl: (22) The requested URL returned error: 503
error: cannot download source-salted-5zj6fci3fdrs from any mirror

@ShamrockLee
Copy link
Contributor Author

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 456751
Commit: 27c7b8f6c286dabb62f86bda4f9ec5bde45781e7


x86_64-linux

✅ 49 packages built:
  • tests.fetchFirefoxAddon.simple (tests.fetchFirefoxAddon.overridden-source)
  • tests.fetchFromBitbucket.withEncodedWhitespace
  • tests.fetchFromBitbucket.withEncodedWhitespaceGit
  • tests.fetchFromBitbucket.withoutWhitespace
  • tests.fetchFromGitHub.dumb-http-signed-tag
  • tests.fetchFromGitHub.fetchTags
  • tests.fetchFromGitHub.leave-git
  • tests.fetchFromGitHub.rootDir
  • tests.fetchFromGitHub.simple
  • tests.fetchFromGitHub.sparseCheckout
  • tests.fetchFromGitHub.sparseCheckoutNonConeMode
  • tests.fetchFromGitHub.submodule-deep
  • tests.fetchFromGitHub.submodule-leave-git
  • tests.fetchFromGitHub.submodule-leave-git-deep
  • tests.fetchFromGitHub.submodule-simple
  • tests.fetchPypiLegacy.fetchSimple
  • tests.fetchgit.dumb-http-signed-tag
  • tests.fetchgit.fetchTags
  • tests.fetchgit.leave-git
  • tests.fetchgit.prefetch-git-no-add-path
  • tests.fetchgit.rootDir
  • tests.fetchgit.simple
  • tests.fetchgit.sparseCheckout
  • tests.fetchgit.sparseCheckoutNonConeMode
  • tests.fetchgit.submodule-deep
  • tests.fetchgit.submodule-leave-git
  • tests.fetchgit.submodule-leave-git-deep
  • tests.fetchgit.submodule-simple
  • tests.fetchgit.withGitConfig
  • tests.fetchpatch2.decode
  • tests.fetchpatch2.fileWithApostrophe
  • tests.fetchpatch2.fileWithSpace
  • tests.fetchpatch2.full
  • tests.fetchpatch2.hunks
  • tests.fetchpatch2.relative
  • tests.fetchpatch2.simple
  • tests.fetchtorrent.http-link (tests.fetchtorrent.http-link-transmission)
  • tests.fetchtorrent.http-link-rqbit (tests.fetchtorrent.http-link-rqbit-flattened)
  • tests.fetchtorrent.http-link-rqbit-unflattened
  • tests.fetchtorrent.magnet-link (tests.fetchtorrent.magnet-link-transmission)
  • tests.fetchtorrent.magnet-link-rqbit (tests.fetchtorrent.magnet-link-rqbit-flattened)
  • tests.fetchtorrent.magnet-link-rqbit-unflattened
  • tests.fetchurl.header
  • tests.fetchzip.hiddenDir
  • tests.fetchzip.postFetch
  • tests.fetchzip.simple
  • tests.haskell.cabalSdist.assumptionLocalHasDirectReference
  • tests.haskell.cabalSdist.localHasNoDirectReference
  • tests.testers.runCommand.bork

@ShamrockLee
Copy link
Contributor Author

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 456751
Commit: c5f90acef939551aa220ff2b2f9d0f575286b1e7


x86_64-linux

✅ 49 packages built:
  • tests.fetchFirefoxAddon.simple (tests.fetchFirefoxAddon.overridden-source)
  • tests.fetchFromBitbucket.withEncodedWhitespace
  • tests.fetchFromBitbucket.withEncodedWhitespaceGit
  • tests.fetchFromBitbucket.withoutWhitespace
  • tests.fetchFromGitHub.dumb-http-signed-tag
  • tests.fetchFromGitHub.fetchTags
  • tests.fetchFromGitHub.leave-git
  • tests.fetchFromGitHub.rootDir
  • tests.fetchFromGitHub.simple
  • tests.fetchFromGitHub.sparseCheckout
  • tests.fetchFromGitHub.sparseCheckoutNonConeMode
  • tests.fetchFromGitHub.submodule-deep
  • tests.fetchFromGitHub.submodule-leave-git
  • tests.fetchFromGitHub.submodule-leave-git-deep
  • tests.fetchFromGitHub.submodule-simple
  • tests.fetchPypiLegacy.fetchSimple
  • tests.fetchgit.dumb-http-signed-tag
  • tests.fetchgit.fetchTags
  • tests.fetchgit.leave-git
  • tests.fetchgit.prefetch-git-no-add-path
  • tests.fetchgit.rootDir
  • tests.fetchgit.simple
  • tests.fetchgit.sparseCheckout
  • tests.fetchgit.sparseCheckoutNonConeMode
  • tests.fetchgit.submodule-deep
  • tests.fetchgit.submodule-leave-git
  • tests.fetchgit.submodule-leave-git-deep
  • tests.fetchgit.submodule-simple
  • tests.fetchgit.withGitConfig
  • tests.fetchpatch2.decode
  • tests.fetchpatch2.fileWithApostrophe
  • tests.fetchpatch2.fileWithSpace
  • tests.fetchpatch2.full
  • tests.fetchpatch2.hunks
  • tests.fetchpatch2.relative
  • tests.fetchpatch2.simple
  • tests.fetchtorrent.http-link (tests.fetchtorrent.http-link-transmission)
  • tests.fetchtorrent.http-link-rqbit (tests.fetchtorrent.http-link-rqbit-flattened)
  • tests.fetchtorrent.http-link-rqbit-unflattened
  • tests.fetchtorrent.magnet-link (tests.fetchtorrent.magnet-link-transmission)
  • tests.fetchtorrent.magnet-link-rqbit (tests.fetchtorrent.magnet-link-rqbit-flattened)
  • tests.fetchtorrent.magnet-link-rqbit-unflattened
  • tests.fetchurl.header
  • tests.fetchzip.hiddenDir
  • tests.fetchzip.postFetch
  • tests.fetchzip.simple
  • tests.haskell.cabalSdist.assumptionLocalHasDirectReference
  • tests.haskell.cabalSdist.localHasNoDirectReference
  • tests.testers.runCommand.bork

@adeci
Copy link
Member

adeci commented Oct 30, 2025

This derivationArgs approach is much cleaner. Attributes are directly accessible as finalAttrs.owner and override naturally now. Much better than nesting in passthru, which felt really hacky.
Tested successfully with my test case from #455637, and I'll be closing that with this succeeding it.

@ShamrockLee ShamrockLee marked this pull request as ready for review November 3, 2025 13:27
@ShamrockLee ShamrockLee force-pushed the fetchgithub-derivationArgs branch from c5f90ac to 8bb3bbb Compare November 3, 2025 13:31
@ShamrockLee
Copy link
Contributor Author

Rebased for CI fix. See issue #457845

Copy link
Contributor

@MattSturgeon MattSturgeon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Diff LGTM, and the rebuilds seem reasonable (exclusively tests). Others have tested, and CI is green.

@nixpkgs-ci nixpkgs-ci bot added the 12.approvals: 1 This PR was reviewed and approved by one person. label Nov 3, 2025
@MattSturgeon MattSturgeon added this pull request to the merge queue Nov 5, 2025
Merged via the queue into NixOS:master with commit 37a08ba Nov 5, 2025
32 of 34 checks passed
@SuperSandro2000
Copy link
Member

SuperSandro2000 commented Nov 7, 2025

This broke fetchgit with prefer-remote-fetch but that will also be fixed in #457313 most likely.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

6.topic: fetch Fetchers (e.g. fetchgit, fetchsvn, ...) 10.rebuild-darwin: 11-100 This PR causes between 11 and 100 packages to rebuild on Darwin. 10.rebuild-linux: 11-100 This PR causes between 11 and 100 packages to rebuild on Linux. 12.approvals: 1 This PR was reviewed and approved by one person.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants