[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
?