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

feature request: nixGL integration #3968

Open
michaelCTS opened this issue May 8, 2023 · 20 comments · May be fixed by #5355
Open

feature request: nixGL integration #3968

michaelCTS opened this issue May 8, 2023 · 20 comments · May be fixed by #5355
Assignees

Comments

@michaelCTS
Copy link

Description

NixOS/nixpkgs#9415 which tracks the problem of libGL in nix on non NixOS systems has been open for about 8 years now. Luckily, nixGL exists which provides a wrapper around programs that require openGL (games, gpu accelerated terminals, browsers, etc.).

It would be great if there were an option to wrap all GUI applications or user-specified applications with nixGL. I'm assuming there's a generic way and bepoke method per application. Care should be taken for the desktop entries to call with nixGL

workarounds.opengl.packages

Is given a list of package names (or maybe packages themselves) that it has to wrap

lib.wrapWithLibGL

It could be a wrapper that takes a package and spits out another which is called by libGL.

@Smona
Copy link

Smona commented Jun 19, 2023

I posted an implementation I've been using to wrap programs with nixGL, which i think could be general enough to be provided as a utility on lib, and wouldn't add a hard nixGL dependency to home manager. With a little more work (to null out the buildInputs on the wrapper derivation), it could be completely transparent, and it still allows home manager to further override packages like firefox and emacs. Hopefully that can provide some inspiration?

@teto
Copy link
Collaborator

teto commented Jun 21, 2023

a good first step would be to have nixGL in nixpkgs

@stale
Copy link

stale bot commented Sep 21, 2023

Thank you for your contribution! I marked this issue as stale due to inactivity. Please be considerate of people watching this issue and receiving notifications before commenting 'I have this issue too'. We welcome additional information that will help resolve this issue. Please read the relevant sections below before commenting.

If you are the original author of the issue

  • If this is resolved, please consider closing it so that the maintainers know not to focus on this.
  • If this might still be an issue, but you are not interested in promoting its resolution, please consider closing it while encouraging others to take over and reopen an issue if they care enough.
  • If you know how to solve the issue, please consider submitting a Pull Request that addresses this issue.

If you are not the original author of the issue

  • If you are also experiencing this issue, please add details of your situation to help with the debugging process.
  • If you know how to solve the issue, please consider submitting a Pull Request that addresses this issue.

Memorandum on closing issues

Don't be afraid to manually close an issue, even if it holds valuable information. Closed issues stay in the system for people to search, read, cross-reference, or even reopen – nothing is lost! Closing obsolete issues is an important way to help maintainers focus their time and effort.

@stale stale bot added the status: stale label Sep 21, 2023
@viraptor
Copy link

hold on a moment stale-bot!

@stale stale bot removed the status: stale label Sep 21, 2023
@izelnakri
Copy link

izelnakri commented Oct 5, 2023

I've accomplished the (nixGL packageName) behavior after utilizing the hacks of @Smona @robhanlon22 here after researching this issue & trials for about a week. Seems absolutely necessary to have this feature as a lib.nixGL function @rycee for non-nixOS systems.

@michaelCTS
Copy link
Author

Can you create a PR @izelnakri ?

@rycee
Copy link
Member

rycee commented Oct 7, 2023

Maybe something that would be good to have under the targets.genericLinux namespace?

Copy link

stale bot commented Jan 5, 2024

Thank you for your contribution! I marked this issue as stale due to inactivity. Please be considerate of people watching this issue and receiving notifications before commenting 'I have this issue too'. We welcome additional information that will help resolve this issue. Please read the relevant sections below before commenting.

If you are the original author of the issue

  • If this is resolved, please consider closing it so that the maintainers know not to focus on this.
  • If this might still be an issue, but you are not interested in promoting its resolution, please consider closing it while encouraging others to take over and reopen an issue if they care enough.
  • If you know how to solve the issue, please consider submitting a Pull Request that addresses this issue.

If you are not the original author of the issue

  • If you are also experiencing this issue, please add details of your situation to help with the debugging process.
  • If you know how to solve the issue, please consider submitting a Pull Request that addresses this issue.

Memorandum on closing issues

Don't be afraid to manually close an issue, even if it holds valuable information. Closed issues stay in the system for people to search, read, cross-reference, or even reopen – nothing is lost! Closing obsolete issues is an important way to help maintainers focus their time and effort.

@stale stale bot added the status: stale label Jan 5, 2024
@viraptor
Copy link

viraptor commented Jan 6, 2024

This still needs solving.

@stale stale bot removed the status: stale label Jan 6, 2024
@diegodorado
Copy link

would love to see this implemented in home manager itself

Copy link

stale bot commented Apr 23, 2024

Thank you for your contribution! I marked this issue as stale due to inactivity. Please be considerate of people watching this issue and receiving notifications before commenting 'I have this issue too'. We welcome additional information that will help resolve this issue. Please read the relevant sections below before commenting.

If you are the original author of the issue

  • If this is resolved, please consider closing it so that the maintainers know not to focus on this.
  • If this might still be an issue, but you are not interested in promoting its resolution, please consider closing it while encouraging others to take over and reopen an issue if they care enough.
  • If you know how to solve the issue, please consider submitting a Pull Request that addresses this issue.

If you are not the original author of the issue

  • If you are also experiencing this issue, please add details of your situation to help with the debugging process.
  • If you know how to solve the issue, please consider submitting a Pull Request that addresses this issue.

Memorandum on closing issues

Don't be afraid to manually close an issue, even if it holds valuable information. Closed issues stay in the system for people to search, read, cross-reference, or even reopen – nothing is lost! Closing obsolete issues is an important way to help maintainers focus their time and effort.

@michaelCTS
Copy link
Author

Still a valid issue

@michaelCTS
Copy link
Author

For those following, I created a PR. You can test it by amending your home.nix

{ config, pkgs, ...}:
let
  michaelCtsHm = builtins.fetchTarball "https://github.com/michaelCTS/home-manager/archive/refs/heads/feat/add-nixgl-workaround.zip";
  nixGlModule = "${michaelCtsHm}/modules/misc/nixgl.nix";
in
{
  imports = [ nixGlModule ];
  workarounds.nixgl.packages = with pkgs; [
    # A package of your choice here. alacritty is just an example
    { pkg = alacritty; }
  ];
}

See the the nixgl module source (either in the nix store or the PR itself) for the objects to pass to workaround.nixgl.packages.

With that, you should be able to open the package in your desktop environment. I tested it in openbox, but it should work with KDE, Gnome and others as well.

@joefiorini
Copy link

@michaelCTS just tried this and ran into an issue. If I have programs.kitty.enable = true then I get a collision since it's trying to add the binary from both the main kitty package and the wrapped one. But if I don't use the enable flag then I don't get home-manager's configuration abilities, right? Is it possible to have this be part of the program configuration (like programs.kitty.enableNixgl = true or something?)

@Smona Smona linked a pull request Apr 30, 2024 that will close this issue
6 tasks
@Smona
Copy link

Smona commented Apr 30, 2024

@michaelCTS thank you for creating that PR! you've inspired me to finally put together my own utilizing the wrapper I posted near the top of this issue. I hope there are no hard feelings for waiting until you posted yours 😅 I am not immune to Cunningham's Law, but ultimately I hope the implementation that gets merged is the one that works best for everyone.

@joefiorini if you want you can give #5355 a try. All you should need to do is set the nixGL.prefix option, since I've integrated kitty by default in the PR.

@michaelCTS
Copy link
Author

@Smona I don't mind. It'll be up to the maintainers to decide which solution they prefer. It's just about time this issue is resolved. Whichever solution is chosen doesn't matter to me. It can always be iterated upon later and IMO there shouldn't be a focus on perfection, otherwise people will keep talking and never do anything.

I modified my PR to make the wrapper available as well. We have different approaches of wrapping the package, so I think that will be the major difference. The one you use from stackoverflow does double the disk-space usage of the package IINM whereas what I used from the nixos wiki simply links to the existing package and writes one new bin.

@Smona
Copy link

Smona commented May 1, 2024

@michaelCTS (see edit below) great point about disk usage. I had misremembered it using symlinks, but that's certainly a downside.

I only have strong opinions about the API for HM users (which is harder to iterate on), not the specific wrapper used. I originally tried using runCommand/makeWrapper, but I found that the result could not be integrated with HM modules because it did not include attributes from the original derivation. If your wrapper accomplishes that, such that wrapped packages can be used just like the original derivation, then it's certainly the better candidate 🙌. If not, I think the approach in my PR could be made to work with symlinks (it's unlikely I'll have time to do that myself).

Feel free to rip any other ideas from that PR, glad it could be of some help! In particular, I think these ideas are worth consideration:

  • Not adding packages to home.packages via a package list settings, but instead letting users pass wrapped packages into home.packages or program modules themselves. This way package specifications are not duplicated.
  • Wrapping all binary outputs for wrapped derivations, since there is a decent chance a package which needs GPU for one binary may need it for other binaries. This would also simplify the interface of lib.nixGl.wrap. There doesn't seem to be any way in your current PR to wrap multiple binaries in a single package.
  • Making the wrapper return the original derivation (no-op) if targets.genericLinux.nixgl.src is not set. This would allow modules to wrap their packages by default (but this one will be easy to iterate on later)

Thanks again for pushing this forward!

EDIT: I looked into it again and the wrapper in my PR does actually symlink the non-bin outputs, because -s is passed to cp, which makes it use symlinks. From man cp:

       -s, --symbolic-link
              make symbolic links instead of copying

So there is no disk usage downside to that wrapper.

@michaelCTS
Copy link
Author

Your right @Smona! I didn't see the -s. Maybe using ln -s would be better as a -s can be easily missed but ln is very commonly used for linking.

I'll close my PR in favor of yours. You'll probably be a more active maintainer. Hopefully it can be merged quickly.

Cheers

@Smona
Copy link

Smona commented May 6, 2024

@michaelCTS I agree that ln -s would be clearer, since even I got a bit turned around by the cp -rs call! I think I remember the cp -rs being important for some reason, either resetting the mode with --no-preserve mode and/or because it links differently: rather than creating links to the top-level directory like ln -s does, it actually creates the directory tree in the destination, and individually softlinks each file (which actually does introduce a very small disk usage disadvantage due to the increased number of inodes). I wish I had left a comment, because it could be totally unnecessary and just a copy/paste that I assumed to be useful.

In the interest of getting the initial implementation merged I might hold off on further refactoring on that branch if the current strategy looks good to maintainers, but changing the linking strategy is absolutely something we can iterate on in the future without breaking changes :)

@giggio
Copy link

giggio commented May 28, 2024

For anyone who wants to test #5355, I couldn't find an extensive way to do this, and I wanted to use Kitty, so this is what I did. I am using home-manager with flakes. I am new to all of this, so it might be obvious to many of the ones reading this, but it was not obvious to me. You can see the working sample at my dotfiles repo:
https://github.com/giggio/dotfiles/blob/main/config/home-manager/home.nix
https://github.com/giggio/dotfiles/blob/main/config/home-manager/flake.nix
And this is is the commit with the diffs:
giggio/dotfiles@c5e6901

I added NixGL to my input on flake.nix:

    nixGL = {
      url = "github:nix-community/nixGL/310f8e49a149e4c9ea52f1adf70cdc768ec53f8a";
      inputs.nixpkgs.follows = "nixpkgs";
    };

In home.nix:

I added @Smona's patch to my imports:

  imports = [
    (builtins.fetchurl {
      url = "https://raw.githubusercontent.com/Smona/home-manager/nixgl-compat/modules/misc/nixgl.nix";
      sha256 = "74f9fb98f22581eaca2e3c518a0a3d6198249fb1490ab4a08f33ec47827e85db";
    })
  ];

I added an alias for nixGL, like so:

let
  nixGLIntel = inputs.nixGL.packages."${pkgs.system}".nixGLIntel;
#...

I installed it by adding nixGLIntel to packages. And, in packages, kitty was changed to (config.lib.nixGL.wrap kitty).

And I added the option:

  nixGL.prefix = "${nixGLIntel}/bin/nixGLIntel";

This changed my kitty executable, which now looks like so:

$ cat `which kitty`
#!/nix/store/306znyj77fv49kwnkpxmb0j2znqpa8bj-bash-5.2p26/bin/bash
exec -a "$0" /nix/store/vbhv6ijhy65ma7z8i67dkci7ddiwzvrf-nixGLIntel/bin/nixGLIntel /nix/store/cmkk0l1qs0hph06nlp0ji1arkpxp44xs-kitty-0.34.1/bin/kitty "$@"

I hope this helps someone.

@Smona I hope your PR is merged soon so I can remove the customizations. Thanks for that!

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

Successfully merging a pull request may close this issue.