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

rqbit: 7.0.1 -> 8.0.0 #372777

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

ToasterUwU
Copy link
Member

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • 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/)
  • 25.05 Release Notes (or backporting 24.11 and 25.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.

Add a 👍 reaction to pull requests you find important.

@ToasterUwU ToasterUwU requested a review from cafkafk January 10, 2025 23:03
@ToasterUwU
Copy link
Member Author

@Daru-san Only mildly related, but best place to do this i think.
Wanna give the desktop app thing another try? Since there has been an update, maybe something works differently now and the issue is fixed or at least easier to fix.

@gdamjan
Copy link
Contributor

gdamjan commented Jan 14, 2025

also you need to update hash and npmDepsHash

@ToasterUwU
Copy link
Member Author

I do? When i changed the version and tried to built, only the hashes i changed were not identical anymore. And since it uses the version number from the top to fetch all the things it needs, i dont see why or even to what i would need to change those hashes.

@gdamjan
Copy link
Contributor

gdamjan commented Jan 14, 2025

I do? When i changed the version and tried to built, only the hashes i changed were not identical anymore. And since it uses the version number from the top to fetch all the things it needs, i dont see why or even to what i would need to change those hashes.

yes you do, if you don't change the src hash, nix will download the old cached (7.0.1) version from the nix caches

@Daru-san
Copy link
Contributor

Daru-san commented Jan 14, 2025

@ToasterUwU I am free to help get the desktop app working. Thankfully tauri was updated to 2.0.0 in this release, so it may work better this time.
Although it could be necessary to open a separate PR to do that, since that could take any magnitude of time to complete.

@ToasterUwU
Copy link
Member Author

@Daru-san Yeah i wanted this in a seperate PR anyways. So just open one when this one gets merged and ping me to let me know.

@@ -12,7 +12,7 @@
let
pname = "rqbit";

version = "7.0.1";
version = "8.0.0";

src = fetchFromGitHub {
Copy link
Member

Choose a reason for hiding this comment

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

You need to update src.hash and npmDepsHash too.

Also the lib.optionals stdenv.hostPlatform.isDarwin [ darwin.apple_sdk.frameworks.SystemConfiguration ]; below can be removed.

av-gal

This comment was marked as duplicate.

@av-gal
Copy link
Contributor

av-gal commented Jan 18, 2025

You could consider adding an update script like nix-update.

  passthru.updateScript = nix-update-script {
    extraArgs = [
      "--subpackage"
      "webui"
    ];
  };```

@gdamjan
Copy link
Contributor

gdamjan commented Jan 20, 2025

please update:

hash = "sha256-Meztr/UxLgnbd3YwkSW0vy+D2N4mFg2v+T4nBnYiQBI=";

and

npmDepsHash = "sha256-vib8jpf7Jn1qv0m/dWJ4TbisByczNbtEd8hIM5ll2Q8=";

@ToasterUwU
Copy link
Member Author

Yes im aware, just busy atm. Give me some time

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants