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

tuxedo-rs: init at 0.2.2 #231384

Merged
merged 3 commits into from
Sep 27, 2023
Merged

tuxedo-rs: init at 0.2.2 #231384

merged 3 commits into from
Sep 27, 2023

Conversation

mrcjkb
Copy link
Member

@mrcjkb mrcjkb commented May 12, 2023

Description of changes

This adds tuxedo-rs, which consists of:

  • A dbus daemon as well as a simple CLI for interacting with the hardware of TUXEDO computers
  • A GUI (tailor_gui), which can be used to manage power/keyboard-led/fan profiles.

tuxedo-rs is an alternative to the TUXEDO Control Center (TCC), for which there exists a packaging request: #132206.

Note that if the packaging request for TCC is fulfilled, the daemons will conflict with each other, so the respective modules should ensure that the other one is disabled.

I had previously contributed a flake to tuxedo-rs, on which I have based this PR - with a small change to the module (options.services -> options.hardware).
If this is merged, I plan to rewrite the module in the flake to use this one and override the package src.
I have been using the tuxedo-rs module on my device for a few weeks now.

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.05 Release Notes (or backporting 22.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.

Copy link
Member

@geri1701 geri1701 left a comment

Choose a reason for hiding this comment

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

Please change the PR title from tuxedo-rs: intit at 0.2.1 to tuxedo-rs: init at 0.2.1.

@NobbZ NobbZ changed the title tuxedo-rs: intit at 0.2.1 tuxedo-rs: init at 0.2.1 May 21, 2023
@NobbZ
Copy link
Contributor

NobbZ commented May 21, 2023

Please rebase and fix merge conflicts.

Also personally I'd prefer to split "init" of the package and module into 2 commits. I am not sure about waht the consensus is on that. Therefore not "demanding" or "requiring" any change on that. Just putting it to the discussion here.

@mrcjkb
Copy link
Member Author

mrcjkb commented May 22, 2023

Please rebase and fix merge conflicts.

Also personally I'd prefer to split "init" of the package and module into 2 commits. I am not sure about waht the consensus is on that. Therefore not "demanding" or "requiring" any change on that. Just putting it to the discussion here.

I have rebased and split the tuxedo-rs init commit into separate package init and module init commits.

@mrcjkb mrcjkb force-pushed the tuxedo-rs branch 2 times, most recently from cc6629d to f8b1d4c Compare May 22, 2023 19:29
@mrcjkb mrcjkb requested a review from geri1701 May 22, 2023 19:29
Copy link
Member

@geri1701 geri1701 left a comment

Choose a reason for hiding this comment

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

If I clone your branch, I can build both tailor_gui and tuxedo-rs without problems. But something is obviously wrong with the output paths here, I don't know what to do about it at the moment, someone with more experience should take a look at that.

@mrcjkb
Copy link
Member Author

mrcjkb commented May 23, 2023

If I clone your branch, I can build both tailor_gui and tuxedo-rs without problems. But something is obviously wrong with the output paths here, I don't know what to do about it at the moment, someone with more experience should take a look at that.

Same here.
Thanks for the review 😄

@mrcjkb
Copy link
Member Author

mrcjkb commented May 23, 2023

It doesn't seem to like

inherit ((lib.importTOML "${src}/tailord/Cargo.toml").package) version;

and the use of builtins.path.

@geri1701
Copy link
Member

It doesn't seem to like

inherit ((lib.importTOML "${src}/tailord/Cargo.toml").package) version;

Yes, in the output of nixpkgs-review it looks like it doesn't like that part.

@mrcjkb
Copy link
Member Author

mrcjkb commented May 23, 2023

It seems like it also doesn't like

cargoDeps = rustPlatform.importCargoLock {
    lockFile = src + "/Cargo.lock";
};

if lockFile is in the source repo.

The latest changes are working on my local machine (tested with nix-env -qaP --out-path -f default.nix --show-trace).

@mrcjkb
Copy link
Member Author

mrcjkb commented Sep 22, 2023

🙏

@mrcjkb mrcjkb requested a review from figsoda September 22, 2023 14:35
@mrcjkb
Copy link
Member Author

mrcjkb commented Sep 22, 2023

# x86_64-linux
> nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD"
2 packages added:
tailor_gui tuxedo-rs (init at 0.2.1)

1 package updated:
nixos-install-tools

1 package blacklisted:
nixos-install-tools

2 packages built:
tailor-gui tuxedo-rs

@mrcjkb mrcjkb requested a review from figsoda September 22, 2023 18:18
@Artturin
Copy link
Member

Artturin commented Sep 24, 2023

added platforms to meta

@mrcjkb
Copy link
Member Author

mrcjkb commented Sep 24, 2023

Updated src.
Note: The author has not changed the version yet.

@mrcjkb mrcjkb changed the title tuxedo-rs: init at 0.2.1 tuxedo-rs: init at 0.2.2 Sep 26, 2023
@mrcjkb
Copy link
Member Author

mrcjkb commented Sep 26, 2023

Updated from 0.2.1 -> 0.2.2.

@Artturin Artturin merged commit 31c6472 into NixOS:master Sep 27, 2023
25 checks passed
@mrcjkb mrcjkb deleted the tuxedo-rs branch September 27, 2023 05:11
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.