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

coolercontrol.*: 1.1.1 -> 1.2.2 #300711

Merged
merged 3 commits into from
Apr 13, 2024
Merged

coolercontrol.*: 1.1.1 -> 1.2.2 #300711

merged 3 commits into from
Apr 13, 2024

Conversation

codifryed
Copy link
Contributor

@codifryed codifryed commented Apr 1, 2024

Description of changes

Update for CoolerControl 1.1.1 -> 1.2.2

https://gitlab.com/coolercontrol/coolercontrol/-/releases/1.2.0
https://gitlab.com/coolercontrol/coolercontrol/-/releases/1.2.1
https://gitlab.com/coolercontrol/coolercontrol/-/releases/1.2.2

No breaking changes.

A nice-to-have would be to include any package changes to allow the coolercontrold systemd daemon access to sh and nvidia binaries if present.
https://gitlab.com/coolercontrol/coolercontrol/-/issues/263
(help needed)

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/)
  • 24.05 Release Notes (or backporting 23.05 and 23.11 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.

@codifryed codifryed requested a review from OPNA2608 April 1, 2024 14:37
@codifryed codifryed changed the title coolercontrol.*: 1.1.1 -> 1.2.0 coolercontrol.*: 1.1.1 -> 1.2.1 Apr 2, 2024
@OPNA2608
Copy link
Contributor

Am abit strapped for time right now, but will get to this on the weekend.

@OPNA2608
Copy link
Contributor

I have added 2 commits (hope pushing to your branch was okay):

  1. Replace sh in the coolercontrold source with a runtimeShell, which saves us the bash wrapping in the non-nvidia case.
  2. Add an option to enable Nvidia support in the module. Hoping to get it to default on/off based on nvidia driver usage - still tinkering with that.

@codifryed
Copy link
Contributor Author

Thanks @OPNA2608 !

I'm build testing for a small patch, the webkit2gtk build is running so I'll wait until that's done and clean that up and push. (That build takes quite some time) And no prob commiting to the branch, appreciate the help. 👍🏼

@codifryed codifryed changed the title coolercontrol.*: 1.1.1 -> 1.2.1 coolercontrol.*: 1.1.1 -> 1.2.2 Apr 13, 2024
@codifryed
Copy link
Contributor Author

All good from my side now. No patches in the near future planned.

@OPNA2608
Copy link
Contributor

@ofborg build coolercontrol.coolercontrol-ui-data coolercontrol.coolercontrold coolercontrol.coolercontrold.passthru.tests coolercontrol.coolercontrol-liqctld coolercontrol.coolercontrol-gui

@OPNA2608
Copy link
Contributor

Builds on ofborg, and works locally. I don't use Nvidia hardware on Linux, so I can only test the option insofar that:

  • enabling the option manually causes nvidia driver & tools to be built
  • the necessary paths are now in the service's overrides:
    Environment="PATH=/nix/store/y2vzxvhy6lmzn290r4yray2rdsk76cq1-nvidia-x11-545.29.02-6.6.10-bin/bin:/nix/store/678lpd6jbbrzhqnisynjzjmxblaq8nn3-nvidia-settings-545.29.02/bin:[...]
    
  • at runtime I get as far as in the issue - a communication error with nvidia-smi, likely because I'm not running Nvidia

LGTM.

@OPNA2608 OPNA2608 merged commit fdfe57c into NixOS:master Apr 13, 2024
35 of 37 checks passed
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.

2 participants