diff mbox series

[bug#65155] gnu: mesa: Add native-search-paths.

Message ID 86jznanbrf.fsf@posteo.net
State New
Headers show
Series [bug#65155] gnu: mesa: Add native-search-paths. | expand

Commit Message

David Elsing Feb. 11, 2024, 5:36 p.m. UTC
Hello,

John Kehayias <john.kehayias@protonmail.com> writes:

> So perhaps this belongs in vulkan-loader? Although I admit I'm less sure
> for XDG_* related paths. Is there a test case or something we can see if
> this does what is intended for vulkan?

I also noticed the same problem and agree that the search path belongs
in the vulkan-loader package:

--8<---------------cut here---------------start------------->8---
--8<---------------cut here---------------end--------------->8---

I tested it with
--8<---------------cut here---------------start------------->8---
guix shell -C vulkan-tools vulkan-loader mesa --no-grafts -- vulkaninfo
--8<---------------cut here---------------end--------------->8---
which works with the patch applied. As documented in [1], vulkan-loader
still needs to be part of the profile for the search path to take effect.

Cheers,
David

[1] https://guix.gnu.org/manual/en/html_node/Search-Paths.html

Comments

John Kehayias March 6, 2024, 5:55 a.m. UTC | #1
Hi David and dan,

On Sun, Feb 11, 2024 at 05:36 PM, David Elsing wrote:

> Hello,
>
> John Kehayias <john.kehayias@protonmail.com> writes:
>
>> So perhaps this belongs in vulkan-loader? Although I admit I'm less sure
>> for XDG_* related paths. Is there a test case or something we can see if
>> this does what is intended for vulkan?
>
> I also noticed the same problem and agree that the search path belongs
> in the vulkan-loader package:
>
> diff --git a/gnu/packages/vulkan.scm b/gnu/packages/vulkan.scm
> index 285d6be7f5..98ce979652 100644
> --- a/gnu/packages/vulkan.scm
> +++ b/gnu/packages/vulkan.scm
> @@ -303,6 +303,11 @@ (define-public vulkan-loader
>             wayland))
>      (inputs
>       (list vulkan-headers libxrandr))
> +    (native-search-paths
> +     (list
> +      (search-path-specification
> +       (variable "XDG_DATA_DIRS")
> +       (files '("share")))))
>      (home-page
>       "https://github.com/KhronosGroup/Vulkan-Loader")
>      (synopsis "Khronos official ICD loader and validation layers for Vulkan")
>

Thanks for the diff, which I also tried.

>
> I tested it with
>
> guix shell -C vulkan-tools vulkan-loader mesa --no-grafts -- vulkaninfo
>
> which works with the patch applied. As documented in [1], vulkan-loader
> still needs to be part of the profile for the search path to take effect.
>
> Cheers,
> David
>
> [1] https://guix.gnu.org/manual/en/html_node/Search-Paths.html

Yes, this also works for me. It is also works on my machine when
dropping "mesa" and the "-C" without this diff; presumably because of
how my environment exists and XDG_DATA_DIRS. Probably in most desktop
setups things already work for Vulkan (or we'd have more bug reports?)
with any typical desktop/graphical packages. Still, this seems like this
should be more explicit like in the above diff for exactly the case of
things like containers or minimal testing environments.

I can apply such a change on mesa-updates soon when I make the other
mesa/vulkan/etc. updates we have pending. Feel free to submit a more
formal patch here so I can credit you as the author, though I should be
able to do the same on my own anyway.

Thanks for the diff and testing!

John
dan March 6, 2024, 6:59 a.m. UTC | #2
Hi John and David,

On 3/6/2024 1:55 PM, John Kehayias wrote:
> I can apply such a change on mesa-updates soon when I make the other
> mesa/vulkan/etc. updates we have pending. Feel free to submit a more
> formal patch here so I can credit you as the author, though I should be
> able to do the same on my own anyway.

This is actually included in my patch when trying to update vulkan-sdk 
packages: <https://issues.guix.gnu.org/69461#5>
John Kehayias March 25, 2024, 1:05 a.m. UTC | #3
Hi David and Dan,

On Wed, Mar 06, 2024 at 02:59 PM, dan wrote:

> Hi John and David,
>
> On 3/6/2024 1:55 PM, John Kehayias wrote:
>> I can apply such a change on mesa-updates soon when I make the other
>> mesa/vulkan/etc. updates we have pending. Feel free to submit a more
>> formal patch here so I can credit you as the author, though I should be
>> able to do the same on my own anyway.
>
> This is actually included in my patch when trying to update vulkan-sdk
> packages: <https://issues.guix.gnu.org/69461#5>

Thanks both! Will look to have that series on mesa-updates soon.

John
John Kehayias April 18, 2024, 4:46 a.m. UTC | #4
On Sun, Mar 24, 2024 at 09:05 PM, John Kehayias wrote:

> Hi David and Dan,
>
> On Wed, Mar 06, 2024 at 02:59 PM, dan wrote:
>
>> Hi John and David,
>>
>> On 3/6/2024 1:55 PM, John Kehayias wrote:
>>> I can apply such a change on mesa-updates soon when I make the other
>>> mesa/vulkan/etc. updates we have pending. Feel free to submit a more
>>> formal patch here so I can credit you as the author, though I should be
>>> able to do the same on my own anyway.
>>
>> This is actually included in my patch when trying to update vulkan-sdk
>> packages: <https://issues.guix.gnu.org/69461#5>
>
> Thanks both! Will look to have that series on mesa-updates soon.
>
> John

This was fixed on mesa-updates and then merged to master in
2d5736cc3e869fadd2592cc13a8d332fac63b144
diff mbox series

Patch

diff --git a/gnu/packages/vulkan.scm b/gnu/packages/vulkan.scm
index 285d6be7f5..98ce979652 100644
--- a/gnu/packages/vulkan.scm
+++ b/gnu/packages/vulkan.scm
@@ -303,6 +303,11 @@  (define-public vulkan-loader
            wayland))
     (inputs
      (list vulkan-headers libxrandr))
+    (native-search-paths
+     (list
+      (search-path-specification
+       (variable "XDG_DATA_DIRS")
+       (files '("share")))))
     (home-page
      "https://github.com/KhronosGroup/Vulkan-Loader")
     (synopsis "Khronos official ICD loader and validation layers for Vulkan")