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

init: workarounds.nixgl module #5332

Closed

Conversation

michaelCTS
Copy link

@michaelCTS michaelCTS commented Apr 24, 2024

Description

The option allows packages to be wrapped by nixgl in order to make GUI applications work when installed with home-manager. The goal of this PR is just to make it easy to use graphically accelerated programs installed with home-manager in a desktop environment.

No need to install and run nixGL $program. Just add the package to workaround.nixgl.packages like so [ { pkg = pkgs.some_package; }].

Anything more advanced can come in further PRs by other people.

Checklist

  • Change is backwards compatible.

  • Code formatted with ./format.

  • Code tested through nix-shell --pure tests -A run.all or nix develop --ignore-environment .#all using Flakes.

  • Test cases updated/added. See example.

  • Commit messages are formatted like

    {component}: {description}
    
    {long description}
    

    See CONTRIBUTING for more information and recent commit messages for examples.

  • If this PR adds a new module

    • Added myself as module maintainer. See example.

Maintainer CC

Closes #3968

@michaelCTS michaelCTS changed the title WIP: init: workarounds.opengl option WIP: #3968 init: workarounds.opengl option Apr 24, 2024
@michaelCTS michaelCTS changed the title WIP: #3968 init: workarounds.opengl option WIP: init: workarounds.opengl option Apr 24, 2024
modules/misc/opengl.nix Outdated Show resolved Hide resolved
modules/misc/opengl.nix Outdated Show resolved Hide resolved
@michaelCTS michaelCTS force-pushed the feat/add-nixgl-workaround branch 2 times, most recently from 4f9475c to 2fc2d54 Compare April 25, 2024 13:18
@michaelCTS michaelCTS changed the title WIP: init: workarounds.opengl option WIP: init: workarounds.nixgl option Apr 25, 2024
@michaelCTS michaelCTS force-pushed the feat/add-nixgl-workaround branch 2 times, most recently from 1e9c8b7 to b746b29 Compare April 25, 2024 13:22
@michaelCTS michaelCTS changed the title WIP: init: workarounds.nixgl option WIP: init: workarounds.nixgl module Apr 25, 2024
@michaelCTS
Copy link
Author

michaelCTS commented Apr 25, 2024

Well, I tried adding tests, but for some reason this fails

{ lib, pkgs, ... }:                                                                           

with lib;

{
 config = {
  workarounds.nixgl.packages = with pkgs; [{ pkg = alacritty; }];                           
  
  nmt.script = ''
   assertFileRegex .nix-profile/bin/alacritty 'exec "/nix/store/.*/bin/nixGL" .*/alacritty'
   assertLinkExists .nix-profile/share/applications/Alacritty.desktop                      
  '';
 };
}

It doesn't find the files even though when I run home-manage switch I do see them.

user@mikes-workstation:~/projects/home-manager$ nix-shell --pure tests -A run.nixgl-simple                                                    
nixgl-simple: FAILED                                                                                                                          
Expected .nix-profile/bin/alacritty to match exec "/nix/store/.*/bin/nixGL" .*/alacritty but it did not.                                      
For further reference please introspect /nix/store/jvnyika0ymigkyn2gp8v89w9i2d78bbz-nmt-report-nixgl-simple                                   
user@mikes-workstation:~/projects/home-manager$ find -H /nix/store/jvnyika0ymigkyn2gp8v89w9i2d78bbz-nmt-report-nixgl-simple                   
/nix/store/jvnyika0ymigkyn2gp8v89w9i2d78bbz-nmt-report-nixgl-simple                                                                           
/nix/store/jvnyika0ymigkyn2gp8v89w9i2d78bbz-nmt-report-nixgl-simple/tested                                                                    
/nix/store/jvnyika0ymigkyn2gp8v89w9i2d78bbz-nmt-report-nixgl-simple/script                                                                    
/nix/store/jvnyika0ymigkyn2gp8v89w9i2d78bbz-nmt-report-nixgl-simple/output                                                                    
/nix/store/jvnyika0ymigkyn2gp8v89w9i2d78bbz-nmt-report-nixgl-simple/result                                                                    
user@mikes-workstation:~/projects/home-manager$ find -H /nix/store/jvnyika0ymigkyn2gp8v89w9i2d78bbz-nmt-report-nixgl-simple/tested/           
activate    bin/        hm-version  home-files/ home-path/                                                                                    
user@mikes-workstation:~/projects/home-manager$ ls -l /nix/store/jvnyika0ymigkyn2gp8v89w9i2d78bbz-nmt-report-nixgl-simple/tested/bin/         
total 4                                                                                                                                       
lrwxrwxrwx 1 user user 76 Jan  1  1970 home-manager-generation -> /nix/store/vayk06bjilg7xznd743gj5lmrsi5byis-home-manager-generation/activate
user@mikes-workstation:~/projects/home-manager$ ls -al /nix/store/jvnyika0ymigkyn2gp8v89w9i2d78bbz-nmt-report-nixgl-simple/tested/home-files/ 
total 1620                                                                                                                                    
dr-xr-xr-x    5 user user    4096 Jan  1  1970 .                                                                                              
drwxr-xr-x 2266 user user 1638400 Apr 25 14:15 ..                                                                                             
dr-xr-xr-x    2 user user    4096 Jan  1  1970 .cache                                                                                         
dr-xr-xr-x    3 user user    4096 Jan  1  1970 .config                                                                                        
dr-xr-xr-x    2 user user    4096 Jan  1  1970 asserts                                                                                        

@michaelCTS michaelCTS marked this pull request as ready for review April 25, 2024 14:22
@michaelCTS michaelCTS changed the title WIP: init: workarounds.nixgl module init: workarounds.nixgl module Apr 25, 2024
modules/misc/nixgl.nix Outdated Show resolved Hide resolved
modules/misc/nixgl.nix Outdated Show resolved Hide resolved
modules/misc/nixgl.nix Outdated Show resolved Hide resolved
"Name of the nixGL binary to be called from within the wrapper package";
};
wrapperPkgPath = lib.mkOption {
type = lib.types.listOf lib.types.str;
Copy link
Member

Choose a reason for hiding this comment

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

If the paths are stable in nixGL then perhaps this could be enum [ "nixGLDefault" "nixGLNvidia" "nixGLNvidiaBumblebee" "nixGLIntel" ]? The module can know the corresponding attribute path.

Copy link
Member

Choose a reason for hiding this comment

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

If so then can rename the option to something like wrapperFlavor or something.

Copy link
Author

Choose a reason for hiding this comment

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

I'm not sure, honestly. There seems to also be nixGLNvidiaBumblebee and nixVulkanNividia according to the README. If we introduced an enum that would make users unable to target them.

You can download the nixGL version of your choice and point to it here.
'';
};
wrapperBinName = lib.mkOption {
Copy link
Member

Choose a reason for hiding this comment

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

Is the binary ever called something other than nixGL?

Copy link
Author

Choose a reason for hiding this comment

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

Yeah, seems like nixGLNvidia, nixGLNvidiaBumblebee, and others exist.

modules/misc/nixgl.nix Outdated Show resolved Hide resolved
modules/misc/nixgl.nix Outdated Show resolved Hide resolved
modules/misc/nixgl.nix Outdated Show resolved Hide resolved
It's expected to have a `pname` attribute.
'';
};
};
Copy link
Member

@rycee rycee Apr 27, 2024

Choose a reason for hiding this comment

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

Can just set the binary option directly in the submodule:

Suggested change
};
};
config = {
binary = mkOptionDefault (config.package.meta.mainProgram or getName config.package);
};

Can then remove allowing null values.

Copy link
Author

Choose a reason for hiding this comment

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

I'm not able to get this to work. It says attribute "package" missing. I tried binary = mkOptionDefault (config: (config.package.meta.mainProgram or getName config.package)); hoping the function would be evaluated, but it doesn't.

modules/lib/maintainers.nix Outdated Show resolved Hide resolved
@rycee
Copy link
Member

rycee commented Apr 27, 2024

Thanks for the contribution! I've added a bunch of, hopefully, useful comments. Note, to actually include the module you need to add it to the modules/modules.nix file.

@exzombie
Copy link
Contributor

I'm really happy that you found the time and will to make a pull request! Many of us have built our own hacks and workarounds, and will appreciate having a solution as part of HM.

I haven't had the time to try this yet, but just looking at the interface, I'm a bit surprised that you chose to implement a list of packages to enable nixgl for. Having followed the various discussions, I got the impression that most of us went with wrapper functions. Is there a particular reason why you chose to go with a list of packages instead?

One reason why a wrapper is more convenient is that you can wrap a package regardless of where it's used, be it in home.packages or in programs.whatever.package. As noted by @joefiorini in #3968, not being able to use nixgl in programs.kitty.package is a real problem.

One more question: does this module require building the home configuration with --impure?

@Smona Smona mentioned this pull request Apr 30, 2024
6 tasks
The option allows packages to be wrapped by nixgl in order to make GUI
applications work when installed with home-manager

It also adds a wrapper to individually wrap packages
@michaelCTS michaelCTS force-pushed the feat/add-nixgl-workaround branch from b746b29 to de92ec7 Compare April 30, 2024 11:07
@michaelCTS
Copy link
Author

@rycee I addressed your comments and applied the changes.

Additionally, I added the wrapper to config.lib.nixGl.wrap like @Smona. That should resolves your problem @joefiorini

programs.kitty = {                                          
 enable = true;                                            
 package = config.lib.nixGl.wrap { package = pkgs.kitty; };
};                                                          

One more question: does this module require building the home configuration with --impure?

@exzombie I have no idea what that does. It works without that for me.

@michaelCTS
Copy link
Author

Closed in favor of #5355

@michaelCTS michaelCTS closed this May 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

feature request: nixGL integration
3 participants