Re: [PATCH 1/4] gnu: base: Add glibc-hurd and hurd-minimal.

[Mark H Weaver <mhw@netris.org>]

Manolis Ragkousis <manolis837@gmail.com> writes:

> There was only a minor change here, to work around the error I told
> you in a previous mail.
>
> http://lists.gnu.org/archive/html/guix-devel/2015-02/msg00156.html
>
> From c9fd013956718c4d3178608b3eca6392b6f5cea3 Mon Sep 17 00:00:00 2001
> From: Manolis Ragkousis <manolis837@gmail.com>
> Date: Sat, 27 Sep 2014 20:07:19 +0300
> Subject: [PATCH 1/4] gnu: base: Add glibc-hurd and hurd-minimal.
>
> Reinstates commit bc046a94, among other things.

What do you mean by "reinstates"?  I'm not sure there's a need to
reference this obsolete commit.

[...]
> diff --git a/gnu/packages/base.scm b/gnu/packages/base.scm
> index be33cb2..556eb47 100644
> --- a/gnu/packages/base.scm
> +++ b/gnu/packages/base.scm
> @@ -4,6 +4,7 @@
>  ;;; Copyright © 2012 Nikita Karetnikov <nikita@karetnikov.org>
>  ;;; Copyright © 2014 Mark H Weaver <mhw@netris.org>
>  ;;; Copyright © 2014 Alex Kost <alezost@gmail.com>
> +;;; Copyright © 2014 Manolis Fragkiskos Ragkousis <manolis837@gmail.com>

I would add 2015 now also.

>  ;;;
>  ;;; This file is part of GNU Guix.
>  ;;;
> @@ -32,9 +33,13 @@
>    #:use-module (gnu packages perl)
>    #:use-module (gnu packages linux)
>    #:use-module (gnu packages texinfo)
> +  #:use-module (gnu packages gettext)
> +  #:use-module (gnu packages hurd)
>    #:use-module (gnu packages pkg-config)
> +  #:use-module (guix utils)
>    #:use-module (guix packages)
>    #:use-module (guix download)
> +  #:use-module (guix git-download)
>    #:use-module (guix build-system gnu))
>  
>  ;;; Commentary:
> @@ -520,6 +525,105 @@ with the Linux kernel.")
>     (license lgpl2.0+)
>     (home-page "http://www.gnu.org/software/libc/")))
>  
> +(define-public glibc/hurd
> +  (package (inherit glibc)
> +    (name "glibc-hurd")
> +    (version "2.18")
> +    (source (origin
> +              (method git-fetch)
> +              (uri (git-reference
> +                    (url "git://git.sv.gnu.org/hurd/glibc")
> +                    (commit "a9d8d3808f18de4da9b587e9bdfb6cca4704344b")))
> +              (sha256
> +               (base32
> +                "0jmczzdyps5syhrqyf7lgl3h77br8s74qw0417jp8b4f29ks7pbz"))
> +              (file-name (string-append name "-" version))
> +              (patches (map search-patch
> +                            '("glibc-make-4.0.patch"
> +                              "glibc-hurd-extern-inline.patch")))))
> +
> +    ;; Libc provides <hurd.h>, which includes a bunch of Hurd and Mach headers,
> +    ;; so both should be propagated.
> +    (propagated-inputs `(("gnumach-headers" ,gnumach-headers)
> +                         ("hurd-headers" ,hurd-headers)
> +                         ("hurd-minimal" ,hurd-minimal)))
> +    (native-inputs
> +     `(,@(package-native-inputs glibc)
> +       ("patch/libpthread-patch" ,(search-patch "libpthread-glibc-preparation.patch"))
> +       ("mig" ,mig)
> +       ("perl" ,perl)
> +       ("libpthread" ,(origin
> +                        (method git-fetch)
> +                        (uri (git-reference
> +                              (url "git://git.sv.gnu.org/hurd/libpthread")
> +                              (commit "f517024dce3e21c525a7b634eab61302d6b99150")))
> +                        (sha256
> +                         (base32
> +                          "0yqfm1hfqlyjzqv3mgf9a3mh4qxx1mqkzn5xiac2vlvji8nns35y"))
> +                        (file-name "libpthread")))))
> +
> +    (arguments
> +     `(#:modules ((guix build utils)
> +                  (guix build gnu-build-system))

This #:modules argument is already the default, and it's not overridden
by the inherited 'glibc' package, so I think it's not needed here.

> +       ,@(substitute-keyword-arguments (package-arguments glibc)
> +           ((#:configure-flags cf)
> +            `(append (list "--host=i686-pc-gnu"
> +
> +                           ;; nscd fails to build for GNU/Hurd:
> +                           ;; <https://lists.gnu.org/archive/html/bug-hurd/2014-07/msg00006.html>.
> +                           ;; Disable it.
> +                           "--disable-nscd")))

Did you intend to omit 'cf', the inherited configure-flags from 'glibc'?
Also, you're passing only one argument to 'append', making it a no-op.

Looking at the configure-flags for 'glibc', most of them look desirable
on Hurd, but some of them don't.  The ones that look desirable are:

  "--enable-add-ons"
  "--sysconfdir=/etc"
  "libc_cv_localedir=/run/current-system/locale"
  (string-append "BASH_SHELL="
                 (assoc-ref %build-inputs "bash")
                 "/bin/bash")
  "libc_cv_ssp=no"

The ones that we probably want to remove are:

  (string-append "--with-headers="
                 (assoc-ref %build-inputs "linux-headers")
                 "/include")
  "--enable-kernel=2.6.32"

How about this: (untested)

--8<---------------cut here---------------start------------->8---
           ((#:configure-flags original-configure-flags)
            `(list "--host=i686-pc-gnu"

                   ;; nscd fails to build for GNU/Hurd:
                   ;; <https://lists.gnu.org/archive/html/bug-hurd/2014-07/msg00006.html>.
                   ;; Disable it.
                   "--disable-nscd"

                   ,@(filter (lambda (flag)
                               (not (or (string-prefix? "--with-headers=" flag)
                                        (string-prefix? "--enable-kernel=" flag))))
                             original-configure-flags)))
--8<---------------cut here---------------end--------------->8---

> +           ((#:phases phases)
> +            `(alist-cons-after
> +              'unpack 'prepare-libpthread
> +              (lambda* (#:key inputs #:allow-other-keys)
> +                (copy-recursively (assoc-ref inputs "libpthread") "libpthread")
> +
> +                (system* "patch" "-p1" "-i"
> +                         (assoc-ref %build-inputs "patch/libpthread-patch"))

Better to use 'inputs' not '%build-inputs' here.  Also, you should also
pass "--force" to 'patch', and check the result status for errors.

--8<---------------cut here---------------start------------->8---
                (unless (zero? (system*
                                "patch" "--force" "-p1" "-i"
                                (assoc-ref inputs "patch/libpthread-patch")))
                  (error "patch/libpthread-patch failed"))
--8<---------------cut here---------------end--------------->8---

> +
> +                ;; Make the file writable.
> +                (chmod "bits/pthreadtypes.h" #o666)

Making it world-writable seems excessive.  How about #o644 ?

> +                (copy-recursively "libpthread/sysdeps/generic/bits" "bits"))

Add a #t to the end here, since 'copy-recursively' doesn't return
anything in particular.

--8<---------------cut here---------------start------------->8---
                (copy-recursively "libpthread/sysdeps/generic/bits" "bits")
                #t)
--8<---------------cut here---------------end--------------->8---

Can you make these changes, see if things still work, and then post an
updated patch?

Thanks for working on this!

      Mark