| Message ID | f6f3b6c5b58a2d16b3ccb947bbaffa65cac48b1a.1708618218.git.maxim.cournoyer@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | Replace pkg-config with pkgconf to reduce propagation / Inkscape updates | expand |
Hi!
Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis:
> This switches the default pkg-config implementation used in Guix to pkgconf.
I didn’t follow discussions and I learned about ‘pkgconf’ only recently.
I’m afraid that adding this to ‘core-updates’ would further postpone its
merger, which was already being discussed beginning of January.
Should it instead be done on a separate branch?
(Aside: where should I read about the rationale of the pkg-config ->
pkgconf move?)
Ludo’.
Hi Ludovic, Ludovic Courtès <ludo@gnu.org> writes: > Hi! > > Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis: > >> This switches the default pkg-config implementation used in Guix to pkgconf. > > I didn’t follow discussions and I learned about ‘pkgconf’ only recently. > I’m afraid that adding this to ‘core-updates’ would further postpone its > merger, which was already being discussed beginning of January. Was it in a good shape to be merged back then? It seems to me we are still ironing things fairly low in the tree such as a glibc upgrade by jpoiret (for security reasons), which leaves the opportunity to tackle well tested changes to it, which this one is. > Should it instead be done on a separate branch? I've manually rebuilt a good chunk of the world (mpv and plasmatube) using pkgconf, and haven't seen any breakage caused by it. You can try do build these patches on top of current core-updates on the hydra-guix-129 machine, which should still have it in its store. > (Aside: where should I read about the rationale of the pkg-config -> > pkgconf move?) It all started with f3fdb4e041cb5740ba0b38b9ad017571f8414d33 ("gnu: mpv: Propagate most libraries."), which was probably triggered by mpv newly using Requires.static fields in their pkg-config files (Meson knows to do that). Looking for nicer alternatives to propagating these, pkgconf picked my interested as it's supposed to fix some of the pkg-config long time "bugs" that upstream is not too keen to fix (for backward compatibility, I think), such as this Requires.static behavior: Current guix, with above f3fdb4e041cb5740ba0b38b9ad017571f8414d33 commit reverted: --8<---------------cut here---------------start------------->8--- $ ./pre-inst-env guix shell --pure pkg-config mpv \ -- pkg-config --print-errors --short-errors --exists mpv Package 'wayland-client', required by 'mpv', not found $ echo $? 1 --8<---------------cut here---------------end--------------->8--- Compare with: --8<---------------cut here---------------start------------->8--- $ ./pre-inst-env guix shell --pure pkg-config pkgconf mpv \ -- pkgconf --print-errors --short-errors --exists mpv $ echo $? 0 --8<---------------cut here---------------end--------------->8--- The above demonstrates that pkgconf's behavior is to consider *.private fields only when provided the --static option, which is what we want. I hope this helps understanding the rationale.
Hi, Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis: > Ludovic Courtès <ludo@gnu.org> writes: > >> Hi! >> >> Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis: >> >>> This switches the default pkg-config implementation used in Guix to pkgconf. >> >> I didn’t follow discussions and I learned about ‘pkgconf’ only recently. >> I’m afraid that adding this to ‘core-updates’ would further postpone its >> merger, which was already being discussed beginning of January. > > Was it in a good shape to be merged back then? No, it wasn’t: <https://lists.gnu.org/archive/html/guix-devel/2024-01/msg00096.html>. But a lot of work has gone into the branch since that time that goes way beyond “fixing problems”. [...] > I've manually rebuilt a good chunk of the world (mpv and plasmatube) > using pkgconf, and haven't seen any breakage caused by it. You can try > do build these patches on top of current core-updates on the > hydra-guix-129 machine, which should still have it in its store. I won’t try, I’m just saying from experience that the “just one last tiny change” strategy never converges. :-) >> (Aside: where should I read about the rationale of the pkg-config -> >> pkgconf move?) [...] > The above demonstrates that pkgconf's behavior is to consider *.private > fields only when provided the --static option, which is what we want. > > I hope this helps understanding the rationale. It does, and it looks like a nice improvement. Thanks for explaining! Ludo’.
Hello, Ludovic Courtès <ludo@gnu.org> writes: >> I've manually rebuilt a good chunk of the world (mpv and plasmatube) >> using pkgconf, and haven't seen any breakage caused by it. You can try >> do build these patches on top of current core-updates on the >> hydra-guix-129 machine, which should still have it in its store. > > I won’t try, I’m just saying from experience that the “just one last > tiny change” strategy never converges. :-) > >>> (Aside: where should I read about the rationale of the pkg-config -> >>> pkgconf move?) > > [...] > >> The above demonstrates that pkgconf's behavior is to consider *.private >> fields only when provided the --static option, which is what we want. >> >> I hope this helps understanding the rationale. > > It does, and it looks like a nice improvement. Thanks for explaining! Great! I've now merged this series in core-updates, after consulting with Josselin, since we are going to need a world rebuild anyway.
diff --git a/gnu/packages/commencement.scm b/gnu/packages/commencement.scm index 3e5e21ca03..e9474a797e 100644 --- a/gnu/packages/commencement.scm +++ b/gnu/packages/commencement.scm @@ -3346,13 +3346,13 @@ (define with-boot5 with-boot4) (define (make-gnu-make-final) "Compute the final GNU Make, which uses the final Guile." + ;; Avoid a circular dependency by creating a new bootstrap pkg-config + ;; variant. (let ((pkg-config (package - (inherit %pkg-config) ;the native pkg-config - (inputs `(("guile" ,guile-final) - ,@(%boot5-inputs))) - (arguments - `(#:implicit-inputs? #f - ,@(package-arguments %pkg-config)))))) + ;; Refer to %pkgconf-as-pkg-config instead of + ;; pkgconf-as-pkg-config to ensure native package is + ;; used. + (inherit (with-boot5 %pkgconf-as-pkg-config))))) (package (inherit (package-with-bootstrap-guile gnu-make)) (inputs `(("guile" ,guile-final) @@ -3362,7 +3362,6 @@ (define (make-gnu-make-final) `(#:implicit-inputs? #f ,@(package-arguments gnu-make)))))) - (define coreutils-final ;; The final Coreutils. Treat them specially because some packages, such as ;; Findutils, keep a reference to the Coreutils they were built with. diff --git a/gnu/packages/pkg-config.scm b/gnu/packages/pkg-config.scm index e8d63be3d7..934449aad2 100644 --- a/gnu/packages/pkg-config.scm +++ b/gnu/packages/pkg-config.scm @@ -31,7 +31,9 @@ (define-module (gnu packages pkg-config) #:use-module (gnu packages bash) #:use-module (gnu packages check) #:use-module (guix memoization) - #:export (pkg-config + #:use-module (srfi srfi-1) + #:export (old-pkg-config ;the original + pkg-config ;alias for pkgconf-as-pkg-config pkgconf pkgconf-as-pkg-config)) @@ -89,7 +91,9 @@ (define-public %pkg-config it can be used for defining the location of documentation tools, for instance."))) -(define-public %pkgconf +;;; This is the package exposed to the CLI, to ease updates via 'guix +;;; refresh'. +(define-public %pkgconf-with-tests (package (name "pkgconf") (version "2.1.0") @@ -119,6 +123,13 @@ (define-public %pkgconf pkgconf.") (license isc))) +;;; This is the minimal, untested variant used to avoid circular dependencies. +(define-public %pkgconf + (hidden-package + (package/inherit %pkgconf-with-tests + (arguments (list #:tests? #f)) + (native-inputs '())))) + (define-public %pkgconf-as-pkg-config (package/inherit %pkgconf (name "pkgconf-as-pkg-config") @@ -145,7 +156,8 @@ (define-public %pkgconf-as-pkg-config (string-append #$output "/share/aclocal")))))))) (native-inputs '()) (inputs (list %pkgconf)) - (propagated-inputs '()))) + (propagated-inputs '()) + (properties (alist-delete 'hidden? (package-properties %pkgconf))))) ;;; @@ -220,7 +232,7 @@ (define pkgconf-as-pkg-config-for-target ;; These are a hacks for automatically choosing the native or the cross ;; `pkg-config' depending on whether it's being used in a cross-build ;; environment or not. -(define-syntax pkg-config +(define-syntax old-pkg-config (identifier-syntax (pkg-config-for-target (%current-target-system)))) (define-syntax pkgconf @@ -230,6 +242,11 @@ (define-syntax pkgconf-as-pkg-config (identifier-syntax (pkgconf-as-pkg-config-for-target (%current-target-system)))) +;;; This alias is to ensure we use pkgconf instead of pkg-config across Guix, +;;; which includes welcome refinements such as proper handling of the +;;; Requires.private field. +(define pkg-config pkgconf-as-pkg-config) + ;;; ;;; pkg-config packages for native use (build-time only).