diff mbox series

[bug#57146] gnu: polkit: Fix cross-compiling

Message ID 20220811231100.22090-1-andrewpatt7@gmail.com
State New
Headers show
Series [bug#57146] gnu: polkit: Fix cross-compiling | expand

Checks

Context Check Description
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

Andrew Patterson Aug. 11, 2022, 11:11 p.m. UTC 
The polkit package used to refer to docbook-xsl via inputs rather native inputs,
which obviously doesn't work when cross-compiling.
---
 gnu/packages/polkit.scm | 165 ++++++++++++++++++++--------------------
 1 file changed, 83 insertions(+), 82 deletions(-)

Comments

Mathieu Othacehe Aug. 12, 2022, 8:29 a.m. UTC | #1 
Hello Andrew,

> The polkit package used to refer to docbook-xsl via inputs rather native inputs,
> which obviously doesn't work when cross-compiling.

Thanks for working on that. It looks like this patch is breaking host
compilation:

--8<---------------cut here---------------start------------->8---
error: in phase 'fix-manpage-generation': uncaught exception:
wrong-type-arg "string-append" "Wrong type (expecting ~A): ~S" ("string" #f) (#f) 
phase `fix-manpage-generation' failed after 0.0 seconds
Backtrace:
           9 (primitive-load "/gnu/store/k2p4qxc23g871rm5q50bcx45f3m…")
In guix/build/gnu-build-system.scm:
    906:2  8 (gnu-build #:source _ #:outputs _ #:inputs _ #:phases . #)
--8<---------------cut here---------------end--------------->8---

Also you are doing a lot of things at the same time: fixing indentation,
introducing gexps, fixing cross-compilation. It would be great if you
could do that in separate patches to ease the review process.

Thanks,

Mathieu
Jean Pierre De Jesus DIAZ Aug. 12, 2022, 9:42 a.m. UTC | #2 
This issue is already solved:

https://issues.guix.gnu.org/56252

>Pushed to 'staging' in 7767d30f55, thanks!

Was fixed for both polkit-mozjs and polkit-duktape.

—
Jean-Pierre De Jesus DIAZ
Jean Pierre De Jesus DIAZ Aug. 12, 2022, 9:57 a.m. UTC | #3 
However, the patch to introduce G-Expressions is welcome as I did
only fix the inputs/native-inputs usage.

>+                            (let ((out (assoc-ref outputs "out")))

Can be removed probably, and replace `out' here:

>+                                     (string-append "sysconfdir=" out "/etc")
>+                                     (string-append "polkit_actiondir=" out
>+                                                    "/share/polkit-1/actions")

By #$output. But, rethinking it again, I would investigate if this phase
can be removed and instead use `#:make-flags' if possible directly as
make-flags can be specified as a G-Expression.

>+                          (lambda* (#:key outputs #:allow-other-keys)
>+                            (let ((out (assoc-ref outputs "out")))
>+                              (substitute* (find-files "." "Makefile.in")
>+                                (("@INTROSPECTION_GIRDIR@")
>+                                 (string-append out "/share/gir-1.0/"))
>+                                (("@INTROSPECTION_TYPELIBDIR@")
>+                                 (string-append out "/lib/girepository-1.0/"))))))

Same here. And then:

>+                          (lambda* (#:key outputs #:allow-other-keys)

Is just then:

--8<---------------cut here---------------start------------->8---
(lambda _
  ...)
--8<---------------cut here---------------start------------->8---

>+     (list #:configure-flags `(list "--sysconfdir=/etc" "--enable-man-pages"
>+                                    ;; Prevent ‘configure: error: cannot check for
>+                                    ;; file existence when cross compiling’.
>+                                    ,@(if (%current-target-system)
>+                                          '("--with-os-type=unknown")
>+                                          '()))

G-Expressions can be used here too, e.g:

--8<---------------cut here---------------start------------->8---
(list #:configure-flags #~(list "--sysconfdir=/etc"
                                "--enable-man-pages"
                               ;; Prevent ‘configure: error: cannot check for
                               ;; file existence when cross compiling’.
                               #$@(if (%current-target-system)
                                     '("--with-os-type=unknown")
                                     '()))
--8<---------------cut here---------------start------------->8---

—
Jean-Pierre De Jesus DIAZ


------- Original Message -------
On Friday, August 12th, 2022 at 11:42 AM, Jean Pierre De Jesus DIAZ <me@jeandudey.tech> wrote:


> This issue is already solved:
> 
> https://issues.guix.gnu.org/56252
> 
> > Pushed to 'staging' in 7767d30f55, thanks!
> 
> 
> Was fixed for both polkit-mozjs and polkit-duktape.
> 
> —
> Jean-Pierre De Jesus DIAZ
Andrew Patterson Aug. 12, 2022, 9:24 p.m. UTC | #4 
> Thanks for working on that. It looks like this patch is breaking 
> host
> compilation:
>
> error: in phase 'fix-manpage-generation': uncaught exception:
> wrong-type-arg "string-append" "Wrong type (expecting ~A): ~S" 
> ("string" #f) (#f) 
> phase `fix-manpage-generation' failed after 0.0 seconds
> Backtrace:
>            9 (primitive-load 
>            "/gnu/store/k2p4qxc23g871rm5q50bcx45f3m…")
> In guix/build/gnu-build-system.scm:
>     906:2  8 (gnu-build #:source _ #:outputs _ #:inputs _ 
>     #:phases . #)
Huh.  Looking at Jean Pierre's patch, apparently native-inputs 
only exists when cross-compiling, good to know.

> Also you are doing a lot of things at the same time: fixing 
> indentation,
> introducing gexps, fixing cross-compilation. It would be great 
> if you
> could do that in separate patches to ease the review process.
I'll make sure to keep that in mind in the future, sorry.
Andrew Patterson Aug. 12, 2022, 9:27 p.m. UTC | #5 
Thanks for the suggestions!  I'll try and work on this some.

Should I continue sending patches in this thread, or start a new 
one?
Jean Pierre De Jesus DIAZ Aug. 12, 2022, 10:48 p.m. UTC | #6 
On this same thread by replying to 57146@debbugs.gnu.org
-------- Original Message --------
On Aug 12, 2022, 23:27, Andrew Patterson wrote:

> Thanks for the suggestions! I'll try and work on this some. Should I continue sending patches in this thread, or start a new one? -- Andrew Patterson
Jean Pierre De Jesus DIAZ Aug. 14, 2022, 10:06 a.m. UTC | #7 
Don't know what happened that made my mail send an empty message :).

Yes you can send the updated patch to this same thread at:

57146@debbugs.gnu.org

—
Jean-Pierre De Jesus DIAZ



------- Original Message -------
On Saturday, August 13th, 2022 at 12:48 AM, Jean Pierre De Jesus DIAZ <me@jeandudey.tech> wrote:


> On this same thread by replying to 57146@debbugs.gnu.org
> -------- Original Message --------
> On Aug 12, 2022, 23:27, Andrew Patterson < andrewpatt7@gmail.com> wrote:
> 
> > 
> > Thanks for the suggestions! I'll try and work on this some. Should I continue sending patches in this thread, or start a new one? -- Andrew Patterson
Maxim Cournoyer Jan. 3, 2023, 10:23 p.m. UTC | #8 
Hi Andrew,

Andrew Patterson <andrewpatt7@gmail.com> writes:

> Thanks for the suggestions!  I'll try and work on this some.
>
> Should I continue sending patches in this thread, or start a new one?

Is there still something that needs fixing here, or can we close this?
Andrew Patterson Jan. 4, 2023, 4:19 a.m. UTC | #9 
I haven't needed to cross-compile any guix images recently, but 
`guix build --target=aarch64-linux-gnu polkit-duktape` on an 
x86_64 machine just completed successfully, so I don't think 
there's any issue still here?
Maxim Cournoyer Jan. 4, 2023, 5:16 a.m. UTC | #10 
Hi,

Andrew Patterson <andrewpatt7@gmail.com> writes:

> I haven't needed to cross-compile any guix images recently, but `guix
> build --target=aarch64-linux-gnu polkit-duktape` on an x86_64 machine
> just completed successfully, so I don't think there's any issue still
> here?

Thanks for checking.  I'll close the issue.  It can be reopen if
somethig w.r.t. cross-compilation and polkit is amiss.
diff mbox series

Patch

diff --git a/gnu/packages/polkit.scm b/gnu/packages/polkit.scm
index a75608cd43..dd0cf42e5d 100644
--- a/gnu/packages/polkit.scm
+++ b/gnu/packages/polkit.scm
@@ -9,6 +9,7 @@ 
 ;;; Copyright © 2018 Ricardo Wurmus <rekado@elephly.net>
 ;;; Copyright © 2021 Morgan Smith <Morgan.J.Smith@outlook.com>
 ;;; Copyright © 2021 Maxim Cournoyer <maxim.cournoyer@gmail.com>
+;;; Copyright © 2022 Andrew Patterson <andrewpatt7@gmail.com>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -27,6 +28,7 @@ 
 
 (define-module (gnu packages polkit)
   #:use-module ((guix licenses) #:select (lgpl2.0+))
+  #:use-module (guix gexp)
   #:use-module (guix packages)
   #:use-module (guix download)
   #:use-module (guix memoization)
@@ -55,92 +57,91 @@  (define-public polkit-mozjs
     (name "polkit")
     (version "0.120")
     (source (origin
-             (method url-fetch)
-             (uri (string-append
-                   "https://www.freedesktop.org/software/polkit/releases/"
-                   name "-" version ".tar.gz"))
-             (sha256
-              (base32
-               "00zfg9b9ivkcj2jcf5b92cpvvyljz8cmfwj86lkvy5rihnd5jypf"))
-             (patches (search-patches "polkit-configure-elogind.patch"
-                                      "polkit-CVE-2021-4034.patch"))
-             (modules '((guix build utils)))
-             (snippet
-              '(begin
-                 (use-modules (guix build utils))
-                 ;; Disable broken test.
-                 (substitute* "test/Makefile.in"
-                   (("SUBDIRS = mocklibc . polkit polkitbackend")
-                    "SUBDIRS = mocklibc . polkit"))
-                 ;; Disable a test that requires Python, D-Bus and a few
-                 ;; libraries and fails with "ERROR: timed out waiting for bus
-                 ;; process to terminate".
-                 (substitute* "test/polkitbackend/Makefile.am"
-                   (("TEST_PROGS \\+= polkitbackendjsauthoritytest-wrapper.py")
-                    ""))
-                 ;; Guix System's polkit
-                 ;; service stores actions under /etc/polkit-1/actions.
-                 (substitute* "src/polkitbackend/polkitbackendinteractiveauthority.c"
-                   (("PACKAGE_DATA_DIR \"/polkit-1/actions\"")
-                    "PACKAGE_SYSCONF_DIR \"/polkit-1/actions\""))
-                 ;; Set the setuid helper's real location.
-                 (substitute* "src/polkitagent/polkitagentsession.c"
-                   (("PACKAGE_PREFIX \"/lib/polkit-1/polkit-agent-helper-1\"")
-                    "\"/run/setuid-programs/polkit-agent-helper-1\""))))))
+              (method url-fetch)
+              (uri (string-append
+                    "https://www.freedesktop.org/software/polkit/releases/"
+                    name "-" version ".tar.gz"))
+              (sha256
+               (base32
+                "00zfg9b9ivkcj2jcf5b92cpvvyljz8cmfwj86lkvy5rihnd5jypf"))
+              (patches (search-patches "polkit-configure-elogind.patch"
+                                       "polkit-CVE-2021-4034.patch"))
+              (modules '((guix build utils)))
+              (snippet '(begin
+                          (use-modules (guix build utils))
+                          ;; Disable broken test.
+                          (substitute* "test/Makefile.in"
+                            (("SUBDIRS = mocklibc . polkit polkitbackend")
+                             "SUBDIRS = mocklibc . polkit"))
+                          ;; Disable a test that requires Python, D-Bus and a few
+                          ;; libraries and fails with "ERROR: timed out waiting for bus
+                          ;; process to terminate".
+                          (substitute* "test/polkitbackend/Makefile.am"
+                            (("TEST_PROGS \\+= polkitbackendjsauthoritytest-wrapper.py")
+                             ""))
+                          ;; Guix System's polkit
+                          ;; service stores actions under /etc/polkit-1/actions.
+                          (substitute* "src/polkitbackend/polkitbackendinteractiveauthority.c"
+                            (("PACKAGE_DATA_DIR \"/polkit-1/actions\"")
+                             "PACKAGE_SYSCONF_DIR \"/polkit-1/actions\""))
+                          ;; Set the setuid helper's real location.
+                          (substitute* "src/polkitagent/polkitagentsession.c"
+                            (("PACKAGE_PREFIX \"/lib/polkit-1/polkit-agent-helper-1\"")
+                             "\"/run/setuid-programs/polkit-agent-helper-1\""))))))
     (build-system gnu-build-system)
-    (inputs
-     (list expat linux-pam elogind mozjs nspr))
-    (propagated-inputs
-     (list glib)) ; required by polkit-gobject-1.pc
-    (native-inputs
-     (list pkg-config
-           `(,glib "bin")                         ;for glib-mkenums
-           intltool
-           gobject-introspection
-           libxslt                                ;for man page generation
-           docbook-xsl))                          ;for man page generation
+    (inputs (list expat linux-pam elogind mozjs nspr))
+    (propagated-inputs (list glib)) ;required by polkit-gobject-1.pc
+    (native-inputs (list pkg-config
+                         `(,glib "bin") ;for glib-mkenums
+                         intltool
+                         gobject-introspection
+                         libxslt ;for man page generation
+                         docbook-xsl)) ;for man page generation
     (arguments
-     `(#:configure-flags '("--sysconfdir=/etc"
-                           "--enable-man-pages"
-                           ;; Prevent ‘configure: error: cannot check for
-                           ;; file existence when cross compiling’.
-                           ,@(if (%current-target-system)
-                                 '("--with-os-type=unknown")
-                                 '()))
-       #:phases
-       (modify-phases %standard-phases
-         (add-after
-          'unpack 'fix-introspection-install-dir
-          (lambda* (#:key outputs #:allow-other-keys)
-            (let ((out (assoc-ref outputs "out")))
-              (substitute* (find-files "." "Makefile.in")
-                (("@INTROSPECTION_GIRDIR@")
-                 (string-append out "/share/gir-1.0/"))
-                (("@INTROSPECTION_TYPELIBDIR@")
-                 (string-append out "/lib/girepository-1.0/"))))))
-         (add-after 'unpack 'fix-manpage-generation
-           (lambda* (#:key inputs #:allow-other-keys)
-             (let ((xsldoc (string-append (assoc-ref inputs "docbook-xsl")
-                                          "/xml/xsl/docbook-xsl-"
-                                          ,(package-version docbook-xsl))))
-               (substitute* '("docs/man/Makefile.am" "docs/man/Makefile.in")
-                 (("http://docbook.sourceforge.net/release/xsl/current")
-                  xsldoc)))))
-         (replace
-          'install
-          (lambda* (#:key outputs (make-flags '()) #:allow-other-keys)
-            ;; Override sysconfdir during "make install", to avoid attempting
-            ;; to install in /etc, and to instead install the skeletons in the
-            ;; output directory.
-            (let ((out (assoc-ref outputs "out")))
-             (apply invoke "make" "install"
-                           (string-append "sysconfdir=" out "/etc")
-                           (string-append "polkit_actiondir="
-                                          out "/share/polkit-1/actions")
-                           make-flags)))))))
+     (list #:configure-flags `(list "--sysconfdir=/etc" "--enable-man-pages"
+                                    ;; Prevent ‘configure: error: cannot check for
+                                    ;; file existence when cross compiling’.
+                                    ,@(if (%current-target-system)
+                                          '("--with-os-type=unknown")
+                                          '()))
+           #:phases #~(modify-phases %standard-phases
+                        (add-after 'unpack 'fix-introspection-install-dir
+                          (lambda* (#:key outputs #:allow-other-keys)
+                            (let ((out (assoc-ref outputs "out")))
+                              (substitute* (find-files "." "Makefile.in")
+                                (("@INTROSPECTION_GIRDIR@")
+                                 (string-append out "/share/gir-1.0/"))
+                                (("@INTROSPECTION_TYPELIBDIR@")
+                                 (string-append out "/lib/girepository-1.0/"))))))
+                        (add-after 'unpack 'fix-manpage-generation
+                          (lambda* (#:key native-inputs #:allow-other-keys)
+                            (let ((xsldoc (string-append (assoc-ref
+                                                          native-inputs
+                                                          "docbook-xsl")
+                                           "/xml/xsl/docbook-xsl-"
+                                           #$(package-version docbook-xsl))))
+                              (substitute* '("docs/man/Makefile.am"
+                                             "docs/man/Makefile.in")
+                                (("http://docbook.sourceforge.net/release/xsl/current")
+                                 xsldoc)))))
+                        (replace 'install
+                          (lambda* (#:key outputs
+                                    (make-flags '()) #:allow-other-keys)
+                            ;; Override sysconfdir during "make install", to avoid attempting
+                            ;; to install in /etc, and to instead install the skeletons in the
+                            ;; output directory.
+                            (let ((out (assoc-ref outputs "out")))
+                              (apply invoke
+                                     "make"
+                                     "install"
+                                     (string-append "sysconfdir=" out "/etc")
+                                     (string-append "polkit_actiondir=" out
+                                                    "/share/polkit-1/actions")
+                                     make-flags)))))))
     (home-page "https://www.freedesktop.org/wiki/Software/polkit/")
     (synopsis "Authorization API for privilege management")
-    (description "Polkit is an application-level toolkit for defining and
+    (description
+     "Polkit is an application-level toolkit for defining and
 handling the policy that allows unprivileged processes to speak to
 privileged processes.  It is a framework for centralizing the decision
 making process with respect to granting access to privileged operations