[PATCH] gnu: deluge: Move librsvg to native inputs.

  • Done
  • quality assurance status badge
Details
3 participants
  • Brice Waegeneire
  • Josselin Poiret
  • Leo Famulari
Owner
unassigned
Submitted by
Josselin Poiret
Severity
normal
J
J
Josselin Poiret wrote on 14 Dec 2021 18:21
(address . guix-patches@gnu.org)(name . Josselin Poiret)(address . dev@jpoiret.xyz)
20211214172151.28831-1-dev@jpoiret.xyz
Grepping the source code shows that librsvg is run by the
builder, and not by deluge itself. This fixes propagation conflicting
with gtk+'s librsvg. For now, use the C variant to build on all archs.
-- >8 --

* gnu/packages/bittorrent.scm (deluge): Do it.
---
gnu/packages/bittorrent.scm | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)

Toggle diff (23 lines)
diff --git a/gnu/packages/bittorrent.scm b/gnu/packages/bittorrent.scm
index 0dcb1ee991..5513878fc0 100644
--- a/gnu/packages/bittorrent.scm
+++ b/gnu/packages/bittorrent.scm
@@ -528,7 +528,6 @@ (define-public deluge
(build-system python-build-system)
(propagated-inputs
`(("gtk+" ,gtk+)
- ("librsvg" ,librsvg)
("libtorrent" ,libtorrent-rasterbar)
("python-pycairo" ,python-pycairo)
("python-chardet" ,python-chardet)
@@ -545,7 +544,7 @@ (define-public deluge
("python-twisted" ,python-twisted)
("python-zope-interface" ,python-zope-interface)))
(native-inputs
- (list intltool python-wheel))
+ (list intltool python-wheel librsvg-2.40))
;; TODO: Enable tests.
;; After "pytest-twisted" is packaged, HOME is set, and an X server is
;; started, some of the tests still fail. There are likely some tests
--
2.34.0
L
L
Leo Famulari wrote on 14 Dec 2021 18:56
(name . Josselin Poiret via Guix-patches via)(address . guix-patches@gnu.org)
YbjaxAxmJ9V2A1Vr@jasmine.lan
On Tue, Dec 14, 2021 at 06:21:51PM +0100, Josselin Poiret via Guix-patches via wrote:
Toggle quote (5 lines)
> Grepping the source code shows that librsvg is run by the
> builder, and not by deluge itself. This fixes propagation conflicting
> with gtk+'s librsvg. For now, use the C variant to build on all archs.
> -- >8 --

I applied your patch and built Deluge. Then, I checked what packages it
keeps a reference to:

------
$ guix gc --references $(./pre-inst-env guix build deluge) | grep librsvg
/gnu/store/2dza2psfbrrbvsni8jjqzzqx3hmm8kw8-librsvg-2.50.7
/gnu/store/zxpbc78z40x7dr3ls4dgclkq7i4agx7a-librsvg-2.40.21
------

So, Guix already records a run-time dependency on the Rust variant of
librsvg (likely via GTK+), as well as the C variant if we apply your
patch.

I agree that, if Deluge does not use librsvg at run-time, we should make
it a non-propagated-input. But, we should use the Rust variant on
platforms where it is supported. Check the package definition of GTK+
for an example.

And we should also make it a 'regular' input, because the package does
keep a run-time reference to it.

We are phasing out the C variant because it is abandoned upstream and no
longer receiving security updates.
J
J
Josselin Poiret wrote on 14 Dec 2021 20:17
[PATCH v2 1/2] gnu: deluge: Move librsvg to native inputs
(name . Leo Famulari)(address . leo@famulari.name)
20211214191713.18665-2-dev@jpoiret.xyz
* gnu/packages/bittorrent.scm (deluge): Do it.
---
gnu/packages/bittorrent.scm | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)

Toggle diff (26 lines)
diff --git a/gnu/packages/bittorrent.scm b/gnu/packages/bittorrent.scm
index 0dcb1ee991..6c6d01991a 100644
--- a/gnu/packages/bittorrent.scm
+++ b/gnu/packages/bittorrent.scm
@@ -528,7 +528,6 @@ (define-public deluge
(build-system python-build-system)
(propagated-inputs
`(("gtk+" ,gtk+)
- ("librsvg" ,librsvg)
("libtorrent" ,libtorrent-rasterbar)
("python-pycairo" ,python-pycairo)
("python-chardet" ,python-chardet)
@@ -545,7 +544,10 @@ (define-public deluge
("python-twisted" ,python-twisted)
("python-zope-interface" ,python-zope-interface)))
(native-inputs
- (list intltool python-wheel))
+ (list intltool python-wheel
+ (if (string-prefix? "x86_64-" (%current-system))
+ librsvg-bootstrap
+ librsvg-2.40)))
;; TODO: Enable tests.
;; After "pytest-twisted" is packaged, HOME is set, and an X server is
;; started, some of the tests still fail. There are likely some tests
--
2.34.0
J
J
Josselin Poiret wrote on 14 Dec 2021 20:17
[PATCH v2 0/2] Fix deluge propagating a librsvg incompatible with that of gtk+
(name . Leo Famulari)(address . leo@famulari.name)
20211214191713.18665-1-dev@jpoiret.xyz
Hello again,

This time, a modified patch that selects the proper librsvg as is done
in the gtk definition, but adapted for native-inputs, as well as
another that removes the reference to the librsvg needed at build
time.
`guix size /gnu/store/azyp0avyr91jzwwdb3q82al44r3a8g1h-deluge-2.0.3`
reports only one librsvg, that of GTK.

I've tested this by adding a torrent, checking that it does in fact
download. The UI looks ok, so I guess it should work.

Best,
Josselin

Josselin Poiret (2):
gnu: deluge: Move librsvg to native inputs
gnu: deluge: Remove reference of build-time librsvg

gnu/packages/bittorrent.scm | 20 ++++++++++++++++----
1 file changed, 16 insertions(+), 4 deletions(-)

--
2.34.0
J
J
Josselin Poiret wrote on 14 Dec 2021 20:17
[PATCH v2 2/2] gnu: deluge: Remove reference of build-time librsvg
(name . Leo Famulari)(address . leo@famulari.name)
20211214191713.18665-3-dev@jpoiret.xyz
* gnu/packages/bittorrent.scm (deluge): Do it.
---
gnu/packages/bittorrent.scm | 14 ++++++++++++--
1 file changed, 12 insertions(+), 2 deletions(-)

Toggle diff (28 lines)
diff --git a/gnu/packages/bittorrent.scm b/gnu/packages/bittorrent.scm
index 6c6d01991a..2a59b94f92 100644
--- a/gnu/packages/bittorrent.scm
+++ b/gnu/packages/bittorrent.scm
@@ -564,9 +564,19 @@ (define-public deluge
(("names='ngettext'") "names=['ngettext']"))
#t))
(add-after 'install 'wrap
- (lambda* (#:key outputs #:allow-other-keys)
+ (lambda* (#:key native-inputs inputs outputs #:allow-other-keys)
(let ((out (assoc-ref outputs "out"))
- (gi-typelib-path (getenv "GI_TYPELIB_PATH")))
+ (gi-typelib-path
+ (string-join (filter
+ (lambda (x) (not (string-prefix?
+ (assoc-ref
+ (or native-inputs inputs)
+ "librsvg")
+ x)))
+ (string-split
+ (getenv "GI_TYPELIB_PATH")
+ #\:))
+ ":")))
(for-each
(lambda (program)
(wrap-program program
--
2.34.0
B
B
Brice Waegeneire wrote on 16 Jan 2022 11:52
Re: bug#52486: [PATCH] gnu: deluge: Move librsvg to native inputs.
(name . Josselin Poiret)(address . dev@jpoiret.xyz)
87k0f0t0jx.fsf_-_@waegenei.re
Hello Josselin

Josselin Poiret <dev@jpoiret.xyz> writes:

Toggle quote (4 lines)
> Josselin Poiret (2):
> gnu: deluge: Move librsvg to native inputs
> gnu: deluge: Remove reference of build-time librsvg

Thanks for the patch, it fixes the issue with librsvg as expected. Pushed as
1471219a8aabd2d8ad1f6bf1216c734ce73ae175, I've added your copyright to it.

Cheers,
- Brice
Closed
B
B
Brice Waegeneire wrote on 16 Jan 2022 11:52
(name . Josselin Poiret)(address . dev@jpoiret.xyz)
87ilukt0jd.fsf_-_@waegenei.re
Hello Josselin

Josselin Poiret <dev@jpoiret.xyz> writes:

Toggle quote (4 lines)
> Josselin Poiret (2):
> gnu: deluge: Move librsvg to native inputs
> gnu: deluge: Remove reference of build-time librsvg

Thanks for the patch, it fixes the issue with librsvg as expected. Pushed as
1471219a8aabd2d8ad1f6bf1216c734ce73ae175, I've added your copyright to it.

Cheers,
- Brice
Closed
L
L
Leo Famulari wrote on 16 Jan 2022 18:58
Re: [PATCH v2 2/2] gnu: deluge: Remove reference of build-time librsvg
(name . Josselin Poiret)(address . dev@jpoiret.xyz)(address . 52486@debbugs.gnu.org)
YeRc0aIvXWsXY/Nf@jasmine.lan
On Tue, Dec 14, 2021 at 08:17:13PM +0100, Josselin Poiret wrote:
Toggle quote (2 lines)
> * gnu/packages/bittorrent.scm (deluge): Do it.

Thanks for working on this!

Can you send a followup patch adding a code comment that explains the
change? It's good practice to explain code that does strange things,
especially in packages. Otherwise later package maintainers may feel
free to remove it.
B
B
Brice Waegeneire wrote on 16 Jan 2022 22:41
Re: bug#52486: [PATCH] gnu: deluge: Move librsvg to native inputs.
(name . Leo Famulari)(address . leo@famulari.name)
87fspntl2g.fsf_-_@waegenei.re
Hi Leo,

It didn't came into my mind to ask for such explanation to be added as a
commentary, even tho I asked Josselin about it on IRC. I have subbmited a patch
adding such comment in https://issues.guix.gnu.org/53308.

Cheers,
- Brice
?
Your comment

This issue is archived.

To comment on this conversation send an email to 52486@debbugs.gnu.org

To respond to this issue using the mumi CLI, first switch to it
mumi current 52486
Then, you may apply the latest patchset in this issue (with sign off)
mumi am -- -s
Or, compose a reply to this issue
mumi compose
Or, send patches to this issue
mumi send-email *.patch