https://bugzilla.redhat.com/show_bug.cgi?id=2251086
--- Comment #30 from Tim Flink tflink@redhat.com --- (In reply to Benson Muite from comment #25) <snip>
Comments: a) For the url field consider using either: https://github.com/ROCm/rocFFT or https://github.com/ROCm/%%7Bupstreamname%7D The repository https://github.com/ROCmSoftwarePlatform/rocFFT does not exist.
I knew that the upstream project was going to change but didn't realize it had already happen. thanks for catching that
b) The file CMakeLists.txt contains set ( VERSION_STRING "1.0.23" ), but website indicates version 1.0.25 - which is correct? Raised issue upstream https://github.com/ROCm/rocFFT/issues/453
patched until that's fixed upstream
c) Using %exclude %{_docdir}/%{name}/LICENSE.md is discouraged It is better to remove the extra installed copy of the license file after the install step like is done for client_info file and helper_binary
fixed
d) There is a possibility to build samples and benchmarks, could some of these be used as smoke tests?
I added the samples to the dev subpackage. benchmarks have been discussed in other comments
e) Can your patches for GNUInstallDirs and rpath removal be upstreamed? Having them as options may be useful elsewhere and will make maintenance easier.
I opted not to submit that one upstream as it's somewhat of a fedora-ism where not all consumers of the rocm components use GNU install dirs.
f) Is it possible to add -pie flags to remove the warnings: rocfft-test.x86_64: W: position-independent-executable-suggested /usr/bin/rocfft-test rocfft-test.x86_64: W: position-independent-executable-suggested /usr/bin/rtc_helper_crash
Done
g) Can the documentation be built? Ideally as man pages. Some additional dependencies such as python3dist(sphinx) are needed as is packaging of https://github.com/RadeonOpenCompute/rocm-docs-core. Initially may want to just package the files in docs/samples
That will require a few leaf packages in addition to rocm-docs-core and the result is mostly what is already available upstream which seems like a bit of a duplication of effort. I agree that it'd be nice to have man pages but is it a deal breaker for the review?
(In reply to Benson Muite from comment #27)
Sorry, no need to build client samples or benchmarks at present. Would suggest just use: -DFETCH_CONTENT_FULLY_DISCONNECTED=ON \ -DFETCH_CONTENT_QUIET=ON \ -DROCFFT_BUILD_OFFLINE_TUNER=ON \
According to the build log, CMake says that FETCH_CONTENT_FULLY_DISCONNECTED and FETCH_CONTENT_QUIET are unused by this project. Do you still think they need to be added?
Spec URL: https://tflink.fedorapeople.org/packages/rocfft/rocfft.spec SRPM URL: https://tflink.fedorapeople.org/packages/rocfft/rocfft-6.0.0-2.fc40.src.rpm COPR Build: https://copr.fedorainfracloud.org/coprs/tflink/rocm-packaging/build/6910671/