Skip to content
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

chia: 1.7.0 -> 2.0.1 #252913

Closed
wants to merge 6 commits into from
Closed

chia: 1.7.0 -> 2.0.1 #252913

wants to merge 6 commits into from

Conversation

bbjubjub2494
Copy link
Member

Description of changes

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 23.11 Release Notes (or backporting 23.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Sorry, something went wrong.

@ofborg ofborg bot added 11.by: package-maintainer This PR was created by the maintainer of the package it changes 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10 labels Sep 2, 2023
@bbjubjub2494
Copy link
Member Author

I think the python tooling is not checking dependencies...

@abueide
Copy link
Contributor

abueide commented Sep 5, 2023

Just curious what the status on this is, let me know if there's anyway I can help! Is it possible we can add an option for the electron gui app to this package or should we add it in a new separate package?

@bbjubjub2494
Copy link
Member Author

Hey, so, what I usually do is copy over the (minimum required subset of) derivations I have in lourkeur/chia.nix and fix anything that comes up. This time around there's an additional difficulty because of #253131. The main reason doCheck is off here is because the test suite is heavy, but maybe we can do pythonImportsCheck? Hopefully it catches both import issues and dependency issues. Do you have a moment to test that?

@chasevasic
Copy link

chasevasic commented Oct 18, 2023

Just curious what the status on this is, let me know if there's anyway I can help! Is it possible we can add an option for the electron gui app to this package or should we add it in a new separate package?

I'm not a package maintainer, however I'll mention that before Nix had Chia, I wrote a derivation for personal use. The GUI is both more difficult to package and maintain, as well as having more dependencies many people would not want to pull in (e.g. if they are running Chia on a server, raspberry pi, headless nodes in general.) (cough electron) It would be great to have the GUI in nixpkgs. I would say definitely a seperate package.

I'd love to help with Chia too, I've been a Chia supporter since the mainnet launch day. I don't currently know any of the best practices or style rules for nixpkgs, and couldn't currently commit to being a maintainer even if I was more familiar though.

btw it appears trivial to tweak this merge to be 2.1.2 instead of 2.0.1


meta = with lib; {
homepage = "https://github.com/Chia-Network/bladebit";
description = "BladeBit - Fast Chia (XCH) k32-only Plotter";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
description = "BladeBit - Fast Chia (XCH) k32-only Plotter";
description = "Fast Chia (XCH) k32-only Plotter";

https://nixos.org/manual/nixpkgs/unstable/#var-meta-description

meta = with lib; {
homepage = "https://github.com/Chia-Network/bladebit";
description = "BladeBit - Fast Chia (XCH) k32-only Plotter";
license = with licenses; [ asl20 ];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
license = with licenses; [ asl20 ];
license = licenses.asl20;

For a single license, we don't use the list.
https://nixos.org/manual/nixpkgs/unstable/#var-meta-license

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need this patch?

@@ -64,7 +66,7 @@ let chia = python3Packages.buildPythonApplication rec {
zstd
];

nativeCheckInputs = with python3Packages; [
checkInputs = with python3Packages; [
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@@ -6,14 +6,14 @@

let chia = python3Packages.buildPythonApplication rec {
pname = "chia";
version = "1.7.0";
version = "2.0.1";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
version = "2.0.1";
version = "2.0.1";
pyproject = true;

Could you please add pyproject = true?
https://github.com/NixOS/nixpkgs/blob/master/doc/languages-frameworks/python.section.md#buildpythonpackage-parameters-buildpythonpackage-parameters

@@ -13,24 +13,24 @@

buildPythonPackage rec {
pname = "blspy";
version = "1.0.16";
version = "2.0.2";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
version = "2.0.2";
version = "2.0.2";
pyproject = true;

buildPythonPackage rec {
pname = "chia-rs";
version = "0.2.0";
version = "0.2.10";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
version = "0.2.10";
version = "0.2.10";
pyproject = true;

@@ -39,8 +39,9 @@ buildPythonPackage rec {
touch wheel/Cargo.lock
'';

nativeCheckInputs = [
checkInputs = [
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
checkInputs = [
nativeCheckInputs = [

@@ -16,21 +18,27 @@

buildPythonPackage rec {
pname = "chiapos";
version = "1.0.11";
version = "2.0.2";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
version = "2.0.2";
version = "2.0.2";
pyproject = true;

Louis Bettens added 6 commits November 21, 2023 21:08

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
@bbjubjub2494
Copy link
Member Author

closed in favor of #270254

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
6.topic: python 8.has: package (new) This PR adds a new package 10.rebuild-darwin: 11-100 10.rebuild-linux: 11-100 11.by: package-maintainer This PR was created by the maintainer of the package it changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants