https://bugzilla.redhat.com/show_bug.cgi?id=1858531
Bug ID: 1858531 Summary: Review Request: partio- Library for reading/writing/manipulating common animation particle Product: Fedora Version: rawhide Hardware: All OS: Linux Status: NEW Component: Package Review Severity: medium Priority: medium Assignee: nobody@fedoraproject.org Reporter: luya_tfz@thefinalzone.net QA Contact: extras-qa@fedoraproject.org CC: package-review@lists.fedoraproject.org Target Milestone: --- Classification: Fedora
Spec URL: https://download.copr.fedorainfracloud.org/results/luya/openshadinglanguage/... SRPM URL: https://download.copr.fedorainfracloud.org/results/luya/openshadinglanguage/... Description: C++ (with python bindings) library for easily reading/writing/manipulating common animation particle formats such as PDB, BGEO, PTC. Fedora Account System Username: luya
https://bugzilla.redhat.com/show_bug.cgi?id=1858531
Luya Tshimbalanga luya_tfz@thefinalzone.net changed:
What |Removed |Added ---------------------------------------------------------------------------- Blocks| |1856589 Doc Type|--- |If docs needed, set a value
Referenced Bugs:
https://bugzilla.redhat.com/show_bug.cgi?id=1856589 [Bug 1856589] Review Request: openshadinglanguage - Advanced shading language for production GI renderers
https://bugzilla.redhat.com/show_bug.cgi?id=1858531
--- Comment #2 from Robert-André Mauchin 🐧 zebob.m@gmail.com --- Sorry I shouldn't have checked "do not send mail". Please see the comment above.
https://bugzilla.redhat.com/show_bug.cgi?id=1858531
--- Comment #3 from Robert-André Mauchin 🐧 zebob.m@gmail.com --- Also the tests contain arch-dependent binaries in /usr/share, which is not good. /usr/share is for arch independent code only:
partio-test.x86_64: E: arch-dependent-file-in-usr-share /usr/share/partio/test/makecircle partio-test.x86_64: E: arch-dependent-file-in-usr-share /usr/share/partio/test/makeline partio-test.x86_64: E: arch-dependent-file-in-usr-share /usr/share/partio/test/testcache partio-test.x86_64: E: arch-dependent-file-in-usr-share /usr/share/partio/test/testclonecopy partio-test.x86_64: E: arch-dependent-file-in-usr-share /usr/share/partio/test/testcluster partio-test.x86_64: E: arch-dependent-file-in-usr-share /usr/share/partio/test/testio partio-test.x86_64: E: arch-dependent-file-in-usr-share /usr/share/partio/test/testiterator partio-test.x86_64: E: arch-dependent-file-in-usr-share /usr/share/partio/test/testkdtree partio-test.x86_64: E: arch-dependent-file-in-usr-share /usr/share/partio/test/testmerge partio-test.x86_64: E: arch-dependent-file-in-usr-share /usr/share/partio/test/teststr
https://bugzilla.redhat.com/show_bug.cgi?id=1858531
--- Comment #4 from Luya Tshimbalanga luya_tfz@thefinalzone.net --- (In reply to Robert-André Mauchin 🐧 from comment #1)
- Please fix the Source0:
Source0: https://github.com/wdas/%%7Bname%7D/archive/v%%7Bversion%7D/%%7Bname%7D-%%7B...
(archives → archive)
Fixed.
- That should be in the Python subpackage:
%{python3_sitearch}/_%{name}.so
Fixed,
(Probably something like that:
set_target_properties(partio PROPERTIES OUTPUT_NAME partio POSITION_INDEPENDENT_CODE ON VERSION ${VERSION} SOVERSION 1 )
in src/lib/CMakeLists.txt)
After applying a patch using the above code, the build managed to build as seen on the scratch build: https://koji.fedoraproject.org/koji/taskinfo?taskID=47414693
I filed a ticket to upstream about the versioning libraries: https://github.com/wdas/partio/issues/82
- You should not provide %license LICENSE for all packages, but for all
packages combination. For ex, -devel depends on the main package which already provide the license, so it shouldn't be included another time in the -devel subpackage. Same with -doc subpackage.
Fixed.
- Is it useful to package the tests? Are they used by the end-user?
Those tests aren't need so they are removed.
- Add Version-Release to your changelog entry.
Done.
Here is the updated files: SPEC: https://download.copr.fedorainfracloud.org/results/luya/openshadinglanguage/... SRPM: https://download.copr.fedorainfracloud.org/results/luya/openshadinglanguage/...
https://bugzilla.redhat.com/show_bug.cgi?id=1858531
--- Comment #5 from Robert-André Mauchin 🐧 zebob.m@gmail.com --- Yeah I have not tested that patch at all. I suspect it is because the $VERSION variable is not available at this point on the build. I'm not an expert of Cmake, so I don't really know how to handle this.
https://bugzilla.redhat.com/show_bug.cgi?id=1858531
--- Comment #6 from Robert-André Mauchin 🐧 zebob.m@gmail.com --- Fixed it.
diff -ru partio-1.10.1.orig/src/lib/CMakeLists.txt partio-1.10.1/src/lib/CMakeLists.txt --- partio-1.10.1.orig/src/lib/CMakeLists.txt 2019-07-10 19:11:02.000000000 -0700 +++ partio-1.10.1/src/lib/CMakeLists.txt 2020-07-18 20:50:13.713384480 -0700 @@ -35,7 +35,11 @@ file(GLOB core_cpp "core/*.cpp")
add_library(partio ${io_cpp} ${core_cpp}) -set_target_properties(partio PROPERTIES OUTPUT_NAME partio POSITION_INDEPENDENT_CODE ON) +set_target_properties(partio PROPERTIES + OUTPUT_NAME partio POSITION_INDEPENDENT_CODE ON + VERSION ${CMAKE_PROJECT_VERSION} + SOVERSION 1 +)
target_include_directories(partio PUBLIC
================================================================
No the other issue I haven't mentioned yet is where to put the library we created. I think it should ship separately into a libpartio subpackage and then you change your devel package into libpartio-devel and make libpartio-devel requires libpartio instead of the main package. You might need to adjust the Summary/Description to reflect what's actually packaged (i.e. main package containing the tools and libpartio containing the lib)
%files -n libpartio %license LICENSE %{_libdir}/*.so.1*
- Also remove the tabs introduced in line 12, line 55, line 58
- Do not use ExcludeArch but ExclusiveArch with the arches you support.
-
https://bugzilla.redhat.com/show_bug.cgi?id=1858531
--- Comment #7 from Luya Tshimbalanga luya_tfz@thefinalzone.net --- Thank you for the fix. The build is successful this time. I figured out some build requirements lack pkgconfig files on Fedora 31 when testing the scratch build so I adjusted the condition.
(In reply to Robert-André Mauchin 🐧 from comment #6)
libpartio-devel requires libpartio instead of the main package. You might need to adjust the Summary/Description to reflect what's actually packaged (i.e. main package containing the tools and libpartio containing the lib)
%files -n libpartio %license LICENSE %{_libdir}/*.so.1*
Is "%files lib" an equivalent partio-devel requires partio-lib? Some packages used that method.
- Also remove the tabs introduced in line 12, line 55, line 58
Fixed
- Do not use ExcludeArch but ExclusiveArch with the arches you support.
I find out it is unneeded.
Here is the updated files:
SPEC: https://download.copr.fedorainfracloud.org/results/luya/openshadinglanguage/... SRPM: https://download.copr.fedorainfracloud.org/results/luya/openshadinglanguage/...
https://bugzilla.redhat.com/show_bug.cgi?id=1858531
Robert-André Mauchin 🐧 zebob.m@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |POST Assignee|nobody@fedoraproject.org |zebob.m@gmail.com Flags| |fedora-review+
--- Comment #8 from Robert-André Mauchin 🐧 zebob.m@gmail.com ---
Is "%files lib" an equivalent partio-devel requires partio-lib? Some packages used that method.
Yes that is a solution, I'm not sure what is the standard in Fedora.
Package approved.
https://bugzilla.redhat.com/show_bug.cgi?id=1858531
--- Comment #9 from Luya Tshimbalanga luya_tfz@thefinalzone.net --- Thank you Robert-André!!
https://bugzilla.redhat.com/show_bug.cgi?id=1858531
--- Comment #10 from Gwyn Ciesla gwync@protonmail.com --- (fedscm-admin): The Pagure repository was created at https://src.fedoraproject.org/rpms/partio
https://bugzilla.redhat.com/show_bug.cgi?id=1858531
Luya Tshimbalanga luya_tfz@thefinalzone.net changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|POST |CLOSED Resolution|--- |CURRENTRELEASE Last Closed| |2020-09-06 05:43:14
--- Comment #11 from Luya Tshimbalanga luya_tfz@thefinalzone.net --- Closing as the package is available in the repository.
package-review@lists.fedoraproject.org