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.

Sign up for GitHub

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

libglvnd, ocl-icd, vulkan-loader: Remove addOpenGLRunpath where not needed. #64945

Merged
merged 1 commit into from Aug 18, 2019
Merged

libglvnd, ocl-icd, vulkan-loader: Remove addOpenGLRunpath where not needed. #64945

merged 1 commit into from Aug 18, 2019

Conversation

ambrop72
Copy link
Contributor

For APIs that use driver config files, the config files should point to libraries using absolute paths, so RUNPATH should be unnecessary. It is still needed for libGLX because that does not use config files but the dispatch library receives just a vendor name from the X server.

Motivation for this change

Remove unneeded RUNPATH from library. See the discussion #60985 (comment). This is is possible for NVidia after #62870 and #62859 were merged; open-source drivers already use absolute paths.

Things done

Tested with drivers:

  • nvidia, nvidiaLegacy390: glxinfo, es2_info, vulkaninfo and opencl-info still work, including 32-bit programs on 64-bit system.
  • amdgpu: glxinfo, es2_info and vulkaninfo still work, including 32-bit programs on 64-bit system (OpenCL does not work anyway).
  • 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.

@ambrop72
libglvnd, ocl-icd, vulkan-loader: Remove addOpenGLRunpath where not n…
9831942 
…eeded.

For APIs that use driver config files, the config files should point to libraries using absolute paths, so `RUNPATH` should be unnecessary. It is still needed for `libGLX` because that does not use config files but the dispatch library receives just a vendor name from the X server.
@FRidh FRidh added this to the 19.09 milestone Jul 17, 2019
@gloaming gloaming mentioned this pull request Jul 18, 2019
Closed
10 tasks
@FRidh
Copy link
Member

FRidh commented Aug 3, 2019

I would like this to be reviewed. @ambrop72 I suggest checking who maintains this bit of code and cc them.

@ambrop72
Copy link
Contributor Author

ambrop72 commented Aug 3, 2019

@abbradar @Ralith Could you review this?

Ralith
Ralith approved these changes Aug 3, 2019
View reviewed changes
Copy link
Contributor

@Ralith Ralith left a comment

Choose a reason for hiding this comment

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

Seems sane to me. Also, you were the person who put this fragment here in the first place, so I expect you know what you're doing!

@abbradar abbradar changed the base branch from master to staging August 9, 2019 09:34
@abbradar
Copy link
Member

abbradar commented Aug 9, 2019

Looks good. I can't run a test build right now. A simple sanity check like "does glxgears run on an Intel system" should suffice; maybe I can run one on a weekend.

@abbradar
Copy link
Member

Sorry for the delay, I tested it for OpenCL and Vulkan on an NVIDIA machine. Looks good.

@abbradar abbradar merged commit 1bba961 into NixOS:staging Aug 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@matthewbauer matthewbauer matthewbauer approved these changes

@Ralith Ralith Ralith approved these changes

Assignees
No one assigned
Labels
10.rebuild-darwin: 501+ 10.rebuild-darwin: 1001-2500 10.rebuild-linux: 501+ 10.rebuild-linux: 5001+
Projects
None yet
Milestone
19.09
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@ambrop72 @FRidh @abbradar @matthewbauer @Ralith