Bug 2257275 - Review Request: vulkan-volk - Required to build Vulkan SDK v1.3.272
Summary: Review Request: vulkan-volk - Required to build Vulkan SDK v1.3.272
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Dave Airlie
QA Contact: Fedora Extras Quality Assurance
URL: https://github.com/zeux/volk
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2024-01-08 14:53 UTC by José Expósito
Modified: 2024-02-13 00:09 UTC (History)
4 users (show)

Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2024-02-13 00:09:50 UTC
Type: ---
Embargoed:
airlied: fedora-review+


Attachments (Terms of Use)
The .spec file difference from Copr build 6872129 to 6872189 (319 bytes, patch)
2024-01-08 15:27 UTC, Fedora Review Service
no flags Details | Diff
The .spec file difference from Copr build 6872189 to 6945676 (543 bytes, patch)
2024-01-24 09:40 UTC, Fedora Review Service
no flags Details | Diff
The .spec file difference from Copr build 6945676 to 6950702 (1014 bytes, patch)
2024-01-25 09:58 UTC, Fedora Review Service
no flags Details | Diff

Description José Expósito 2024-01-08 14:53:23 UTC
Spec URL: https://jexposit.fedorapeople.org/new-packages/vulkan-volk/vulkan-volk.spec
SRPM URL: https://jexposit.fedorapeople.org/new-packages/vulkan-volk/vulkan-volk-1.3.268.0-1.fc39.src.rpm
Description: Meta loader for Vulkan API, required to build vulkan-tools v1.3.272
Fedora Account System Username: jexposit

Comment 1 Fedora Review Service 2024-01-08 14:58:22 UTC
Copr build:
https://copr.fedorainfracloud.org/coprs/build/6872129
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2257275-vulkan-volk/fedora-rawhide-x86_64/06872129-vulkan-volk/fedora-review/review.txt

Found issues:

- Package has .a files: vulkan-volk-devel. Does not provide -static: vulkan-volk-devel.
  Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/#packaging-static-libraries

Please know that there can be false-positives.

---
This comment was created by the fedora-review-service
https://github.com/FrostyX/fedora-review-service

If you want to trigger a new Copr build, add a comment containing new
Spec and SRPM URLs or [fedora-review-service-build] string.

Comment 2 José Expósito 2024-01-08 15:13:18 UTC
fedora-review-service-build

Comment 3 José Expósito 2024-01-08 15:21:52 UTC
[fedora-review-service-build]

Comment 4 Fedora Review Service 2024-01-08 15:27:32 UTC
Created attachment 2007799 [details]
The .spec file difference from Copr build 6872129 to 6872189

Comment 5 Fedora Review Service 2024-01-08 15:27:35 UTC
Copr build:
https://copr.fedorainfracloud.org/coprs/build/6872189
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2257275-vulkan-volk/fedora-rawhide-x86_64/06872189-vulkan-volk/fedora-review/review.txt

Please take a look if any issues were found.


---
This comment was created by the fedora-review-service
https://github.com/FrostyX/fedora-review-service

If you want to trigger a new Copr build, add a comment containing new
Spec and SRPM URLs or [fedora-review-service-build] string.

Comment 6 Dave Airlie 2024-01-11 01:24:37 UTC
Note: Directories without known owners: /usr/lib64/cmake/volk

vulkan-volk-devel.x86_64: E: static-library-without-debuginfo /usr/lib64/libvolk.a

probably should address these?

Comment 7 José Expósito 2024-01-24 09:34:47 UTC
Hi Dave,

Thanks a lot for your review and sorry for the delay looking into it.

About debug info, I learned that Fedora doesn't ship debug information for packages that only include .a libraries.
There were a proposal to change this behavior, but it wasn't accepted.

A few links about the topic: 

 - https://fedoraproject.org/wiki/Changes/StaticLibraryDebuginfo
 - https://bugzilla.redhat.com/show_bug.cgi?id=209316
 - https://lists.fedoraproject.org/archives/list/devel@lists.fedoraproject.org/thread/OFNQT4QPQB4TRHAD4FQTOHXBMORXJWQV/

And the link to the Fedora packaging guide that suggest to disable debug packages for .a files with "%global debug_package %{nil}":
https://docs.fedoraproject.org/en-US/packaging-guidelines/Debuginfo/#_useless_or_incomplete_debuginfo_packages_due_to_other_reasons

I'm not sure why `fedpkg --release f39 lint` throws this error though, but `fedpkg --release f40 lint` doesn't complain about it.
I think that ignoring this error should be fine.



About the "Directories without known owners" error, I added a "%dir" entry that should fix the issue.

Comment 8 José Expósito 2024-01-24 09:34:59 UTC
[fedora-review-service-build]

Comment 9 Fedora Review Service 2024-01-24 09:40:16 UTC
Created attachment 2010143 [details]
The .spec file difference from Copr build 6872189 to 6945676

Comment 10 Fedora Review Service 2024-01-24 09:40:18 UTC
Copr build:
https://copr.fedorainfracloud.org/coprs/build/6945676
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2257275-vulkan-volk/fedora-rawhide-x86_64/06945676-vulkan-volk/fedora-review/review.txt

Please take a look if any issues were found.


---
This comment was created by the fedora-review-service
https://github.com/FrostyX/fedora-review-service

If you want to trigger a new Copr build, add a comment containing new
Spec and SRPM URLs or [fedora-review-service-build] string.

Comment 11 Jonathan Steffan 2024-01-24 15:25:49 UTC
Please consider updating to 1.3.268.0 version for the package import so it matches the current vulkan headers and versioning format. This will ensure the package works at import and the version format matches the other packages. A subsequent update to 1.3.275.0 can be done as part of the larger update.

Please also consider updating the source definition to more closely match other vulkan packages.

The goal of these changes are to make it easier to update all related packages to match the upstream tags (seems they are all standardizing some now).

--- vulkan-volk.spec.old	2024-01-24 02:31:33.000000000 -0700
+++ vulkan-volk.spec	2024-01-24 08:18:02.067393608 -0700
@@ -1,13 +1,13 @@
 %global debug_package %{nil}
 
 Name:           vulkan-volk
-Version:        1.3.270
+Version:        1.3.268.0
 Release:        %autorelease
 Summary:        Meta loader for Vulkan API
 
 License:        MIT
 URL:            https://github.com/zeux/volk
-Source0:        %url/archive/refs/tags/%{version}.tar.gz
+Source0:        %url/archive/vulkan-sdk-%{version}.tar.gz#/Vulkan-Volk-sdk-%{version}.tar.gz
 
 BuildRequires:  gcc
 BuildRequires:  cmake3
@@ -27,7 +27,7 @@
 %{summary}
 
 %prep
-%autosetup -n volk-%{version} -p1
+%autosetup -n volk-vulkan-sdk-%{version} -p1
 
 %build
 %cmake3 -DCMAKE_BUILD_TYPE=Release \

Comment 12 José Expósito 2024-01-25 09:53:55 UTC
Thanks for your review @jonathansteffan 

I planned to upgrade the Vulkan SDK to 1.3.275.0 as soon as volk was accepted, but that's a good point. I changed its version to 1.3.268.0 and I'll upgrade with the other packages.

I applied your changes: https://jexposit.fedorapeople.org/new-packages/vulkan-volk/vulkan-volk.spec

Comment 13 José Expósito 2024-01-25 09:54:12 UTC
[fedora-review-service-build]

Comment 14 Fedora Review Service 2024-01-25 09:58:17 UTC
Created attachment 2010446 [details]
The .spec file difference from Copr build 6945676 to 6950702

Comment 15 Fedora Review Service 2024-01-25 09:58:19 UTC
Copr build:
https://copr.fedorainfracloud.org/coprs/build/6950702
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2257275-vulkan-volk/fedora-rawhide-x86_64/06950702-vulkan-volk/fedora-review/review.txt

Please take a look if any issues were found.


---
This comment was created by the fedora-review-service
https://github.com/FrostyX/fedora-review-service

If you want to trigger a new Copr build, add a comment containing new
Spec and SRPM URLs or [fedora-review-service-build] string.

Comment 16 José Expósito 2024-01-30 14:27:00 UTC
What do you think about the current spec file? Do you think it is ready to be included in Fedora?

Comment 17 Jonathan Steffan 2024-01-30 19:41:12 UTC
I can take over the review if @airlied wants, but it's currently already assigned.

Comment 18 Fedora Admin user for bugzilla script actions 2024-01-31 14:05:56 UTC
The Pagure repository was created at https://src.fedoraproject.org/rpms/vulkan-volk

Comment 19 Jonathan Steffan 2024-02-13 00:09:50 UTC
Thanks.

https://bodhi.fedoraproject.org/updates/FEDORA-2024-7466fed0e5


Note You need to log in before you can comment on or make changes to this bug.