unFTP: init at 0.14.5 #281588
unFTP: init at 0.14.5 #281588
Conversation
rafaelsgirao
force-pushed
the
rafaelsgirao/package-unFTP
branch
from
6445b0b
to
42c3589
Compare
|
|
||
| src = fetchFromGitHub { | ||
| owner = "bolcom"; | ||
| repo = pname; |
There was a problem hiding this comment.
pname and repo are not considered the same and can cause issues down the path.
| repo = pname; | |
| repo = "unFTP"; |
| owner = "bolcom"; | ||
| repo = pname; | ||
| rev = "v${version}"; | ||
| sha256 = "sha256-CPq4CH7reS5AI145l8U1nekm2MKnmoBfVqENR9QOKF4"; |
There was a problem hiding this comment.
| sha256 = "sha256-CPq4CH7reS5AI145l8U1nekm2MKnmoBfVqENR9QOKF4"; | |
| hash = "sha256-CPq4CH7reS5AI145l8U1nekm2MKnmoBfVqENR9QOKF4"; |
There was a problem hiding this comment.
Why is hash preferred over sha256? I used to use hash, but thought being explicit on the hash type was better.
There was a problem hiding this comment.
The "sha256-" part of the hash is already making it specific (:
There was a problem hiding this comment.
I'll provide some more details.
This comes from nix adding support for SRI hashes, using the hash argument. There are several reasons why SRI hashes are preferred to normal hashes, but I believe most of it comes down to compatibility with the rest of the ecosystem. Somewhere along the road, the old to-be-deprecated-at-some-point arguments like sha256 also started accepting SRI hashes, creating a bit of confusion. There has been discussions around disabling this behavior to be more consistent, but nothing has been done at the moment. Also see RFC 131 for some relevant discussions.
You'll frequently see reviewers add this suggestion to both new packages, but also version bumps for packages that still use the "legacy" hashes (with legacy in quotes because AFAIK they haven't been officially deprecated anywhere)
| unFTP = callPackage ../servers/unftp { }; | ||
|
|
There was a problem hiding this comment.
The package should be placed under the pkgs/by-name/so/some-package/package.nix hierarchy. Which would make this unnecessary.
| unFTP = callPackage ../servers/unftp { }; |
| rafaelsgirao = { | ||
| email = "[email protected]"; | ||
| github = "rafaelsgirao"; | ||
| githubId = 7544832; | ||
| name = "Rafael Girão"; | ||
| }; |
There was a problem hiding this comment.
The commit adding you as a maintainer should come before the package commit (:
|
|
||
| cargoSha256 = "sha256-vFu1D2GYItVGeTmd/rwmZHM/mf4zQ3tzBLux7vb+yZ0="; | ||
|
|
||
| meta = with lib; { |
There was a problem hiding this comment.
Please add the meta.mainProgram tag.
| }: | ||
|
|
||
| rustPlatform.buildRustPackage rec { | ||
| pname = "unFTP"; |
There was a problem hiding this comment.
| pname = "unFTP"; | |
| pname = "unftp"; |
pname must not contain uppercase letters.
ofborg
bot
added
8.has: package (new)
| homepage = "https://unftp.rs"; | ||
| license = licenses.asl20; | ||
| maintainers = with maintainers; [ rafaelsgirao ]; | ||
| platforms = platforms.unix; |
There was a problem hiding this comment.
I'm a bit unsure if this is reasonable, considering that we build it with gnu features and use linux-pam. I haven't looked further into it, but it sounds like this package would only run on linux, and not something like OpenBSD.
wegank
added
the
2.status: merge conflict
Description of changes
https://unftp.rs
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.