https://bugzilla.redhat.com/show_bug.cgi?id=2256940



--- Comment #10 from Neal Gompa <[email protected]> ---
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.


-- 
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
https://bugzilla.redhat.com/show_bug.cgi?id=2256940

Report this comment as SPAM: 
https://bugzilla.redhat.com/enter_bug.cgi?product=Bugzilla&format=report-spam&short_desc=Report%20of%20Bug%202256940%23c10
--
_______________________________________________
package-review mailing list -- [email protected]
To unsubscribe send an email to [email protected]
Fedora Code of Conduct: 
https://docs.fedoraproject.org/en-US/project/code-of-conduct/
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives: 
https://lists.fedoraproject.org/archives/list/[email protected]
Do not reply to spam, report it: 
https://pagure.io/fedora-infrastructure/new_issue

Reply via email to