[PR PATCH] smplayer: Add mpv as runtime dependency

[PR PATCH] smplayer: Add mpv as runtime dependency

[TrueTechie]

[-- Attachment #1: Type: text/plain, Size: 1124 bytes --]

There is a new pull request by TrueTechie against master on the void-packages repository

https://github.com/TrueTechie/void-packages smplayer
https://github.com/void-linux/void-packages/pull/40457

smplayer: Add mpv as runtime dependency
<!-- Uncomment relevant sections and delete options which are not applicable -->

#### Testing the changes
- I tested the changes in this PR: **briefly**

<!--
#### New package
- This new package conforms to the [package requirements](https://github.com/void-linux/void-packages/blob/master/CONTRIBUTING.md#package-requirements): **YES**|**NO**
-->

<!-- Note: If the build is likely to take more than 2 hours, please add ci skip tag as described in
https://github.com/void-linux/void-packages/blob/master/CONTRIBUTING.md#continuous-integration
and test at least one native build and, if supported, at least one cross build.
Ignore this section if this PR is not skipping CI.
-->

#### Local build testing
- I built this PR locally for my native architecture, (x86_64-glibc)


A patch file from https://github.com/void-linux/void-packages/pull/40457.patch is attached

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: github-pr-smplayer-40457.patch --]
[-- Type: text/x-diff, Size: 980 bytes --]

From a180897da8c80c56fe13b9392b14f387aa576469 Mon Sep 17 00:00:00 2001
From: TrueTechie <TrueTechie@mailo.com>
Date: Thu, 10 Nov 2022 16:17:25 -0500
Subject: [PATCH] smplayer: Add mpv as runtime dependency

---
 srcpkgs/smplayer/template | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/srcpkgs/smplayer/template b/srcpkgs/smplayer/template
index d82880fd1a87..70387e7f4a21 100644
--- a/srcpkgs/smplayer/template
+++ b/srcpkgs/smplayer/template
@@ -1,12 +1,12 @@
 # Template file for 'smplayer'
 pkgname=smplayer
 version=22.7.0
-revision=1
+revision=2
 build_style=gnu-makefile
 build_helper=qmake
 hostmakedepends="qt5-host-tools qt5-tools qt5-script-devel tar"
 makedepends="zlib-devel qt5-script-devel qt5-tools-devel"
-depends="hicolor-icon-theme desktop-file-utils"
+depends="hicolor-icon-theme desktop-file-utils mpv"
 short_desc="Complete front-end for mplayer and mpv"
 maintainer="Helmut Pozimski <helmut@pozimski.eu>"
 license="GPL-2.0-or-later"

Re: smplayer: Add mpv as runtime dependency

[TrueTechie]

[-- Attachment #1: Type: text/plain, Size: 159 bytes --]

New comment by TrueTechie on void-packages repository

https://github.com/void-linux/void-packages/pull/40457#issuecomment-1310926019

Comment:
Closes #40456 

Re: smplayer: Add mpv as runtime dependency

[classabbyamp]

[-- Attachment #1: Type: text/plain, Size: 249 bytes --]

New comment by classabbyamp on void-packages repository

https://github.com/void-linux/void-packages/pull/40457#issuecomment-1310940066

Comment:
> Closes #40456

put this in the PR description and/or the commit body so it automatically closes it

Re: smplayer: Add mpv as runtime dependency

[sgn]

[-- Attachment #1: Type: text/plain, Size: 206 bytes --]

New comment by sgn on void-packages repository

https://github.com/void-linux/void-packages/pull/40457#issuecomment-1311150342

Comment:
smplayer can use mplayer as backend, mpv is not a hard dependencies.

Re: smplayer: Add mpv as runtime dependency

[TrueTechie]

[-- Attachment #1: Type: text/plain, Size: 159 bytes --]

New comment by TrueTechie on void-packages repository

https://github.com/void-linux/void-packages/pull/40457#issuecomment-1310926019

Comment:
Closes #40456 

Re: smplayer: Add mpv as runtime dependency

[TrueTechie]

[-- Attachment #1: Type: text/plain, Size: 410 bytes --]

New comment by TrueTechie on void-packages repository

https://github.com/void-linux/void-packages/pull/40457#issuecomment-1311169268

Comment:
> smplayer can use mplayer as backend, mpv is not a hard dependencies.

So should `mplayer` be the dependency or what? 

Unlike other packages that are broken  without "optional" dependencies such as `steam`, there is no documentation in the package about this.

Re: smplayer: Add mpv as runtime dependency

[paper42]

[-- Attachment #1: Type: text/plain, Size: 242 bytes --]

New comment by paper42 on void-packages repository

https://github.com/void-linux/void-packages/pull/40457#issuecomment-1311352866

Comment:
If you run smplayer and don't have mpv or mplayer installed, do you get a descriptive error message?

Re: smplayer: Add mpv as runtime dependency

[TrueTechie]

[-- Attachment #1: Type: text/plain, Size: 561 bytes --]

New comment by TrueTechie on void-packages repository

https://github.com/void-linux/void-packages/pull/40457#issuecomment-1311904669

Comment:
> If you run smplayer and don't have mpv or mplayer installed, do you get a descriptive error message?
I guess that is up to interpretation: 

```
MPlayer/mpv failed to start. Please check the MPlayer/mpv path in preferences.
```
This is a little cryptic as it tells the user it is improperly configured, possibly implying the need for a standalone version of `mpv` or `mplayer`, rather than missing a package.

Re: smplayer: Add mpv as runtime dependency

[TrueTechie]

[-- Attachment #1: Type: text/plain, Size: 563 bytes --]

New comment by TrueTechie on void-packages repository

https://github.com/void-linux/void-packages/pull/40457#issuecomment-1311904669

Comment:
> If you run smplayer and don't have mpv or mplayer installed, do you get a descriptive error message?

I guess that is up to interpretation: 

```
MPlayer/mpv failed to start. Please check the MPlayer/mpv path in preferences.
```
This is a little cryptic as it tells the user it is improperly configured, possibly implying the need for a standalone version of `mpv` or `mplayer`, rather than missing a package.

Re: smplayer: Add mpv as runtime dependency

[lxlml]

[-- Attachment #1: Type: text/plain, Size: 525 bytes --]

New comment by lxlml on void-packages repository

https://github.com/void-linux/void-packages/pull/40457#issuecomment-1312152258

Comment:
I personally prefer smplayer as it is now because I need to complile MPV with vapoursynth support and I am not sure how it would interact if smplayer comes with a hard mpv dependecy.

Besides if one were to search up smplayer, it specifically states it's a frontend of MPV or Mplayer so there should be no confusion. I think letting people choose what they want is the better choice.

Re: smplayer: Add mpv as runtime dependency

[lxlml]

[-- Attachment #1: Type: text/plain, Size: 533 bytes --]

New comment by lxlml on void-packages repository

https://github.com/void-linux/void-packages/pull/40457#issuecomment-1312152258

Comment:
I personally prefer smplayer as it is now because I need to compile MPV with vapoursynth support and I am not sure how it would interact if smplayer comes with a hard mpv dependency.

Besides if one were to search up smplayer, it specifically states it's a frontend of MPV or Mplayer so there should be no confusion. I think letting people choose what backend they want is the better choice.

Re: smplayer: Add mpv as runtime dependency

[lxlml]

[-- Attachment #1: Type: text/plain, Size: 556 bytes --]

New comment by lxlml on void-packages repository

https://github.com/void-linux/void-packages/pull/40457#issuecomment-1312152258

Comment:
I personally prefer smplayer as it is now because I need to compile MPV with vapoursynth support and I am not sure how it would interact if smplayer comes with a hard mpv dependency.

Besides if one were to search up smplayer, you will easily find information that states it's a frontend of MPV or Mplayer. So there should be no confusion. I think letting people choose what backend they want is the better choice.

Re: smplayer: Add mpv as runtime dependency

[lxlml]

[-- Attachment #1: Type: text/plain, Size: 557 bytes --]

New comment by lxlml on void-packages repository

https://github.com/void-linux/void-packages/pull/40457#issuecomment-1312152258

Comment:
I personally prefer smplayer as it is now because I need to compile MPV with vapoursynth support and I am not sure how it would interact if smplayer comes with a hard mpv dependency.

Besides if one were to search up smplayer, you will easily find information that states it's a frontend for MPV or Mplayer. So there should be no confusion. I think letting people choose what backend they want is the better choice.

Re: smplayer: Add mpv as runtime dependency

[classabbyamp]

[-- Attachment #1: Type: text/plain, Size: 314 bytes --]

New comment by classabbyamp on void-packages repository

https://github.com/void-linux/void-packages/pull/40457#issuecomment-1312162360

Comment:
if it is not a dependency caused by requiring a shared library, it is possible to use [ignorepkg](https://man.voidlinux.org/xbps.d#ignorepkg=pkgname) to not install it

Re: smplayer: Add mpv as runtime dependency

[sgn]

[-- Attachment #1: Type: text/plain, Size: 250 bytes --]

New comment by sgn on void-packages repository

https://github.com/void-linux/void-packages/pull/40457#issuecomment-1312719589

Comment:
I prefer to patch `smplayer` to provide better error message. I dont use smplayer, so I don't care much, anyway.

Re: smplayer: Add mpv as runtime dependency

[paper42]

[-- Attachment #1: Type: text/plain, Size: 379 bytes --]

New comment by paper42 on void-packages repository

https://github.com/void-linux/void-packages/pull/40457#issuecomment-1312744730

Comment:
> I prefer to patch `smplayer` to provide better error message. I dont use smplayer, so I don't care much, anyway.

I second that, but the patch should be upstream, we shouldn't carry functional patches like this one in the distribution.

Re: smplayer: Add mpv as runtime dependency

[paper42]

[-- Attachment #1: Type: text/plain, Size: 381 bytes --]

New comment by paper42 on void-packages repository

https://github.com/void-linux/void-packages/pull/40457#issuecomment-1312744730

Comment:
> I prefer to patch `smplayer` to provide better error message. I dont use smplayer, so I don't care much, anyway.

I second that, but the patch should be upstreamed, we shouldn't carry functional patches like this one in the distribution.

Re: smplayer: Add mpv as runtime dependency

[sgn]

[-- Attachment #1: Type: text/plain, Size: 658 bytes --]

New comment by sgn on void-packages repository

https://github.com/void-linux/void-packages/pull/40457#issuecomment-1312918019

Comment:
Even if we want to put one optional dependency to `smplayer`, it should be `mplayer` not `mpv`.
Quoting from their homepage:

> SMPlayer is a graphical user interface (GUI) for the award-winning [**MPlayer**](http://www.mplayerhq.hu/design7/info.html), which is capable of playing almost all known video and audio formats. But apart from providing access for the most common and useful options of **MPlayer**, SMPlayer adds other interesting features like the possibility to play Youtube videos or download subtitles.

Re: smplayer: Add mpv as runtime dependency

[ahesford]

[-- Attachment #1: Type: text/plain, Size: 1052 bytes --]

New comment by ahesford on void-packages repository

https://github.com/void-linux/void-packages/pull/40457#issuecomment-1320964279

Comment:
I disagree that the error message, "MPlayer/mpv failed to start", is particularly cryptic. "Please check the path" is a recommendation about how to solve the problem, and a user who tries to inspect the path should be able to determine that neither of these alternatives is available.

Although I would be very happy to see a decent implementation of optional dependencies for XBPS, we currently have no facility for this. Our policy is not to force hard dependencies on software that works with several viable alternatives and, as sgn noted, the natural choice would be `mplayer` anyway because that seems to be the first-class backend for SMplayer. Of course, forcing a user to `ignorepkg` something to swap backends is at least as lousy an experience (and less obviously resolved) than the current situation.

Any team member disagreeing with me can reopen this PR, but it seems like a WONTFIX for now.

Re: [PR PATCH] [Closed]: smplayer: Add mpv as runtime dependency

[ahesford]

[-- Attachment #1: Type: text/plain, Size: 983 bytes --]

There's a closed pull request on the void-packages repository

smplayer: Add mpv as runtime dependency
https://github.com/void-linux/void-packages/pull/40457

Description:
<!-- Uncomment relevant sections and delete options which are not applicable -->

#### Testing the changes
- I tested the changes in this PR: **briefly**

<!--
#### New package
- This new package conforms to the [package requirements](https://github.com/void-linux/void-packages/blob/master/CONTRIBUTING.md#package-requirements): **YES**|**NO**
-->

<!-- Note: If the build is likely to take more than 2 hours, please add ci skip tag as described in
https://github.com/void-linux/void-packages/blob/master/CONTRIBUTING.md#continuous-integration
and test at least one native build and, if supported, at least one cross build.
Ignore this section if this PR is not skipping CI.
-->

#### Local build testing
- I built this PR locally for my native architecture, (x86_64-glibc)

Closes #40456