Spec URL: https://sergiomb.fedorapeople.org/smplayer/smplayer.spec SRPM URL: https://sergiomb.fedorapeople.org/smplayer/smplayer-23.12.0-1.fc40.src.rpm Description: SMPlayer is a graphical user interface (GUI) for the award-winning mplayer and also for mpv. But apart from providing access for the most common and useful options of mplayer and mpv, SMPlayer adds other interesting features like the possibility to play Youtube videos or search and download subtitles. One of the main features is the ability to remember the state of a played file, so when you play it later it will be resumed at the same point and with the same settings. SMPlayer is developed with the Qt toolkit, so it's multi-platform. Fedora Account System Username: sergiomb
This package built on koji: https://koji.fedoraproject.org/koji/taskinfo?taskID=111330160
Copr build: https://copr.fedorainfracloud.org/coprs/build/6866211 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2256940-smplayer/fedora-rawhide-x86_64/06866211-smplayer/fedora-review/review.txt Found issues: - Not a valid SPDX expression 'GPLv2+'. It seems that you are using the old Fedora license abbreviations. Try `license-fedora2spdx' for converting it to SPDX. Read more: https://fedoraproject.org/wiki/Changes/SPDX_Licenses_Phase_1 Please know that there can be false-positives. --- This comment was created by the fedora-review-service https://github.com/FrostyX/fedora-review-service If you want to trigger a new Copr build, add a comment containing new Spec and SRPM URLs or [fedora-review-service-build] string.
Spec URL: https://sergiomb.fedorapeople.org/smplayer/smplayer.spec SRPM URL: https://sergiomb.fedorapeople.org/smplayer/smplayer-23.12.0-2.fc40.src.rpm Migrated to SPDX license
Created attachment 2007370 [details] The .spec file difference from Copr build 6866211 to 6866645
Copr build: https://copr.fedorainfracloud.org/coprs/build/6866645 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2256940-smplayer/fedora-rawhide-x86_64/06866645-smplayer/fedora-review/review.txt Please take a look if any issues were found. --- This comment was created by the fedora-review-service https://github.com/FrostyX/fedora-review-service If you want to trigger a new Copr build, add a comment containing new Spec and SRPM URLs or [fedora-review-service-build] string.
Spec URL: https://sergiomb.fedorapeople.org/smplayer/smplayer.spec SRPM URL: https://sergiomb.fedorapeople.org/smplayer/smplayer-23.12.0-2.fc40.src.rpm Change the themes subpackage to noarch
Created attachment 2007379 [details] The .spec file difference from Copr build 6866645 to 6866979
Copr build: https://copr.fedorainfracloud.org/coprs/build/6866979 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2256940-smplayer/fedora-rawhide-x86_64/06866979-smplayer/fedora-review/review.txt Please take a look if any issues were found. --- This comment was created by the fedora-review-service https://github.com/FrostyX/fedora-review-service If you want to trigger a new Copr build, add a comment containing new Spec and SRPM URLs or [fedora-review-service-build] string.
Taking this review.
Initial spec review: > %if 0%{?rhel} >= 9 > %bcond_with system_qtsingleapplication > %else > %bcond_without system_qtsingleapplication > %endif QtSingleApplication is in EPEL 9, so this conditional must be dropped. > # smplayer without mplayer is quite useless > %if 0%{?fedora} || 0%{?rhel} > 7 > Recommends: smtube > Requires: mplayer-backend > Suggests: mpv > %else > Requires: mplayer > %endif This conditional should be dropped since the fedora || rhel > 7 conditional is always true in Fedora. > %if 0%{?fedora} > # we only have yt-dlp on fedora > # it is used by the playlist > Requires: yt-dlp > %endif yt-dlp is present in EPEL 9. > %{?kf5_kinit_requires} Why is this here? This doesn't depend on KF5 from what I can see. > Requires(post): desktop-file-utils > Requires(postun): desktop-file-utils > [...] > %if (0%{?rhel} && 0%{?rhel} <= 7) > %post > /usr/bin/update-desktop-database &> /dev/null || : > /bin/touch --no-create %{_datadir}/icons/hicolor &>/dev/null || : > > %postun > /usr/bin/update-desktop-database &> /dev/null || : > if [ $1 -eq 0 ] ; then > /bin/touch --no-create %{_datadir}/icons/hicolor &>/dev/null > /usr/bin/gtk-update-icon-cache %{_datadir}/icons/hicolor &>/dev/null || : > fi > > %posttrans > /usr/bin/gtk-update-icon-cache %{_datadir}/icons/hicolor &>/dev/null || : > %endif This package is only valid in EPEL 9 and Fedora, which doesn't need any of this. Drop it. > Requires: smplayer This should be "Requires: %{name} = %{version}-%{release}". > %setup -q -a3 -a4 > [...] > %patch -P0 -p1 -b .desktop-files > %patch -P1 -p1 -b .qtsingleapplication This should be swapped for "%autosetup -p1 -a3 -a4". > %if 0%{?fedora} > sed -i 's/DEFINES += YT_CODEDOWNLOADER/DEFINES -= YT_CODEDOWNLOADER/' smplayer.pro > %endif This works on EPEL 9 too. Drop the conditional. > %if (0%{?rhel} && 0%{?rhel} <= 7) > mkdir -p %{buildroot}/usr/share/appdata/ > mv %{buildroot}/usr/share/metainfo/%{name}.appdata.xml %{buildroot}%{_metainfodir}/%{name}.appdata.xml > %endif We're not building for RHEL 7 here, drop it. > #https://bugzilla.redhat.com/show_bug.cgi?id=1830923 > %if (0%{?rhel} == 0) > appstream-util validate-relax --nonet %{buildroot}%{_metainfodir}/%{name}.appdata.xml > %endif This bug is not valid on RHEL 9, so this conditional can be dropped and metainfo validation must be done unconditionally.
# autosetup doesn't work with 2 sources # https://github.com/rpm-software-management/rpm/issues/1204 and why not support epel 8 and epel 7 ? I'm fixing the others %{?kf5_kinit_requires} is an old hack, initially for kde 4 (https://github.com/smplayer-dev/smplayer/blob/f2270fed4dfbbe3051b7cb228f70a6cfe22dce16/smplayer.spec#L43) , I'm not sure if it is needed when is just gnome installed ,
Spec URL: https://sergiomb.fedorapeople.org/smplayer/smplayer.spec SRPM URL: https://sergiomb.fedorapeople.org/smplayer/smplayer-23.12.0-2.fc40.src.rpm enable system_qtsingleapplication and yt-dlp on epel 9 -%{?kf5_kinit_requires} -Requires: smplayer +Requires: %{name} = %{version}-%{release} -%if (0%{?rhel} == 0) +%if 0%{?fedora} || 0%{?rhel} > 8 appstream-util validate-relax --nonet %{buildroot}%{_metainfodir}/%{name}.appdata.xml I'd like keep epel 8 and epel 7 support, I'd like keep builds of smplayer on that releases.
Created attachment 2008175 [details] The .spec file difference from Copr build 6866979 to 6880716
Copr build: https://copr.fedorainfracloud.org/coprs/build/6880716 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2256940-smplayer/fedora-rawhide-x86_64/06880716-smplayer/fedora-review/review.txt Please take a look if any issues were found. --- This comment was created by the fedora-review-service https://github.com/FrostyX/fedora-review-service If you want to trigger a new Copr build, add a comment containing new Spec and SRPM URLs or [fedora-review-service-build] string.
(In reply to Sergio Basto from comment #12) > > -%if (0%{?rhel} == 0) > +%if 0%{?fedora} || 0%{?rhel} > 8 > appstream-util validate-relax --nonet > %{buildroot}%{_metainfodir}/%{name}.appdata.xml > > I'd like keep epel 8 and epel 7 support, I'd like keep builds of smplayer on > that releases. Neither of those targets are possible in Fedora, though, and it adds complexity for no benefit. Especially for EPEL 7 which is EOL in six months.
(In reply to Sergio Basto from comment #11) > # autosetup doesn't work with 2 sources > > # https://github.com/rpm-software-management/rpm/issues/1204 > You can use "%autopatch -p1" after "%setup" then.
smplayer is to add to epel9 , epel8 and epel7 , as is RPMFusion , I just saw that mpv is not available on epel 8 , but is epel 9 we can't use %autopatch because one of the patches [1] is just applied if we build with system_qtsingleapplication %if %{with system_qtsingleapplication} %patch -P1 -p1 -b .qtsingleapplication %endif
Spec URL: https://sergiomb.fedorapeople.org/smplayer/smplayer.spec SRPM URL: https://sergiomb.fedorapeople.org/smplayer/smplayer-23.12.0-4.fc40.src.rpm Removed Requires : mplayer Removed support to epel-7 and epel-8 because we don't have mpv in that repos
There seems to be some problem with the following file. SRPM URL: https://sergiomb.fedorapeople.org/smplayer/smplayer-23.12.0-4.fc40.src.rpm Fetching it results in a 404 Not Found error. Please make sure the URL is correct and publicly available. --- This comment was created by the fedora-review-service https://github.com/FrostyX/fedora-review-service If you want to trigger a new Copr build, add a comment containing new Spec and SRPM URLs or [fedora-review-service-build] string.
Spec URL: https://sergiomb.fedorapeople.org/smplayer/smplayer.spec SRPM URL: https://sergiomb.fedorapeople.org/smplayer/smplayer-23.12.0-4.fc41.src.rpm
[fedora-review-service-build]
Created attachment 2030300 [details] The .spec file difference from Copr build 6880716 to 7393742
Copr build: https://copr.fedorainfracloud.org/coprs/build/7393742 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2256940-smplayer/fedora-rawhide-x86_64/07393742-smplayer/fedora-review/review.txt Please take a look if any issues were found. --- This comment was created by the fedora-review-service https://github.com/FrostyX/fedora-review-service If you want to trigger a new Copr build, add a comment containing new Spec and SRPM URLs or [fedora-review-service-build] string.
Review notes: * Packaging complies with the guidelines * Package builds and installs * No serious issues from rpmlint * Licensing is correct and license files are correctly installed PACKAGE APPROVED.
The Pagure repository was created at https://src.fedoraproject.org/rpms/smplayer
FEDORA-2024-51f649032a (smplayer-23.12.0-4.fc40) has been submitted as an update to Fedora 40. https://bodhi.fedoraproject.org/updates/FEDORA-2024-51f649032a
FEDORA-2024-06a997a645 (smplayer-23.12.0-4.fc39) has been submitted as an update to Fedora 39. https://bodhi.fedoraproject.org/updates/FEDORA-2024-06a997a645
FEDORA-EPEL-2024-b2bf9b752c (smplayer-23.12.0-4.el9) has been submitted as an update to Fedora EPEL 9. https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2024-b2bf9b752c
FEDORA-2024-51f649032a has been pushed to the Fedora 40 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-51f649032a \*` You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2024-51f649032a See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.
FEDORA-EPEL-2024-b2bf9b752c has been pushed to the Fedora EPEL 9 testing repository. You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2024-b2bf9b752c See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.
FEDORA-2024-06a997a645 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-06a997a645 \*` You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2024-06a997a645 See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.
FEDORA-2024-51f649032a (smplayer-23.12.0-4.fc40) has been pushed to the Fedora 40 stable repository. If problem still persists, please make note of it in this bug report.
FEDORA-2024-06a997a645 (smplayer-23.12.0-4.fc39) has been pushed to the Fedora 39 stable repository. If problem still persists, please make note of it in this bug report.
FEDORA-EPEL-2024-b2bf9b752c (smplayer-23.12.0-4.el9) has been pushed to the Fedora EPEL 9 stable repository. If problem still persists, please make note of it in this bug report.