newlisp: init at 10.7.5 #326778
newlisp: init at 10.7.5 #326778
Conversation
|
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/prs-ready-for-review/3032/4517 |
| pname = "newlisp"; | ||
| version = "10.7.5"; | ||
|
|
||
| src = fetchurl { |
There was a problem hiding this comment.
fetchzip is better than fetchurl, see https://discourse.nixos.org/t/fetchzip-or-fetchurl-which-should-i-use/36572/2
This is just a suggestion which does not block the merge of this PR.
There was a problem hiding this comment.
But I guess that leaving the unpacking process to an explicit, (easier-to) injectable phase may provides a better interface.
There was a problem hiding this comment.
leaving the unpacking process to an explicit, (easier-to) injectable phase may provides a better interface.
Could you give me a concrete example showing the benefits?
FYI, it indeed happened that upstream changed mtime of files in the tarball and thus invalidated our hash of fetchurl.
As I said above, I do not have a strong opinion on this and can merge this PR as is.
There was a problem hiding this comment.
No. Just a matter of sense. Have no strong preference, too.
|
cc @NixOS/lisp for review |
| email = "rc-zb@outlook.com"; | ||
| github = "rc-zb"; | ||
| githubId = 161540043; | ||
| }; |
| version = "10.7.5"; | ||
|
|
||
| src = fetchurl { | ||
| url = "https://www.newlisp.org/downloads/newlisp-${finalAttrs.version}.tgz"; |
There was a problem hiding this comment.
I love the use of finalAttrs.version but note that it will always fail the embedded hash here, unless you allow overriding that, too. Which is probably overkill since it's easier for anyone who wants a different version to just bring their own src and inject that, but it's worth noting that this doesn't actually have the effect it suggests it will.
There was a problem hiding this comment.
unless you allow overriding that
It is possible to override hash separately, see #310373 (comment)
There was a problem hiding this comment.
I do this just for reusing the version while keeping the scope clean (unlike the rough rec). Never thought about someone overriding version only. 😶
There was a problem hiding this comment.
Nice yeah good point you can do a nested override. Fair enough!
| pname = "newlisp"; | ||
| version = "10.7.5"; | ||
|
|
||
| src = fetchurl { |
There was a problem hiding this comment.
leaving the unpacking process to an explicit, (easier-to) injectable phase may provides a better interface.
Could you give me a concrete example showing the benefits?
FYI, it indeed happened that upstream changed mtime of files in the tarball and thus invalidated our hash of fetchurl.
As I said above, I do not have a strong opinion on this and can merge this PR as is.
Description of changes
Add the reference implementation of newLISP, a Lisp-like, general-purpose scripting language.
And add myself to the maintainer lists of Nixpkgs as well as this package.
Limitations
meta.platforms.Things done
nix.conf? (See Nix manual)sandbox = relaxedsandbox = truenix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)Add a 👍 reaction to pull requests you find important.