https://bugzilla.redhat.com/show_bug.cgi?id=1671571 --- Comment #6 from Justin W. Flory <jflory7@xxxxxxxxx> --- I pushed a new commit [1] to address the above feedback. The build completed successfully in Koji [2][3] and COPR [4]. (In reply to Dridi Boukelmoune from comment #4) > The xz archive is handled just fine, and you should file a bug for COPR if > it's not working there. We are targeting Fedora proper here so I'd rather > stick to the upstream release archive. > Odd. Not sure why the build failed before because xz was an unrecognized format. I made this change again and the package built successfully. Maybe the archive was corrupt originally. In either case, this is done now. > I also moved the static library and un-versioned shared object to the devel > packages, and added the full soname in the main package so we don't miss > soname bumps in the future. I built this with mock locally and I'm currently > using the package on my system with no regression. > > Finally, I removed the %doc indicators because those are well-known > locations in Fedora so they already get flagged as such, and I made sure the > %{_datadir}/gtk-doc/html/%{name} is owned by the package, not just its > contents. > Thanks for these tips. About the static library / shared objects, could you link me to documentation or explain why this change is helpful? I'm new to building C packages. Understanding why this change is helpful will help me build better packages later too. > When you update the spec, make sure to add a new comment with corresponding > Spec and SRPM URLs so that during the formal review fedora-review picks up > your latest update (especially since you updated the release tag, that > changes the SRPM URL). > Spec: https://pagure.io/jflory7-rpm-specs/blob/master/f/rpmbuild/SPECS/playerctl.spec SRPM: https://pagure.io/jflory7-rpm-specs/blob/master/f/rpmbuild/SRPMS/playerctl-2.0.1-3.fc29.src.rpm (In reply to Fabio Valentini from comment #5) > Just some small comments from a Packaging Committee member: > > - Please don't include static libraries, unless *absolutely necessary*. > In this case, I think it can be safely removed (probably by rm-ing it from > the buildroot at the end of %install): > rm %{buildroot}/%{_libdir}/lib%{name}.a > It seems the static library is necessary? My Koji build [5] and COPR build [6] failed without it. > - Depending on the size of the html docs, they should be moved to a > separate, (probably noarch) -doc sub-package. > > For example, fedora-review recommends to move documentation to a separate > sub-package if it's bigger than 1 MB. The HTML docs were only 500KB or so, but I went ahead and split them out into their own package anyways. -- [1] https://pagure.io/jflory7-rpm-specs/c/476d05cbafe8913a8d0af4c29bb439a3d1264af5?branch=master [2] https://koji.fedoraproject.org/koji/taskinfo?taskID=32534256 [3] https://koji.fedoraproject.org/koji/taskinfo?taskID=32534242 [4] https://copr.fedorainfracloud.org/coprs/jflory7/playerctl/build/853863/ [5] https://koji.fedoraproject.org/koji/taskinfo?taskID=32533723 [6] https://copr.fedorainfracloud.org/coprs/jflory7/playerctl/build/853848/ -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component _______________________________________________ package-review mailing list -- package-review@xxxxxxxxxxxxxxxxxxxxxxx To unsubscribe send an email to package-review-leave@xxxxxxxxxxxxxxxxxxxxxxx Fedora Code of Conduct: https://getfedora.org/code-of-conduct.html List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/package-review@xxxxxxxxxxxxxxxxxxxxxxx
