https://bugzilla.redhat.com/show_bug.cgi?id=2124244
Bug ID: 2124244 Summary: Review Request: mpv - Movie player playing most video formats and DVDs Product: Fedora Version: rawhide Hardware: All OS: Linux Status: NEW Component: Package Review Severity: medium Priority: medium Assignee: nobody@fedoraproject.org Reporter: vitaly@easycoding.org QA Contact: extras-qa@fedoraproject.org CC: package-review@lists.fedoraproject.org Target Milestone: --- Classification: Fedora
Spec URL: https://xvitaly.fedorapeople.org/for-review/mpv.spec SRPM URL: https://xvitaly.fedorapeople.org/for-review/mpv-0.34.1-10.fc36.src.rpm Description: Movie player playing most video formats and DVDs Fedora Account System Username: xvitaly
Moving this package from the RPM Fusion repository. I'm its maintainer in RPM Fusion.
https://bugzilla.redhat.com/show_bug.cgi?id=2124244
Vitaly Zaitsev vitaly@easycoding.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Doc Type|--- |If docs needed, set a value
--- Comment #1 from Vitaly Zaitsev vitaly@easycoding.org --- Fixed rpmlint warnings.
Spec URL: https://xvitaly.fedorapeople.org/for-review/mpv.spec SRPM URL: https://xvitaly.fedorapeople.org/for-review/mpv-0.34.1-11.fc36.src.rpm
https://bugzilla.redhat.com/show_bug.cgi?id=2124244
Neal Gompa ngompa13@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags| |fedora-review? CC| |ngompa13@gmail.com Assignee|nobody@fedoraproject.org |ngompa13@gmail.com
--- Comment #2 from Neal Gompa ngompa13@gmail.com --- Taking this review.
https://bugzilla.redhat.com/show_bug.cgi?id=2124244
Neal Gompa ngompa13@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED
https://bugzilla.redhat.com/show_bug.cgi?id=2124244
--- Comment #3 from Neal Gompa ngompa13@gmail.com --- Initial spec review:
%if 0%{?fedora} BuildRequires: libshaderc-devel BuildRequires: pkgconfig(vulkan) BuildRequires: pkgconfig(libplacebo) %else %ifarch x86_64 BuildRequires: pkgconfig(vulkan) %endif %endif
All this conditional logic can be removed. The dependencies listed in the Fedora case also exist in EPEL 9, and Vulkan is not locked to x86_64 on RHEL 9.
%if 0%{?fedora} Recommends: (yt-dlp or youtube-dl) %else Recommends: youtube-dl %endif
Rich dependencies work in all versions of RHEL that support weak dependencies, so you can rip this conditional out and use just the Fedora one for everything.
%package libs-devel
The "libs-devel" name is... odd. I've not seen very many examples of this. Consider renaming it to "mpv-devel" or adding Provides to make it fit common conventions.
%files %docdir %{_docdir}/%{name}/ %{_docdir}/%{name}/
Wouldn't "%doc %{_docdir}/%{name}/" do the same thing here?
%files libs %{_libdir}/lib%{name}.so.*
The mpv library soname needs to be tracked.
Cf. https://docs.fedoraproject.org/en-US/packaging-guidelines/#_shared_libraries
https://bugzilla.redhat.com/show_bug.cgi?id=2124244
--- Comment #4 from Vitaly Zaitsev vitaly@easycoding.org ---
All this conditional logic can be removed. The dependencies listed in the Fedora case also exist in EPEL 9, and Vulkan is not locked to x86_64 on RHEL 9.
Removed.
Rich dependencies work in all versions of RHEL that support weak dependencies, so you can rip this conditional out and use just the Fedora one for everything.
Fixed.
The "libs-devel" name is... odd. I've not seen very many examples of this. Consider renaming it to "mpv-devel" or adding Provides to make it fit common conventions.
Fixed.
Wouldn't "%doc %{_docdir}/%{name}/" do the same thing here?
No. %docdir qualifier is required to mark all files from %{_docdir}/%{name}/ as documentation on RPM level.
The mpv library soname needs to be tracked.
Done.
https://bugzilla.redhat.com/show_bug.cgi?id=2124244
--- Comment #5 from Vitaly Zaitsev vitaly@easycoding.org --- Spec URL: https://xvitaly.fedorapeople.org/for-review/mpv.spec SRPM URL: https://xvitaly.fedorapeople.org/for-review/mpv-0.34.1-11.fc36.src.rpm
https://bugzilla.redhat.com/show_bug.cgi?id=2124244
Neal Gompa ngompa13@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |POST Flags|fedora-review? |fedora-review+
--- Comment #6 from Neal Gompa ngompa13@gmail.com --- Package review notes:
* Package is named properly per packaging guidelines * Package builds and installs * Package licensing is correct and license files are correctly installed * No serious issues from rpmlint
PACKAGE APPROVED.
https://bugzilla.redhat.com/show_bug.cgi?id=2124244
--- Comment #7 from Gwyn Ciesla gwync@protonmail.com --- (fedscm-admin): The Pagure repository was created at https://src.fedoraproject.org/rpms/mpv
https://bugzilla.redhat.com/show_bug.cgi?id=2124244
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|POST |MODIFIED
--- Comment #8 from Fedora Update System updates@fedoraproject.org --- FEDORA-2022-2cd1e85046 has been submitted as an update to Fedora 37. https://bodhi.fedoraproject.org/updates/FEDORA-2022-2cd1e85046
https://bugzilla.redhat.com/show_bug.cgi?id=2124244
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|MODIFIED |ON_QA
--- Comment #9 from Fedora Update System updates@fedoraproject.org --- FEDORA-2022-2cd1e85046 has been pushed to the Fedora 37 testing repository. Soon you'll be able to install the update with the following command: `sudo dnf install --enablerepo=updates-testing --refresh --advisory=FEDORA-2022-2cd1e85046 *` You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2022-2cd1e85046
See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.
https://bugzilla.redhat.com/show_bug.cgi?id=2124244
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Resolution|--- |ERRATA Status|ON_QA |CLOSED Last Closed| |2022-09-14 00:20:15
--- Comment #10 from Fedora Update System updates@fedoraproject.org --- FEDORA-2022-2cd1e85046 has been pushed to the Fedora 37 stable repository. If problem still persists, please make note of it in this bug report.
package-review@lists.fedoraproject.org