nix-upfetch: init at 0.1.0 #56311
nix-upfetch: init at 0.1.0 #56311
Conversation
GrahamcOfBorg
added
8.has: package (new)
| }; | ||
|
|
||
| in stdenv.mkDerivation rec { | ||
| name = "${pname}-${version}"; |
There was a problem hiding this comment.
This is not necessary – mkDerivation does that by default.
There was a problem hiding this comment.
I had it for backwards compatibility, since I developed / tested it under 18.09, but since this PR is based on master, I can indeed remove it.
There was a problem hiding this comment.
I also tried removing it at rustPlatform.buildRustPackage, but it still expects a name attribute, so I will keep it there.
| ]; | ||
|
|
||
| configurePhase = '' | ||
| . configure.sh |
There was a problem hiding this comment.
Does [configureScript] (
nixpkgs/pkgs/stdenv/generic/setup.sh
Line 1002 in bccab16
There was a problem hiding this comment.
It probably will, simply did not know about it, thanks for the suggestion!
There was a problem hiding this comment.
Tried it, but since it is simply setting a variable, turning it into a configure scripts complicates matters unnecessary, so I am keeping it as it is for now, unless I find some better way.
| ''; | ||
|
|
||
| installPhase = '' | ||
| lib=$out/lib/${pname} |
There was a problem hiding this comment.
Why not create a makefile in the project?
There was a problem hiding this comment.
I have limited Makefile experience, what would this gain me? If it's worth doing, could you maybe give an example of what you have in mind?
There was a problem hiding this comment.
I think one reason might be that most of that code in installPhase could remain upstream. I get that it seems silly to you as you are upstream... but keeping code like that upstream wherever possible is the NixOS way.
We're so close on this PR. It would be nice to see it to completion.
There was a problem hiding this comment.
@aanderse If you want me to add a Makefile, I will, but I will not be attempting this myself without being pointed to some Makefile that does something similar to what I need to do, because I have only limited experience with making Makefiles (small C projects).
There was a problem hiding this comment.
If @msteen wants to make nix-upfetch available in different package repositories in the future, he can upstream his script and update this derivation.
| ''; | ||
|
|
||
| installPhase = '' | ||
| lib=$out/lib/${pname} |
There was a problem hiding this comment.
If @msteen wants to make nix-upfetch available in different package repositories in the future, he can upstream his script and update this derivation.
|
|
||
| let | ||
| nix-update-fetch = rustPlatform.buildRustPackage rec { | ||
| name = "${pname}-${version}"; |
There was a problem hiding this comment.
| name = "${pname}-${version}"; |
|
|
||
| src = fetchFromGitHub { | ||
| owner = "msteen"; | ||
| repo = "nix-update-fetch"; |
There was a problem hiding this comment.
| repo = "nix-update-fetch"; | |
| repo = pname; |
|
I never had any plans of packaging it for other distros. However since the suggestion to remove the few lines of script that might be reusable in other distros has been turned into a blocking issue, I choose to close this PR, because my local version has long since diverged from this version. The local version fits my needs, but in its current state is not ready to be published and at the moment I have no interest in putting in the time to patch up this old version or make my local version ready to be published again. So unless someone wants to see it in nixpkgs other than me, I will just make a new PR once I find the time and interest to make changes to the project again. |
|
@msteen sorry if my short comment gave the wrong impression, but I do not think the script upstreaming thing is a blocking issue and didn't mean to give that impression. That said, based on your last post, it sounds fine to close this. |
|
@ryantm Well, regardless of original intent, whether it meant it to be purely a suggestion or a blocking issue, it was as far as I know the only reason it was not merged. And since everybody else kept on focusing on this suggestion as if it had to be cleared to be merged, it actually did turn into a block issue. Regardless of this, I indeed explained why by now it makes sense to just close it. |
Motivation for this change
Update a call made to a fetcher function call and its surrounding bindings.
Based on
nix-prefetch(to locate the fetcher call and update the fetcher arguments passed to it) andrnix(to parse the file containing the fetcher call and update fetcher arguments and surrounding bindings).Makes it really convenient to update Nix packages as described in the README:
https://github.com/msteen/nix-upfetch
Things done
sandboxinnix.confon non-NixOS)nix-shell -p nox --run "nox-review wip"./result/bin/)nix path-info -Sbefore and after)