diff mbox series

[bug#72943] gnu: gd: Requires.private to propagated inputs

Message ID 87v7zeezr1.fsf@protonmail.com
State New
Headers show
Series [bug#72943] gnu: gd: Requires.private to propagated inputs | expand

Commit Message

Rutherther Sept. 2, 2024, 5:23 p.m. UTC 
Hello Carlo,

thank you for finding that!
That explains why this issue was caused in the first place,
I was wondering how this could've been unspotted till now.

It seems much better to me to not use propagated-inputs whenever possible,
so I prefer your solution to this one. However, I do wonder about this: other packages that
have Requires.private do provide the libraries in these as propagated-inputs. Also
the cookbook shows to do this with such libraries. What is the correct approach here
then? Maybe every package that has Requires.private could be patched like this, possibly
somehow automatically instead of manual patches?

I am quite new here, this is my first patch. So I am not really sure how to go about this.
Will you open a new issue with your patch, and maintainers will decide patch from which
issue to use? Or is it enough it's in this issue?

I am sending an updated patch with a comment for why specifying
propagates-inputs would be necessary in the meantime,
as Tobias suggested me in a review in #guix IRC channel.

---
 gnu/packages/gd.scm | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Carlo Zancanaro Sept. 3, 2024, 12:19 a.m. UTC | #1 
On Mon, Sep 02 2024, Rutherther via Guix-patches via wrote:
> It seems much better to me to not use propagated-inputs whenever possible,
> so I prefer your solution to this one. However, I do wonder about this: other packages that
> have Requires.private do provide the libraries in these as propagated-inputs. Also
> the cookbook shows to do this with such libraries. What is the correct approach here
> then? Maybe every package that has Requires.private could be patched like this, possibly
> somehow automatically instead of manual patches?

Ah, I didn't look into it, so I didn't know that other packages in the
same situation just propagate their inputs. If that's more consistent
with the rest of Guix's packages then it would probably be better to do
that than restoring the patch.

> I am quite new here, this is my first patch. So I am not really sure how to go about this.
> Will you open a new issue with your patch, and maintainers will decide patch from which
> issue to use? Or is it enough it's in this issue?

I wasn't planning to open a new issue. Ideally I think we would agree on
which change is more appropriate, then someone with commit access would
apply it.

If this problem is solved elsewhere in Guix with propagated-inputs, then
I think your change is a better idea than mine.

This still leaves the issue of the failing tests, though. I investigated
a bit and looking at [1] and [2], I believe the issue is that our
version of libgd doesn't support the BICUBIC interpolation method.

I'm rebuilding PHP now with those tests removed. I'll send through some
patches soon.

Carlo

[1]: https://github.com/php/php-src/issues/11252
[2]: https://github.com/libgd/libgd/issues/847
diff mbox series

Patch

diff --git a/gnu/packages/gd.scm b/gnu/packages/gd.scm
index 98d34cfa71..1380493c44 100644
--- a/gnu/packages/gd.scm
+++ b/gnu/packages/gd.scm
@@ -85,7 +85,8 @@  (define-public gd
                                          "\\.la$")))))))
     (native-inputs
      (list pkg-config))
-    (inputs
+    ;; These libraries are in 'Requires.private' in libgd.pc.
+    (propagated-inputs
      (list fontconfig
            freetype
            libjpeg-turbo