Re: [PATCH] gnu: hurd: Add Hurd Minimal.

[ludo@gnu.org (Ludovic Courtès)]

Manolis Ragkousis <manolis837@gmail.com> skribis:

> This patch adds Hurd minimal package which builds 'libihash.a' and a
> patch which disables linking of libihash against glibc. Next is the
> glibc/hurd package, but I will send it later/tomorrow cause there is
> something I need to ask the Hurd guys first.

Great, keep up the good work.  :-)

> From 137cb57dfb6b983521f758992a4424301acb4281 Mon Sep 17 00:00:00 2001
> From: Manolis Ragkousis <manolis837@gmail.com>
> Date: Sat, 24 May 2014 17:17:49 +0000
> Subject: [PATCH] gnu: hurd: Add Hurd Minimal.
>
> * gnu/packages/hurd.scm (hurd-minimal): New variable.
> * gnu/packages/patches/hurd-minimal.patch: New patch.
> * gnu-system.am (dist_patch_DATA): Add it.

[...]

> +(define-public hurd-minimal
> +  (package
> +    (name "hurd-minimal")
> +    (version "0.5")
> +    (source 
> +     (origin
> +       (method git-fetch)
> +       (uri (git-reference
> +             (url "git://git.savannah.gnu.org/hurd/hurd") 
> +             (commit "353fe33eb2481a14dc324c7c2e52b2032df86465")))

It may be OK currently if the version of hurd-headers does not match
that of the actual hurd, but we’ll have to be cautious.

Eventually we should consider changing hurd-headers to use the same
source (and hurd-minimal and hurd would just use (package-source
hurd-headers).)

> +    (outputs '("out"))

Can be omitted (it’s the same as the default value.)

> +    (arguments
> +     `(#:out-of-source? #t
> +       #:phases (alist-replace
> +                 'install
> +                 (lambda* (#:key outputs #:allow-other-keys)
> +                   (let ((out (assoc-ref outputs "out")))
> +                     ;; We need to copy libihash.a to the output folder manually,
> +                     ;; since there is no target for that in the makefile
> +                     (copy-recursively "libihash"
> +                                       (string-append out "/libihash"))

This creates
/gnu/store/.../libihash/{libihash.a,Makefile,ihash.c,ihash.o,...}.  Is
it really what’d intended?

I would expect that the intent is to create
/gnu/store/.../{lib/libihash.a,include/ihash.h}, no?

> +                   (lambda* (#:key inputs source #:allow-other-keys)
> +                     (let ((glibc (assoc-ref inputs "glibc-hurd-headers")))
> +                       (zero? (system* "autoreconf" "-vif"))
> +                       #t))

Can be simplified to:

  (lambda _
    (zero? (system* "autoreconf" "-vfi")))

Note that the effect of the trailing #t above was to ignore the result
of ‘system*’, which means ignoring errors when they occur.

> --- /dev/null
> +++ b/gnu/packages/patches/minimal-hurd.patch

Please add a line or two in the patch saying what it does, and linking
to the original discussion.

> @@ -0,0 +1,47 @@
> +diff --git a/configure.ac b/configure.ac
> +index ecabfdf..7ede6db 100644
> +--- a/configure.ac
> ++++ b/configure.ac
> +@@ -83,6 +83,13 @@ AC_PROG_INSTALL
> + AC_PROG_AWK
> + AC_PROG_SED
> + 
> ++if test "x$cross_compiling" = "xyes"; then
> ++   # It may be that we don't have a working libc yet, for instance
> ++   # because we're bootstrapping the cross-compilation tool chain.
> ++   # Thus, use this undocumented Autoconf macro designed for this.
> ++   AC_NO_EXECUTABLES
> ++   echo done
> ++fi

I gather that this works as advertised, but could you confirm by
replying to
<http://lists.gnu.org/archive/html/bug-hurd/2014-04/msg00139.html>?
That would allow us to push the patch upstream.

> +-# Check if libc contains getgrouplist and/or uselocale.
> +-AC_CHECK_FUNCS(getgrouplist uselocale)
> ++# We do not need to check if libc contains getgrouplist 
> ++# and/or uselocale for now.
> ++# AC_CHECK_FUNCS(getgrouplist uselocale)

I’ve just posted a patch for that, we’ll see.

> +-# Check for Sun RPC headers and library.
> ++# Check only for Sun RPC headers. We do not need the library yet.
> + AC_CHECK_HEADER([rpc/types.h], [HAVE_SUN_RPC=yes], [HAVE_SUN_RPC=no])
> +-AC_SEARCH_LIBS([clnt_create], [], [:], [HAVE_SUN_RPC=no])
> ++# AC_SEARCH_LIBS([clnt_create], [], [:], [HAVE_SUN_RPC=no])
> + AC_SUBST([HAVE_SUN_RPC])

What’s the problem that’s addressed here?

Thanks,
Ludo’.