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

tlpui: init at 1.5.0 #188278

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from
Draft

Conversation

GeorgesAlkhouri
Copy link
Member

Description of changes

Added tlpui as a nix package.

A GTK user interface for TLP written in Python
https://github.com/d4nj1/TLPUI

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/)
  • 22.11 Release Notes (or backporting 22.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
    • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
  • Fits CONTRIBUTING.md.

@Aleksanaa
Copy link
Member

Is there any unfinished work?

@GeorgesAlkhouri
Copy link
Member Author

Yes, unfortunately this was merely a quick draft to test the build of tlpui. Also see d4nj1/TLPUI#86 and d4nj1/TLPUI#112.

@Aleksanaa
Copy link
Member

Maybe write a simple patch on it and wait for the pr to be merged?

@d4nj1
Copy link

d4nj1 commented Apr 8, 2023

The issue is that TLP-UI needs an intrinsic default file on runtime and therefore it won't run properly when just deactivated as suggested in https://github.com/d4nj1/TLPUI/pull/112/files
Maybe the application will start then but it won't show the proper default configs at least for TLP versions 1.3 and onwards. As suggested in d4nj1/TLPUI#86 (comment) I could probably add some fallback for Nix packages but other than that you would have to patch this file yourself before creating the Nix package. I will not accept https://github.com/d4nj1/TLPUI/pull/112/files.

@GeorgesAlkhouri
Copy link
Member Author

GeorgesAlkhouri commented Apr 9, 2023

so the package know uses the intrinisic defaults provided by the tlp package.
One thing to keep in mind for NixOS users is that if you're using tlp via the NixOS module service.tlp, any changes you make to the /etc/tlp.conf file will be overwritten by NixOS with the values from services.tlp.settings defined in your nix configuration every time you rebuild the system.

Also, there is still an issue with using pkexec for the sudo dialog.

@wegank wegank added 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md 2.status: merge conflict labels Mar 19, 2024
@stale stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Mar 20, 2024
@LordGrimmauld
Copy link
Contributor

is this still being worked on? i might pick it up over the weekend, but i also understand the inherent flaws of using tlpui, as definitions in nix config will override it. A potential solution would be to have tlpui generate the tlp.conf in /etc/tlp.conf and move the nix module for tlp to use /etc/tlp.d/* dropping a file there instead?

@LordGrimmauld LordGrimmauld mentioned this pull request Apr 19, 2024
13 tasks
@LordGrimmauld
Copy link
Contributor

LordGrimmauld commented Apr 19, 2024

I gave this PR an update as well as looked into fixing the inherent flaws of the existing nixos module overriding what tlpui generates, that approach can be found in #305278. Happy to take criticism over there too, i understand i made some changes that need to be considered carefully

@wegank wegank added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jul 4, 2024
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