allow fetchpatch everywhere, without restrictions #188587
Conversation
This comment was marked as resolved.
This comment was marked as resolved.
ghost
closed this
ghost
reopened this
ofborg
bot
added
the
2.status: merge conflict
ofborg
bot
removed
the
2.status: merge conflict
ghost
mentioned this pull request
This commit assumes that NixOS/nix#7052 merges. If it has not yet merged and you did not cherry-pick it into your copy of Nix, revert this commit before testing.
ghost
marked this pull request as ready for review
ghost
mentioned this pull request
There was a problem hiding this comment.
I think we must drop 30a099f until the minimal nix version contains that fix :/
In addition to that can you do a treewide change to remove the warnings that fetchpatch cannot be used?
| # the parts of patchutils used by fetchpatch do not | ||
| # require perl, so we could eliminate this by patching | ||
| # patchutils' build scripts... | ||
| perl = bootstrapTools; |
There was a problem hiding this comment.
I am not sure if bootstrapTools contains anything related to perl. If not then I think it would be cleaner to exclude it like makeWrapper.
There was a problem hiding this comment.
The bootstrapTools contain a copy of perl. Mainly because autoconf/automake need it.
I would love to eliminate perl from the bootstrapTools. I tried a while back and while it is probably possible it is a very major undertaking. If it ever were eliminated we could (as my comment explains) apply a small patch to patchutils to drop all the parts of it that need perl (fetchpatch doesn't use those parts).
Co-authored-by: Sandro <sandro.jaeckel@gmail.com>
Co-authored-by: Sandro <sandro.jaeckel@gmail.com>
Co-authored-by: Sandro <sandro.jaeckel@gmail.com>
Co-authored-by: Sandro <sandro.jaeckel@gmail.com>
This reverts commit 30a099f.
I have reverted 30a099f. NixOS/nix#7052 just merged, so "minimal nix version" for 30a099f is now known to be "2.12". Keep in mind that this PR still needs
Yes, once this PR is deemed mergeable I would be happy to do that (and squash these commits too). But a treewide like that will accrue tons of merge conflicts if is is left sitting unmerged, so I'd rather wait until "the last minute" to push that commit into this PR. |
The method for signaling that `__impure` is desired went through a few minor revisions between this PR being filed and NixOS/nix#7052 being merged. This commit accounts for those changes so the "pre-Nix-2.12 emulation" uses the same interface that Nix 2.12 will use. See also: NixOS/nix#7052 (comment)
|
@ofborg eval |
@SuperSandro2000, please see the giant text at the very top of the PR. If there were some way I could add |
|
I guess I should draftify this until/if impure-derivations is stabilized. That may be years in the future, when this patch no longer applies, but I'm cool with that. I just wanted to prove that this was possible. |
ghost
marked this pull request as draft
ghost
mentioned this pull request
This commit adds an optional `__impure` parameter to fetchurl.nix, which allows the caller to use `libfetcher`'s fetcher in an impure derivation. This allows nixpkgs' patch-normalizing fetcher (fetchpatch) to be rewritten to use nix's internal fetchurl, thereby eliminating the awkward "you can't use fetchpatch here" banners scattered all over the place. See also: NixOS/nixpkgs#188587
|
This needs |
Description of changes
This PR eliminates all restrictions on the use of
fetchpatch, and makes a point of using it in the earliest part of thestdenvbootstrap (i.e.binutils) to demonstrate the functionality and serve as a regression test going forward.If this merges it can be followed by a treewide to delete all of the (untested and untestable) "cannot use fetchpatch" comments.
See also NixOS/nix#7052; if you are running Nix >=2.12 you can revert the last two commits in this series.
Things done
stdenv) on platform(s)sandbox = trueset innix.conf? (See Nix manual)nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)