diff mbox series

[bug#48028,wip-gnome,5/8] gnu: libsoup: Add missing argument and input.

Message ID 20210426081145.28926-5-rg@raghavgururajan.name
State New
Headers show
Series [bug#48028,wip-gnome,1/8] gnu: gtkmm@2: Remove inheritance from gtkmm@3. | expand

Checks

Context Check Description
cbaines/submitting builds success
cbaines/comparison success View comparision
cbaines/git branch success View Git branch
cbaines/applying patch success View Laminar job
cbaines/issue success View issue

Commit Message

Raghav Gururajan April 26, 2021, 8:11 a.m. UTC
Propagate glib-networking as per .pc file.

* gnu/packages/gnome.scm (libsoup)[arguments](glib-or-gtk): New argument.
[inputs]: Add samba. Move glib-networking to ...
[propagated-inputs]: ... here.
---
 gnu/packages/gnome.scm | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

Comments

Leo Prikler April 26, 2021, 8:40 a.m. UTC | #1
This headline does not match the contents of the patch.

Am Montag, den 26.04.2021, 04:11 -0400 schrieb Raghav Gururajan:
> Propagate glib-networking as per .pc file.
This looks like it could be its own patch were it not for the fact,
that samba needs to be added as well (what for?)

> * gnu/packages/gnome.scm (libsoup)[arguments](glib-or-gtk): New
> argument.
> [inputs]: Add samba. Move glib-networking to ...
> [propagated-inputs]: ... here.
> ---
>  gnu/packages/gnome.scm | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/gnu/packages/gnome.scm b/gnu/packages/gnome.scm
> index 29779c39af..b69980091b 100644
> --- a/gnu/packages/gnome.scm
> +++ b/gnu/packages/gnome.scm
> @@ -4913,7 +4913,7 @@ libxml to ease remote use of the RESTful API.")
>       `(#:modules ((guix build utils)
>                    (guix build meson-build-system)
>                    (ice-9 popen))
> -
> +       #:glib-or-gtk? #t ; To wrap binaries and/or compile schemas
That doesn't sound very sure to me.  Why are you wrapping binaries
and/or compiling schemas?
>         #:configure-flags '("-Dgtk_doc=true")
>         #:phases
>         (modify-phases %standard-phases
> @@ -4964,13 +4964,14 @@ libxml to ease remote use of the RESTful
> API.")
>       ;; libsoup-2.4.pc refers to all these.
>       `(("brotli" ,google-brotli)
>         ("glib" ,glib)
> +       ("glib-networking" ,glib-networking)
I haven't checked, but if the .pc file says so, so be it.
>         ("libpsl" ,libpsl)
>         ("libxml2" ,libxml2)
>         ("sqlite" ,sqlite)
>         ("zlib" ,zlib)))
>      (inputs
> -     `(("glib-networking" ,glib-networking)
> -       ("mit-krb5" ,mit-krb5)))
> +     `(("mit-krb5" ,mit-krb5)
> +        ("ntlm_auth" ,samba)))
Careful with the indentation.
>      (home-page "https://live.gnome.org/LibSoup/")
>      (synopsis "GLib-based HTTP Library")
>      (description

Regards,
Leo
Raghav Gururajan April 26, 2021, 9:28 a.m. UTC | #2
Hi Leo!

> This looks like it could be its own patch were it not for the fact,
> that samba needs to be added as well (what for?)

I have split them into two patches and added comment, in v3.

> That doesn't sound very sure to me.  Why are you wrapping binaries
> and/or compiling schemas?

It is mainly for compiling schemas, as libsoup doesn't have binaries to 
wrap. But I just now realized that there is no 
`[out]/share/glib-2.0/schemas` either. So I removed this in v3.

> Careful with the indentation.

Ah, good catch. I have fixed it in v3.

Thanks!

Regards,
RG.
Leo Prikler April 26, 2021, 9:47 a.m. UTC | #3
Am Montag, den 26.04.2021, 05:28 -0400 schrieb Raghav Gururajan:
> Hi Leo!
> 
> > This looks like it could be its own patch were it not for the fact,
> > that samba needs to be added as well (what for?)
> 
> I have split them into two patches and added comment, in v3.
> 
> > That doesn't sound very sure to me.  Why are you wrapping binaries
> > and/or compiling schemas?
> 
> It is mainly for compiling schemas, as libsoup doesn't have binaries
> to 
> wrap. But I just now realized that there is no 
> `[out]/share/glib-2.0/schemas` either. So I removed this in v3.
> 
> > Careful with the indentation.
> 
> Ah, good catch. I have fixed it in v3.
v3 06/07 LGTM, but could in my opinon be merged into a single patch
with the line: "gnu: libsoup: Add missing inputs." or perhaps "gnu:
libsoup: Adjust inputs".  I've CC'd Mark Weaver to hear their input.

Regards,
Leo
Raghav Gururajan April 26, 2021, 10:32 a.m. UTC | #4
Hi Leo!

> v3 06/07 LGTM, but could in my opinon be merged into a single patch
> with the line: "gnu: libsoup: Add missing inputs." or perhaps "gnu:
> libsoup: Adjust inputs".  I've CC'd Mark Weaver to hear their input.

Makes sense! I have merged the changes into single patch, in v4.

Regards,
RG.
Mark H Weaver April 26, 2021, 5:26 p.m. UTC | #5
Hi Raghav,

Version 4 of your patch set mostly looks good to me, but I have one
concern about patch 3 (gnu: gtkmm: Add missing native-input and correct
propagated-inputs).

That patch modifies the 'propagated-inputs' field of 'gtkmm' to refer to
specific versions of packages: 'atkmm-2.28', 'cairomm-1.13', and
'pangomm-2.42'.  These references to specific versions are not ideal,
because:

(1) They will likely lead to conflicts within profiles.  For example, a
profile that includes both 'gtkmm' and 'cairomm' may fail to build,
because it would require including both 'cairomm' and 'cairomm-1.13',
and

(2) We might forget to update these references in the future.

It would be good to avoid these version-specific references, if
possible.  Can you help me understand the rationale?  Did you find that
there is an incompatibility between the latest stable versions of
'gtkmm', 'atkmm', 'cairomm', and 'pangomm'?  If so, could you help me
understand the nature of that incompatibility?  Perhaps it can be
addressed in another way.

If it turns out that these versioned references are truly unavoidable,
it would be good to add comments next to those references, briefly
explaining the rationale.

Thanks very much for your work on this, Raghav.

     Regards,
       Mark
Raghav Gururajan April 27, 2021, 9:37 a.m. UTC | #6
Hi Mark,

> It would be good to avoid these version-specific references, if
> possible.  Can you help me understand the rationale?  Did you find that
> there is an incompatibility between the latest stable versions of
> 'gtkmm', 'atkmm', 'cairomm', and 'pangomm'?  If so, could you help me
> understand the nature of that incompatibility?  Perhaps it can be
> addressed in another way.

Its because of ABI incompatibility. The pkg-config for gtkmm, checks for 
specific ABI versions of atkmm, cairomm and pangomm.

> If it turns out that these versioned references are truly unavoidable,
> it would be good to add comments next to those references, briefly
> explaining the rationale.

I have added comment in the code, in v5.

Regards,
RG.
Mark H Weaver April 27, 2021, 9:49 p.m. UTC | #7
Hi Raghav,

Raghav Gururajan <rg@raghavgururajan.name> writes:

>> It would be good to avoid these version-specific references, if
>> possible.  Can you help me understand the rationale?  Did you find that
>> there is an incompatibility between the latest stable versions of
>> 'gtkmm', 'atkmm', 'cairomm', and 'pangomm'?  If so, could you help me
>> understand the nature of that incompatibility?  Perhaps it can be
>> addressed in another way.
>
> Its because of ABI incompatibility. The pkg-config for gtkmm, checks for 
> specific ABI versions of atkmm, cairomm and pangomm.
>
>> If it turns out that these versioned references are truly unavoidable,
>> it would be good to add comments next to those references, briefly
>> explaining the rationale.
>
> I have added comment in the code, in v5.

Respectfully, it seems to me that you've been too quick to dismiss my
concerns.  As I pointed out in my previous email:

  (1) [These versioned references] will likely lead to conflicts within
  profiles.  For example, a profile that includes both 'gtkmm' and
  'cairomm' may fail to build, because it would require including both
  'cairomm' and 'cairomm-1.13',

This could be a real annoyance.  Guix users should be able to run "guix
install gtkmm atkmm cairomm pangomm" and have that work.  With these
proposed patches applied, I suspect that it might not work.

Traditional GNU/Linux distributions that package GNOME 40 will certainly
choose versions of 'gtkmm', 'atkmm', 'cairomm', and 'pangomm' that are
compatible with each other.  We should too, I think.

From my own experience performing a GNOME upgrade for Guix a few years
ago, I remember that when the GNOME developers produce a new GNOME
release, they provide somewhere a list of the versions of each component
that are part of that release.  Presumably they choose those versions to
be compatible with each other.

This makes me wonder if some of the GNOME components on the 'wip-gnome'
branch are newer than they should be (perhaps a development version) or
older than they should be.

What do you think?

    Regards,
      Mark
Raghav Gururajan May 5, 2021, 7:06 p.m. UTC | #8
Hi Mark!

> Respectfully, it seems to me that you've been too quick to dismiss my
> concerns.  As I pointed out in my previous email:
> 
>    (1) [These versioned references] will likely lead to conflicts within
>    profiles.  For example, a profile that includes both 'gtkmm' and
>    'cairomm' may fail to build, because it would require including both
>    'cairomm' and 'cairomm-1.13',

I didn't dismiss your concerns. Since you mentioned "If it turns out 
that these versioned references are truly unavoidable [...]", at the 
time of my reply, I was convinced that its unavoidable.


> This could be a real annoyance.  Guix users should be able to run "guix
> install gtkmm atkmm cairomm pangomm" and have that work.  With these
> proposed patches applied, I suspect that it might not work.
> 
> Traditional GNU/Linux distributions that package GNOME 40 will certainly
> choose versions of 'gtkmm', 'atkmm', 'cairomm', and 'pangomm' that are
> compatible with each other.  We should too, I think.
> 
>  From my own experience performing a GNOME upgrade for Guix a few years
> ago, I remember that when the GNOME developers produce a new GNOME
> release, they provide somewhere a list of the versions of each component
> that are part of that release.  Presumably they choose those versions to
> be compatible with each other.
> 
> This makes me wonder if some of the GNOME components on the 'wip-gnome'
> branch are newer than they should be (perhaps a development version) or
> older than they should be.
> 
> What do you think?

I tried dirty hack of patching meson.build of gtkmm, to refer to latest 
versions of cairomm, atkmm, pangomm. Though, 'configure phase passes, 
'build phase fails with *numerous* errors.

If I de-propagate those inputs, packages that depends on gtkmm via 
pkg-config, gonna barf "gtkmm not found".

Do you have any ideas?

Regards,
RG.
Raghav Gururajan May 5, 2021, 8:10 p.m. UTC | #9
Hi Mark!

> I tried dirty hack of patching meson.build of gtkmm, to refer to latest 

> versions of cairomm, atkmm, pangomm. Though, 'configure phase passes, 
> 'build phase fails with *numerous* errors.
> 
> If I de-propagate those inputs, packages that depends on gtkmm via 
> pkg-config, gonna barf "gtkmm not found".
> 
> Do you have any ideas?

Libsigc++, glibmm, cairomm, pangomm and atkmm; play well with each 
other's latest stable version. The gtkmm is the only outlier.

So I think we can either de-propagate inputs in gtkmm or use the patch 
as-is; and revert changes when new gtkmm version is released.

WDYT?

Regards,
RG.
Raghav Gururajan May 5, 2021, 8:55 p.m. UTC | #10
Hi Mark,

>> I tried dirty hack of patching meson.build of gtkmm, to refer to latest 
> 
>> versions of cairomm, atkmm, pangomm. Though, 'configure phase passes, 
>> 'build phase fails with *numerous* errors.
>>
>> If I de-propagate those inputs, packages that depends on gtkmm via 
>> pkg-config, gonna barf "gtkmm not found".
>>
>> Do you have any ideas?
> 
> Libsigc++, glibmm, cairomm, pangomm and atkmm; play well with each 
> other's latest stable version. The gtkmm is the only outlier.
> 
> So I think we can either de-propagate inputs in gtkmm or use the patch 
> as-is; and revert changes when new gtkmm version is released.
> 
> WDYT?

I just realized packages that depend on gtkmm, doesn't propagate it. So 
only way gtkmm, ends up in user-profiles is when explicitly installed. 
Thus conflicts doesn't happen while building profiles right?

Regards,
RG.
diff mbox series

Patch

diff --git a/gnu/packages/gnome.scm b/gnu/packages/gnome.scm
index 29779c39af..b69980091b 100644
--- a/gnu/packages/gnome.scm
+++ b/gnu/packages/gnome.scm
@@ -4913,7 +4913,7 @@  libxml to ease remote use of the RESTful API.")
      `(#:modules ((guix build utils)
                   (guix build meson-build-system)
                   (ice-9 popen))
-
+       #:glib-or-gtk? #t ; To wrap binaries and/or compile schemas
        #:configure-flags '("-Dgtk_doc=true")
        #:phases
        (modify-phases %standard-phases
@@ -4964,13 +4964,14 @@  libxml to ease remote use of the RESTful API.")
      ;; libsoup-2.4.pc refers to all these.
      `(("brotli" ,google-brotli)
        ("glib" ,glib)
+       ("glib-networking" ,glib-networking)
        ("libpsl" ,libpsl)
        ("libxml2" ,libxml2)
        ("sqlite" ,sqlite)
        ("zlib" ,zlib)))
     (inputs
-     `(("glib-networking" ,glib-networking)
-       ("mit-krb5" ,mit-krb5)))
+     `(("mit-krb5" ,mit-krb5)
+        ("ntlm_auth" ,samba)))
     (home-page "https://live.gnome.org/LibSoup/")
     (synopsis "GLib-based HTTP Library")
     (description