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

Jump to bottom

nvidia-x11: Make vulkan library path absolute for >= 435. #69820

Merged
merged 1 commit into from Oct 2, 2019
Merged

nvidia-x11: Make vulkan library path absolute for >= 435. #69820

merged 1 commit into from Oct 2, 2019

Conversation

ambrop72
Copy link
Contributor

The original file contains just a library name, which does not work when LD_LIBRARY_PATH does not contain /run/opengl-driver/lib, as is the case in unstable NixOS.

Fixes #69264

Motivation for this change

The PR #68024 did not realize that the path is supposed to be absolute, as already indicated in a comment. Due to this it still did not fix Vulkan for nixos-unstable, where LD_LIBRARY_PATH is normally not set to /run/opengl-driver/lib.

Things done

Tested that vulkaninfo now works when it did not before, both 64- and 32-bit version.

  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nix-review --run "nix-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.
Notify maintainers

cc @FRidh @abbradar

@ambrop72 ambrop72 mentioned this pull request Sep 28, 2019
Closed
@eadwu
Copy link
Member

eadwu commented Sep 28, 2019

Should be something like 031b069, since libGLX_nvidia will end up with a path with $out/lib twice.

@ambrop72
Copy link
Contributor Author

It's fine the way it is, the sed is piped to a different file, the original is unchanged. The result is correct, I've checked.

@ambrop72
Copy link
Contributor Author

I'm not sure what you pointed me to, which PR is this from?

@eadwu
Copy link
Member

eadwu commented Sep 28, 2019

#66601

@ambrop72
Copy link
Contributor Author

I'm guessing that PR also fixes this issue, though in a slightly different way. Should I refactor this PR to do the same thing? Or just merge that one?

@eadwu
Copy link
Member

eadwu commented Sep 28, 2019

It's fine whichever one, I'll rebase if needed. Though this one does look nicer.

@ambrop72
Copy link
Contributor Author

Ok then let's go with the PR as is. Actually I don't like the one you linked because the result of the first sed is also reused in the second iteration, which is non-obvious.

@ambrop72
Copy link
Contributor Author

Sorry wait I need to fix up the first case so it writes to .fixed, I didn't notice this.

@ambrop72
nvidia-x11: Make vulkan library path absolute for >= 435.
0314211 
The original file contains just a library name, which does not work when LD_LIBRARY_PATH does not contain /run/opengl-driver/lib, as is the case in unstable NixOS.

Fixes NixOS#69264
@ambrop72
Copy link
Contributor Author

Refactored such that it never writes to nvidia_icd.json but always to nvidia_icd.json.fixed which is then installed. Also added some spacing and comments.

@abbradar abbradar added the 9.needs: port to stable A PR needs a backport to the stable release. label Oct 1, 2019
@abbradar
Copy link
Member

abbradar commented Oct 2, 2019

Tested, thanks!

@abbradar abbradar merged commit d156b2b into NixOS:master Oct 2, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
9.needs: port to stable A PR needs a backport to the stable release. 10.rebuild-darwin: 0 10.rebuild-linux: 101-500
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Vulkan broken with Nvidia on unstable
3 participants
@ambrop72 @eadwu @abbradar