[bug#68250,v2] gnu: mpv: Fix pkgconfig file.

  This is a follow-up to ce7b2b57aa6da0ceace94ea5fb42392c7ff97d53.

* gnu/packages/video.scm (mpv)[arguments]<#:phases>: Add 'fix-mpv.pc.

Suggested-by: 宋文武 <iyzsong@member.fsf.org>
Change-Id: I9826d5d6c957ca3022fa326aee111edb533f05bc
---
 gnu/packages/video.scm | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)


base-commit: c0e21e523d93081153a2ffc91e5a9f06afe62b91
--
2.41.0
  

Hi Hilton,

Hilton Chain <hako@ultrarare.space> writes:

> This is a follow-up to ce7b2b57aa6da0ceace94ea5fb42392c7ff97d53.
>
> * gnu/packages/video.scm (mpv)[arguments]<#:phases>: Add 'fix-mpv.pc.

Does this completely replace the previous patch, instead of augmenting it?

Best,
  

Hi Josselin,

On Thu, 11 Jan 2024 02:24:16 +0800,
Josselin Poiret wrote:
>
> [1  <text/plain (quoted-printable)>]
> Hi Hilton,
>
> Hilton Chain <hako@ultrarare.space> writes:
>
> > This is a follow-up to ce7b2b57aa6da0ceace94ea5fb42392c7ff97d53.
> >
> > * gnu/packages/video.scm (mpv)[arguments]<#:phases>: Add 'fix-mpv.pc.
>
> Does this completely replace the previous patch, instead of augmenting it?

Yes!  I should clarify this before.
  

Hi,

Hilton Chain <hako@ultrarare.space> writes:

> This is a follow-up to ce7b2b57aa6da0ceace94ea5fb42392c7ff97d53.
>
> * gnu/packages/video.scm (mpv)[arguments]<#:phases>: Add 'fix-mpv.pc.
>
> Suggested-by: 宋文武 <iyzsong@member.fsf.org>
> Change-Id: I9826d5d6c957ca3022fa326aee111edb533f05bc
> ---
>  gnu/packages/video.scm | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/gnu/packages/video.scm b/gnu/packages/video.scm
> index e70aa5352e..10d46db38b 100644
> --- a/gnu/packages/video.scm
> +++ b/gnu/packages/video.scm
> @@ -2365,7 +2365,13 @@ (define-public mpv
>                ;; and passed as linker flags, but the order in which they are added
>                ;; varies.  See <https://github.com/mpv-player/mpv/issues/7855>.
>                ;; Set PYTHONHASHSEED as a workaround for deterministic results.
> -              (setenv "PYTHONHASHSEED" "1"))))
> +              (setenv "PYTHONHASHSEED" "1")))
> +          ;; mpv.pc is generated by meson.  libmpv's headers don't actually
> +          ;; require these dependencies so it's safe to remove these two fields.
> +          (add-after 'install 'fix-mpv.pc
> +            (lambda _
> +              (substitute* (string-append #$output "/lib/pkgconfig/mpv.pc")
> +                (("^(Requires|Libs)\\.private:.*") "")))))
>        #:configure-flags
>        #~(list "-Dlibmpv=true"
>                "-Dcdda=enabled"

I've just seen this, after pushing a hot fix as
f3fdb4e041cb5740ba0b38b9ad017571f8414d33, which propagates all listed
libraries in Requires.private.

I suppose that someone wanting to build something statically from mpv
would benefit from having the original mpv.pc file without
modifications.

Perhaps we can try it out for a bit, and if the propagation causes
problems, we can fall-back to this change here?
  

Hi Maxim,

On Fri, 19 Jan 2024 10:05:41 +0800,
Maxim Cournoyer wrote:
>
> Hi,
>
> Hilton Chain <hako@ultrarare.space> writes:
>
> > This is a follow-up to ce7b2b57aa6da0ceace94ea5fb42392c7ff97d53.
> >
> > * gnu/packages/video.scm (mpv)[arguments]<#:phases>: Add 'fix-mpv.pc.
> >
> > Suggested-by: 宋文武 <iyzsong@member.fsf.org>
> > Change-Id: I9826d5d6c957ca3022fa326aee111edb533f05bc
> > ---
> >  gnu/packages/video.scm | 8 +++++++-
> >  1 file changed, 7 insertions(+), 1 deletion(-)
> >
> > diff --git a/gnu/packages/video.scm b/gnu/packages/video.scm
> > index e70aa5352e..10d46db38b 100644
> > --- a/gnu/packages/video.scm
> > +++ b/gnu/packages/video.scm
> > @@ -2365,7 +2365,13 @@ (define-public mpv
> >                ;; and passed as linker flags, but the order in which they are added
> >                ;; varies.  See <https://github.com/mpv-player/mpv/issues/7855>.
> >                ;; Set PYTHONHASHSEED as a workaround for deterministic results.
> > -              (setenv "PYTHONHASHSEED" "1"))))
> > +              (setenv "PYTHONHASHSEED" "1")))
> > +          ;; mpv.pc is generated by meson.  libmpv's headers don't actually
> > +          ;; require these dependencies so it's safe to remove these two fields.
> > +          (add-after 'install 'fix-mpv.pc
> > +            (lambda _
> > +              (substitute* (string-append #$output "/lib/pkgconfig/mpv.pc")
> > +                (("^(Requires|Libs)\\.private:.*") "")))))
> >        #:configure-flags
> >        #~(list "-Dlibmpv=true"
> >                "-Dcdda=enabled"
>
> I've just seen this, after pushing a hot fix as
> f3fdb4e041cb5740ba0b38b9ad017571f8414d33, which propagates all listed
> libraries in Requires.private.
>
> I suppose that someone wanting to build something statically from mpv
> would benefit from having the original mpv.pc file without
> modifications.
>
> Perhaps we can try it out for a bit, and if the propagation causes
> problems, we can fall-back to this change here?


Then we should follow the approach in v1 to add libmpv as a separate package.
I'll send v3 later.

Thanks
  

Hi Hilton,

[...]

>> I've just seen this, after pushing a hot fix as
>> f3fdb4e041cb5740ba0b38b9ad017571f8414d33, which propagates all listed
>> libraries in Requires.private.
>>
>> I suppose that someone wanting to build something statically from mpv
>> would benefit from having the original mpv.pc file without
>> modifications.
>>
>> Perhaps we can try it out for a bit, and if the propagation causes
>> problems, we can fall-back to this change here?
>
>
> Then we should follow the approach in v1 to add libmpv as a separate package.
> I'll send v3 later.

Actually I don't think we need to split out a library for mpv here;
there are two things we can do in the short term:

1. Build MPV strictly as a shared library (-Dbuild=shared) or something
in Meson.  Then Meson doesn't output all these Requires.private
dependencies, which are only useful for static linking, and which
pkg-config confusingly uses for its --exists check.

2. On core-updates, I've been experimenting with replacing pkg-config
with pkgconf, which seems to be designed better for avoiding the above
pkg-config's pitfall.  Currently stuck on a hard to understand cycle.
  

Hi Hilton,

Maxim Cournoyer <maxim.cournoyer@gmail.com> writes:

> Hi Hilton,
>
> [...]
>
>>> I've just seen this, after pushing a hot fix as
>>> f3fdb4e041cb5740ba0b38b9ad017571f8414d33, which propagates all listed
>>> libraries in Requires.private.
>>>
>>> I suppose that someone wanting to build something statically from mpv
>>> would benefit from having the original mpv.pc file without
>>> modifications.
>>>
>>> Perhaps we can try it out for a bit, and if the propagation causes
>>> problems, we can fall-back to this change here?
>>
>>
>> Then we should follow the approach in v1 to add libmpv as a separate package.
>> I'll send v3 later.
>
> Actually I don't think we need to split out a library for mpv here;
> there are two things we can do in the short term:
>
> 1. Build MPV strictly as a shared library (-Dbuild=shared) or something
> in Meson.  Then Meson doesn't output all these Requires.private
> dependencies, which are only useful for static linking, and which
> pkg-config confusingly uses for its --exists check.
>
> 2. On core-updates, I've been experimenting with replacing pkg-config
> with pkgconf, which seems to be designed better for avoiding the above
> pkg-config's pitfall.  Currently stuck on a hard to understand cycle.

I've implemented 2. above; see: <https://issues.guix.gnu.org/68813>,
which removes the need to propagate pkg-config *.private fields, which
should only be needed when doing static builds.

I'm thus closing this issue.  Let me know if I've missed anything!
  

Hi Maxim,

On Tue, 27 Feb 2024 00:01:31 +0800,
Maxim Cournoyer wrote:
>
> Hi Hilton,
>
> Maxim Cournoyer <maxim.cournoyer@gmail.com> writes:
>
> > Hi Hilton,
> >
> > [...]
> >
> >>> I've just seen this, after pushing a hot fix as
> >>> f3fdb4e041cb5740ba0b38b9ad017571f8414d33, which propagates all listed
> >>> libraries in Requires.private.
> >>>
> >>> I suppose that someone wanting to build something statically from mpv
> >>> would benefit from having the original mpv.pc file without
> >>> modifications.
> >>>
> >>> Perhaps we can try it out for a bit, and if the propagation causes
> >>> problems, we can fall-back to this change here?
> >>
> >>
> >> Then we should follow the approach in v1 to add libmpv as a separate package.
> >> I'll send v3 later.
> >
> > Actually I don't think we need to split out a library for mpv here;
> > there are two things we can do in the short term:
> >
> > 1. Build MPV strictly as a shared library (-Dbuild=shared) or something
> > in Meson.  Then Meson doesn't output all these Requires.private
> > dependencies, which are only useful for static linking, and which
> > pkg-config confusingly uses for its --exists check.
> >
> > 2. On core-updates, I've been experimenting with replacing pkg-config
> > with pkgconf, which seems to be designed better for avoiding the above
> > pkg-config's pitfall.  Currently stuck on a hard to understand cycle.
>
> I've implemented 2. above; see: <https://issues.guix.gnu.org/68813>,
> which removes the need to propagate pkg-config *.private fields, which
> should only be needed when doing static builds.
>
> I'm thus closing this issue.  Let me know if I've missed anything!


Great to see!  Thank you very much!