Bug 2254392 - Review Request: renderdoc - stand-alone graphics debugging tool
Summary: Review Request: renderdoc - stand-alone graphics debugging tool
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: ---
Assignee: Marc-Andre Lureau
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On: 1492746
Blocks: 2242058
TreeView+ depends on / blocked
 
Reported: 2023-12-13 18:20 UTC by kb1000
Modified: 2024-02-20 01:39 UTC (History)
6 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2024-02-20 01:37:10 UTC
Type: Bug
Embargoed:
marcandre.lureau: fedora-review+


Attachments (Terms of Use)


Links
System ID Private Priority Status Summary Last Updated
Red Hat Bugzilla 2049621 0 unspecified CLOSED renderdoc fails to build with Python 3.11: error: 'struct _frame' has no member named 'f_globals' 2023-12-13 18:23:05 UTC

Description kb1000 2023-12-13 18:20:41 UTC
Spec URL: https://download.copr.fedorainfracloud.org/results/kb1000/renderdoc/fedora-rawhide-x86_64/06750284-renderdoc/renderdoc.spec
SRPM URL: https://download.copr.fedorainfracloud.org/results/kb1000/renderdoc/fedora-rawhide-x86_64/06750284-renderdoc/renderdoc-1.30-1.fc40~kb1000.src.rpm

Description:
A free MIT licensed stand-alone graphics debugger that allows quick
and easy single-frame capture and detailed introspection of any
application using Vulkan, OpenGL.

Fedora Account System Username: kb1000

This package was already part of the Fedora repositories, but its maintainer left, leaving the package orphaned, and broken due to https://bugzilla.redhat.com/show_bug.cgi?id=2049621
All I changed from the previous package is updating it to the latest version, which suffices to fix that issue. (The spec/SRPM linked above come from my personal copr repository, I had added the ~kb1000 tag to distinguish it from upstream Fedora builds and to make sure it cleanly updates should it get added back, that tag should be removed if this gets merged back in.)
I'm new to this so I'm sorry if I did something wrong...

Comment 1 Marc-Andre Lureau 2023-12-15 10:11:06 UTC
Issues:
=======
- missing BuildRequires against gcc-c++
- No known owner of /usr/lib64/renderdoc, should be fixed with a %dir


Others:
- pcre-devel is deprecated, but probably ok (https://github.com/baldurk/renderdoc/issues/3123)
- E: binary-or-shlib-defines-rpath /usr/bin/qrenderdoc
  This should be okay according to https://bugzilla.redhat.com/show_bug.cgi?id=1492746#c13

Comment 2 Fabio Valentini 2023-12-15 10:19:20 UTC
(In reply to Marc-Andre Lureau from comment #1)
> Issues:
> =======
> - missing BuildRequires against gcc-c++
> - No known owner of /usr/lib64/renderdoc, should be fixed with a %dir
> 
> 
> Others:
> - pcre-devel is deprecated, but probably ok
> (https://github.com/baldurk/renderdoc/issues/3123)

For the record: No, this is not OK.

> other packages in Fedora MUST NOT add a dependency on a deprecated package (that includes Requires, BuildRequires, Recommends, Suggests, etc.). This applies both for updates of existing packages and new packages added to Fedora

see https://docs.fedoraproject.org/en-US/packaging-guidelines/deprecating-packages/#_consequences_of_a_package_being_deprecated

Comment 3 kb1000 2023-12-15 19:06:10 UTC
Fixing the first two issues is pretty easy, but the PCRE problem seems to be a bit harder to address. Upstream still doesn't really seem to want to change this or accept a PR, so I'd need to apply the two upstream SWIG commits to the custom SWIG build, but I'm not entirely sure how to do that properly. It seems like it's possible to just set RENDERDOC_SWIG_PACKAGE to a directory instead of a file, but that also means I need to extract both source tarballs.
What's the current best practices for packages with multiple sources and patches? I can't seem to find a lot of information about this in the packaging guidelines or RPM documentation. It doesn't look like %autosetup would help me there? Do I have to use %setup and %patch?

Comment 4 kb1000 2023-12-21 07:04:37 UTC
Spec URL: https://download.copr.fedorainfracloud.org/results/kb1000/renderdoc/fedora-rawhide-x86_64/06776770-renderdoc/renderdoc.spec
SRPM URL: https://download.copr.fedorainfracloud.org/results/kb1000/renderdoc/fedora-rawhide-x86_64/06776770-renderdoc/renderdoc-1.30-2.fc40.src.rpm

I've now added those two patches and replaced the PCRE dependency with PCRE2. I've also added the gcc-c++ BuildRequires and added that %dir.

Comment 5 Marc-Andre Lureau 2023-12-31 09:57:23 UTC
(In reply to kb1000 from comment #4)
> Spec URL:
> https://download.copr.fedorainfracloud.org/results/kb1000/renderdoc/fedora-
> rawhide-x86_64/06776770-renderdoc/renderdoc.spec
> SRPM URL:
> https://download.copr.fedorainfracloud.org/results/kb1000/renderdoc/fedora-
> rawhide-x86_64/06776770-renderdoc/renderdoc-1.30-2.fc40.src.rpm
> 
> I've now added those two patches and replaced the PCRE dependency with
> PCRE2. I've also added the gcc-c++ BuildRequires and added that %dir.

excellent! I couldn't find your patches upstream though.

Comment 6 kb1000 2024-01-01 22:53:38 UTC
I rebased them from upstream SWIG onto RenderDoc's custom SWIG fork.
RenderDoc's developer has told me (on Discord) that these patches will not be accepted into the custom SWIG fork, because this would affect all platforms instead of just Linux distributions that no longer allow using PCRE and he would, I quote, "rather find a way to build the pcre dependency if distros start dropping it". While I think that's a fair point, it really doesn't help us here, so these patches cannot be upstreamed.

Comment 7 Marc-Andre Lureau 2024-01-02 08:26:05 UTC
(In reply to kb1000 from comment #6)
> I rebased them from upstream SWIG onto RenderDoc's custom SWIG fork.
> RenderDoc's developer has told me (on Discord) that these patches will not
> be accepted into the custom SWIG fork, because this would affect all
> platforms instead of just Linux distributions that no longer allow using
> PCRE and he would, I quote, "rather find a way to build the pcre dependency
> if distros start dropping it". While I think that's a fair point, it really
> doesn't help us here, so these patches cannot be upstreamed.

Ok, please proceed with the package. have you requested package sponsorship and part of packager group?

Comment 8 kb1000 2024-01-08 23:39:20 UTC
I've created https://pagure.io/packager-sponsors/issue/615 now. It looks like it was supposed to be created by a bot after you added the fedora-review+ flag, but I guess that didn't work... I was thinking FE-NEEDSPONSOR was all you need but it seems that's not true.

Unrelated to that: Someone approached me and requested me to add aarch64 support to the COPR repository. According to the previous maintainer, RenderDoc is only officially supported on x86_64. The person wanting aarch64 support has told me it seems to work, though couldn't fully test it as the drivers weren't fully working (Open-source Nvidia and Asahi with Vulkan, so...). Should I add aarch64 to the package's ExclusiveArch anyways?

Comment 9 Daniel Berrangé 2024-01-15 11:29:55 UTC
(In reply to kb1000 from comment #8)
> Unrelated to that: Someone approached me and requested me to add aarch64
> support to the COPR repository. According to the previous maintainer,
> RenderDoc is only officially supported on x86_64. The person wanting aarch64
> support has told me it seems to work, though couldn't fully test it as the
> drivers weren't fully working (Open-source Nvidia and Asahi with Vulkan,
> so...). Should I add aarch64 to the package's ExclusiveArch anyways?

If a package builds on an architecture and is conceptually relevant, then Fedora maintainers are expected to enable it even if not officially supported by upstream.

https://docs.fedoraproject.org/en-US/packaging-guidelines/#_architecture_support

Generally I would say use 'ExcludeArch' to omit arches where it is known to have problem, rather than ExclusiveArch. ie default to enabled, rather than disabled.

Comment 10 Marc-Andre Lureau 2024-01-22 07:33:13 UTC
Daniel, that's how the original package was (from the start):
https://src.fedoraproject.org/rpms/renderdoc/blob/f7e8e34976bff31403d0aefc05f883d75bb6d2a9/f/renderdoc.spec#_13

@kaeptmblaubaer1000, it builds for aarch64, and fails on ppc/s390: https://koji.fedoraproject.org/koji/taskinfo?taskID=112157907

Can you update with 'ExcludeArch' usage? thanks

Comment 11 Dan Horák 2024-01-22 09:40:02 UTC
Looks to me that new arches like ppc64le and s390x need some porting efforts, so ExclusiveArch would be OK.

...
[ 60%] Building CXX object renderdoc/CMakeFiles/rdoc.dir/os/posix/linux/linux_process.cpp.o
cd /builddir/build/BUILD/renderdoc-1.30/redhat-linux-build/renderdoc && /usr/bin/g++ -DDISTRIBUTION_CONTACT=\"https://bugzilla.redhat.com\" -DDISTRIBUTION_NAME=\"fedora\" -DRDOC_BASE_NAME=renderdoc -DRELEASE -DRENDERDOC_EXPORTS -DRENDERDOC_LIB_SUBFOLDER=renderdoc -DRENDERDOC_LIB_SUFFIX=64 -DRENDERDOC_PLATFORM_LINUX -DRENDERDOC_STABLE_BUILD=1 -DRENDERDOC_SUPPORT_EGL -DRENDERDOC_SUPPORT_GL -DRENDERDOC_SUPPORT_GLES -DRENDERDOC_SUPPORT_VULKAN -DRENDERDOC_VULKAN_JSON_SUFFIX="" -DRENDERDOC_WINDOWING_XCB -DRENDERDOC_WINDOWING_XLIB -I/builddir/build/BUILD/renderdoc-1.30/renderdoc -I/builddir/build/BUILD/renderdoc-1.30/renderdoc/3rdparty -O2  -fexceptions -g -grecord-gcc-switches -pipe -Wall -Werror=format-security -Wp,-U_FORTIFY_SOURCE,-D_FORTIFY_SOURCE=3 -Wp,-D_GLIBCXX_ASSERTIONS -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -fstack-protector-strong -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1  -m64 -mcpu=power8 -mtune=power8 -fasynchronous-unwind-tables -fstack-clash-protection -std=c++11 -fno-strict-aliasing -fvisibility=hidden -fvisibility-inlines-hidden -Wall -Wextra -Wno-unused-variable -Wno-unused-parameter -Wno-unused-result -Wno-type-limits -Wno-missing-field-initializers -Wno-unknown-pragmas -Wno-reorder -Wno-unused-but-set-variable -Wno-maybe-uninitialized -Wno-class-memaccess -Wimplicit-fallthrough=2 -Wno-unused-value -DNDEBUG -fPIC -MD -MT renderdoc/CMakeFiles/rdoc.dir/os/posix/linux/linux_process.cpp.o -MF CMakeFiles/rdoc.dir/os/posix/linux/linux_process.cpp.o.d -o CMakeFiles/rdoc.dir/os/posix/linux/linux_process.cpp.o -c /builddir/build/BUILD/renderdoc-1.30/renderdoc/os/posix/linux/linux_process.cpp
/builddir/build/BUILD/renderdoc-1.30/renderdoc/os/posix/linux/linux_process.cpp: In function ‘uint64_t get_child_ip(pid_t)’:
/builddir/build/BUILD/renderdoc-1.30/renderdoc/os/posix/linux/linux_process.cpp:256:3: error: ‘user_regs_struct’ was not declared in this scope
  256 |   user_regs_struct regs = {};
      |   ^~~~~~~~~~~~~~~~
/builddir/build/BUILD/renderdoc-1.30/renderdoc/os/posix/linux/linux_process.cpp:258:28: error: expected primary-expression before ‘,’ token
  258 |   iovec regs_iovec = {&regs, sizeof(regs)};
      |                            ^
/builddir/build/BUILD/renderdoc-1.30/renderdoc/os/posix/linux/linux_process.cpp:258:30: error: invalid application of ‘sizeof’ to incomplete type ‘regs’
  258 |   iovec regs_iovec = {&regs, sizeof(regs)};
...

Comment 12 kb1000 2024-01-23 07:25:53 UTC
SRPM URL: https://download.copr.fedorainfracloud.org/results/kb1000/renderdoc/fedora-rawhide-x86_64/06942494-renderdoc/renderdoc-1.30-3.fc40.src.rpm
Spec URL: https://download.copr.fedorainfracloud.org/results/kb1000/renderdoc/fedora-rawhide-x86_64/06942494-renderdoc/renderdoc.spec

I've added aarch64 now... after running into https://github.com/fedora-copr/copr/issues/3114 where COPR would, seemingly incorrectly, ignore the second ExclusiveArch line if I have more than one.
Also, seems like I was lucky that I could login to COPR before FAS broke :)

Comment 13 Kevin Fenzi 2024-02-01 00:58:21 UTC
I've sponsored you now. Please continue the process and let me know if you have any questions! Welcome.

Comment 14 kb1000 2024-02-02 21:39:44 UTC
Thanks a lot! As far as I understood, getting the package unblocked/unretired would be the next step, so I've opened https://pagure.io/releng/issue/11928 now.

Comment 15 Fedora Update System 2024-02-11 01:32:10 UTC
FEDORA-2024-357a61cf8b (renderdoc-1.31-1.fc39) has been submitted as an update to Fedora 39.
https://bodhi.fedoraproject.org/updates/FEDORA-2024-357a61cf8b

Comment 16 Fedora Update System 2024-02-11 01:32:14 UTC
FEDORA-2024-5eb940d906 (renderdoc-1.31-1.fc38) has been submitted as an update to Fedora 38.
https://bodhi.fedoraproject.org/updates/FEDORA-2024-5eb940d906

Comment 17 Fedora Update System 2024-02-12 01:51:59 UTC
FEDORA-2024-357a61cf8b has been pushed to the Fedora 39 testing repository.
Soon you'll be able to install the update with the following command:
`sudo dnf install --enablerepo=updates-testing --refresh --advisory=FEDORA-2024-357a61cf8b \*`
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2024-357a61cf8b

See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.

Comment 18 Fedora Update System 2024-02-12 02:42:32 UTC
FEDORA-2024-5eb940d906 has been pushed to the Fedora 38 testing repository.
Soon you'll be able to install the update with the following command:
`sudo dnf install --enablerepo=updates-testing --refresh --advisory=FEDORA-2024-5eb940d906 \*`
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2024-5eb940d906

See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.

Comment 19 Fedora Update System 2024-02-20 01:37:10 UTC
FEDORA-2024-5eb940d906 (renderdoc-1.31-1.fc38) has been pushed to the Fedora 38 stable repository.
If problem still persists, please make note of it in this bug report.

Comment 20 Fedora Update System 2024-02-20 01:39:09 UTC
FEDORA-2024-357a61cf8b (renderdoc-1.31-1.fc39) has been pushed to the Fedora 39 stable repository.
If problem still persists, please make note of it in this bug report.


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