-
-
Notifications
You must be signed in to change notification settings - Fork 14.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
sirula: 1.0.0 -> unstable-2023-09-02 #281963
Conversation
}; | ||
}; | ||
|
||
cargoSha256 = lib.fakeSha256; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cargoSha256 = lib.fakeSha256; |
this is not needed.
@@ -8,18 +9,27 @@ | |||
|
|||
rustPlatform.buildRustPackage rec { | |||
pname = "sirula"; | |||
version = "1.0.0"; | |||
version = "unstable-2023-09-02"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's been a recent change in versioning scheme, prompting unstable versions to be prefixed with 0-
: https://github.com/NixOS/nixpkgs/blob/master/pkgs/README.md#package-naming
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If a package is a commit from a repository without a version assigned, then the version attribute should be the latest upstream version preceding that commit, followed by -unstable- and the date of the (fetched) commit. The date must be in "YYYY-MM-DD" format.
so that makes it 1.0.1-unstable-2023-09-02
-- 0-unstable...
is only when there exists no previous version.
there is a version bump in the commits from v1.0.0 to the current rev.
DorianRudolph/sirula@v1.0.0...b15efe8
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, I was a bit quick. Yes, almost. I think it should be 1.0.0-unstable-2023-09-02
, since the current version is 1.0.0
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, I was a bit quick. Yes, almost. I think it should be
1.0.0-unstable-2023-09-02
, since the current version is1.0.0
note that there is a version bump to 1.0.1 along with the update, though the git tag was not updated.
DorianRudolph/sirula@v1.0.0...b15efe8
or maybe in am misreading the diff
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, no I see it too. Do you know whether that would be an official release, or whether the maintainers just prebumps the version in preparation for a release?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
DorianRudolph/sirula@70b5ce1 doesn't really say much
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looking at the commit for tag v1.0.0
DorianRudolph/sirula@24c7151
shows the version bump in cargo.toml from 0.0.1 -> 1.0.0
my guess is 1.0.1 but am fine with either as the tag suggests 1.0.0
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll open an issue for clarification upstream :)
rev = "v${version}"; | ||
sha256 = "sha256-C5mVO10+jD4TDg6R9rVJO6fdDiOD5tT+bEaI53WVdFA="; | ||
rev = "b15efe85ef1fe50849a33e5919d53d05f4f66090"; | ||
sha256 = "sha256-S0WbqY49nKaBUMWfgDKZxFLJuk7uFcnTfV8s86V0Zxs="; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since we're version bumping anyway
sha256 = "sha256-S0WbqY49nKaBUMWfgDKZxFLJuk7uFcnTfV8s86V0Zxs="; | |
hash = "sha256-S0WbqY49nKaBUMWfgDKZxFLJuk7uFcnTfV8s86V0Zxs="; |
nativeBuildInputs = [ | ||
pkg-config | ||
]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would be nice to either not do this, or put it in a different cleanup commit to keep the diff clean.
Done in #309535. |
Description of changes
I updated the sirula source, and had to make a few changes to the code to comply with a git source. Seems like the compilation error of the osstrtools dependency was failing, so the program's author decided that he would add a specific version of this dependency to the Cargo.toml. Because of that, I had to add the cargoLock session.
Things done
nix.conf
? (See Nix manual)sandbox = relaxed
sandbox = true
nix-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.