Bug 2254392

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

(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

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?

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.

(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.

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.

(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?

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?

(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.

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

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)};
...

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 :)

I've sponsored you now. Please continue the process and let me know if you have any questions! Welcome.

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.

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

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

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.

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.

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.

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.