[PATCH] gnu: gettext: Enable "xgettext --language=glade"

[PATCH] gnu: gettext: Enable "xgettext --language=glade"

[Andreas Enge]

[-- Attachment #1: Type: text/plain, Size: 390 bytes --]

As discussed yesterday on irc, the current gettext does not handle glade,
as it is not linked with expat. This causes problems when patching pspp,
for instance. The attached patch solves the problem and compiles on master.
Is it okay to push it to core-updates, to avoid disruption on master?
Since guile currently does not compile in core-updates, I cannot test
the patch there.

Andreas


[-- Attachment #2: 0001-gnu-gettext-Enable-xgettext-language-glade.patch --]
[-- Type: text/plain, Size: 2098 bytes --]

From a1c81413937dbaae283a1fb4ea9d80c13e62dd90 Mon Sep 17 00:00:00 2001
From: Andreas Enge <andreas@enge.fr>
Date: Sun, 17 Nov 2013 13:22:15 +0100
Subject: [PATCH] gnu: gettext: Enable "xgettext --language=glade".

* gnu/packages/gettext.scm (gettext): Add input expat, explicitly link with it.
---
 gnu/packages/gettext.scm | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/gnu/packages/gettext.scm b/gnu/packages/gettext.scm
index 07d2b0d..6436e26 100644
--- a/gnu/packages/gettext.scm
+++ b/gnu/packages/gettext.scm
@@ -17,11 +17,12 @@
 ;;; along with GNU Guix.  If not, see <http://www.gnu.org/licenses/>.
 
 (define-module (gnu packages gettext)
-  #:use-module (guix licenses)
+  #:use-module ((guix licenses) #:select (gpl3))
   #:use-module (gnu packages)
   #:use-module (guix packages)
   #:use-module (guix download)
-  #:use-module (guix build-system gnu))
+  #:use-module (guix build-system gnu)
+  #:use-module (gnu packages xml))
 
 (define-public gettext
   (package
@@ -35,8 +36,16 @@
               (base32
                "0j7rp56c61j4k1bz1xdc041hzv7186yyzhbp95fmc0zq7l2c3wrn"))))
     (build-system gnu-build-system)
+    (inputs
+     `(("expat" ,expat)))
     (arguments
      `(#:phases (alist-cons-before
+                 'configure 'link-expat
+                 (lambda _
+                   (substitute* "gettext-tools/configure"
+                     (("LIBEXPAT=\"-ldl\"") "LIBEXPAT=\"-ldl -lexpat\"")
+                     (("LTLIBEXPAT=\"-ldl\"") "LTLIBEXPAT=\"-ldl -lexpat\"")))
+                (alist-cons-before
                  'check 'patch-tests
                  (lambda* (#:key inputs #:allow-other-keys)
                    (let ((bash (which "sh")))
@@ -48,7 +57,7 @@
                                               "posix_spawn")
                        (("/bin/sh")
                         bash))))
-                 %standard-phases)))
+                 %standard-phases))))
     (home-page "http://www.gnu.org/software/gettext/")
     (synopsis "Tools and documentation for translation")
     (description
-- 
1.8.4

Re: [PATCH] gnu: gettext: Enable "xgettext --language=glade"

[Ludovic Courtès]

Andreas Enge <andreas@enge.fr> skribis:

> As discussed yesterday on irc, the current gettext does not handle glade,
> as it is not linked with expat. This causes problems when patching pspp,
> for instance. The attached patch solves the problem and compiles on master.
> Is it okay to push it to core-updates, to avoid disruption on master?
> Since guile currently does not compile in core-updates, I cannot test
> the patch there.

The patch looks good to me.

I’m currently working on a patch to fix the libgcc_s issue that breaks
Guile in core-updates.  If you don’t mind, I’ll push both your patch and
mine at once, to avoid overloading Hydra.

Thanks!

Ludo’.

Re: [PATCH] gnu: gettext: Enable "xgettext --language=glade"

[Ludovic Courtès]

Andreas Enge <andreas@enge.fr> skribis:

>       `(#:phases (alist-cons-before
> +                 'configure 'link-expat
> +                 (lambda _
> +                   (substitute* "gettext-tools/configure"
> +                     (("LIBEXPAT=\"-ldl\"") "LIBEXPAT=\"-ldl -lexpat\"")
> +                     (("LTLIBEXPAT=\"-ldl\"") "LTLIBEXPAT=\"-ldl -lexpat\"")))

Actually, it seems to be solvable without adding the dependency:

--8<---------------cut here---------------start------------->8---
$ xgettext -L glade /dev/null 
xgettext: Language "glade" is not supported. xgettext relies on expat.
          This version was built without expat.

$ LD_LIBRARY_PATH=$(guix build expat)/lib xgettext -L glade /dev/null 
xgettext: /dev/null:1:1: no element found
--8<---------------cut here---------------end--------------->8---

(The error message is misleading because it’s not a build-time issue.)

Would it work for you to just use that LD_LIBRARY_PATH trick where it’s
needed?

That way we avoid making gettext “fatter”.  I suppose we’ll seldom need
to do that trick anyway.

WDYT?

Ludo’.

Re: [PATCH] gnu: gettext: Enable "xgettext --language=glade"

[Andreas Enge]

On Mon, Nov 18, 2013 at 12:19:24AM +0100, Ludovic Courtès wrote:
> Actually, it seems to be solvable without adding the dependency:
> 
> --8<---------------cut here---------------start------------->8---
> $ xgettext -L glade /dev/null 
> xgettext: Language "glade" is not supported. xgettext relies on expat.
>           This version was built without expat.
> 
> $ LD_LIBRARY_PATH=$(guix build expat)/lib xgettext -L glade /dev/null 
> xgettext: /dev/null:1:1: no element found
> --8<---------------cut here---------------end--------------->8---

I know, that was one of the things we tested early on; but...

> Would it work for you to just use that LD_LIBRARY_PATH trick where it’s
> needed?
> That way we avoid making gettext “fatter”.  I suppose we’ll seldom need
> to do that trick anyway.

... might there not be users who want a fully functional xgettext? Now that
we found a simple solution, I am rather in favour of implementing it.
Not to mention that this solves the issue once and for all, and we do not
need to remember the LD_LIBRARY_PATH trick later on...

Andreas

Re: [PATCH] gnu: gettext: Enable "xgettext --language=glade"

[Ludovic Courtès]

Andreas Enge <andreas@enge.fr> skribis:

> On Mon, Nov 18, 2013 at 12:19:24AM +0100, Ludovic Courtès wrote:
>> Actually, it seems to be solvable without adding the dependency:
>> 
>> --8<---------------cut here---------------start------------->8---
>> $ xgettext -L glade /dev/null 
>> xgettext: Language "glade" is not supported. xgettext relies on expat.
>>           This version was built without expat.
>> 
>> $ LD_LIBRARY_PATH=$(guix build expat)/lib xgettext -L glade /dev/null 
>> xgettext: /dev/null:1:1: no element found
>> --8<---------------cut here---------------end--------------->8---
>
> I know, that was one of the things we tested early on; but...

OK, sorry for stating the obvious then (apologies for not following the
discussion closely enough!).

>> Would it work for you to just use that LD_LIBRARY_PATH trick where it’s
>> needed?
>> That way we avoid making gettext “fatter”.  I suppose we’ll seldom need
>> to do that trick anyway.
>
> ... might there not be users who want a fully functional xgettext? Now that
> we found a simple solution, I am rather in favour of implementing it.
> Not to mention that this solves the issue once and for all, and we do not
> need to remember the LD_LIBRARY_PATH trick later on...

Yeah, makes sense to me; so feel free to commit now.

Are there other optional dependencies that are dlopen’d, or is it the
only one?  It’s OK if it’s just expat, but we wouldn’t want to add
bigger things.

Thanks,
Ludo’.

Re: [PATCH] gnu: gettext: Enable "xgettext --language=glade"

[Andreas Enge]

On Mon, Nov 18, 2013 at 10:17:36AM +0100, Ludovic Courtès wrote:
> Yeah, makes sense to me; so feel free to commit now.

Would you still like to commit in conjunction with your gcc patch? There is
no particular hurry.

> Are there other optional dependencies that are dlopen’d, or is it the
> only one?  It’s OK if it’s just expat, but we wouldn’t want to add
> bigger things.

Not that I know of, this just came up as a problem... But anyway, things
should not become bigger, right? If I understood correctly, the only effect
of adding "-lexpat" will be to encode an rpath (and to need expat as an
input to gettext, of course).

Andreas

Re: [PATCH] gnu: gettext: Enable "xgettext --language=glade"

[Andreas Enge]

On Mon, Nov 18, 2013 at 10:58:20AM +0100, Andreas Enge wrote:
> Would you still like to commit in conjunction with your gcc patch? There is
> no particular hurry.

Actually, the patch collides with your gettext update and requires manual
intervention. I will push it then, because anyway with guile broken, nothing
will be compiled on hydra, so we need not worry about its load.

Andreas

Re: [PATCH] gnu: gettext: Enable "xgettext --language=glade"

[Ludovic Courtès]

Andreas Enge <andreas@enge.fr> skribis:

> On Mon, Nov 18, 2013 at 10:17:36AM +0100, Ludovic Courtès wrote:
>> Yeah, makes sense to me; so feel free to commit now.
>
> Would you still like to commit in conjunction with your gcc patch? There is
> no particular hurry.

No, you can do it separately now (my GCC patch from yesterday didn’t
quite make it.)

>> Are there other optional dependencies that are dlopen’d, or is it the
>> only one?  It’s OK if it’s just expat, but we wouldn’t want to add
>> bigger things.
>
> Not that I know of, this just came up as a problem... But anyway, things
> should not become bigger, right? If I understood correctly, the only effect
> of adding "-lexpat" will be to encode an rpath (and to need expat as an
> input to gettext, of course).

Yes.  It also means that from now on, gettext will depend not only on
glibc but also on Expat, and that upgrading Expat will entail a full
rebuild (so it can only be done in core-updates.)

It’s probably OK because Expat has no extra dependencies, is quite
small, and fairly stable.  Otherwise that would be less convenient.

Thanks,
Ludo’.