GNU bug report logs - #27675
[PATCH] gnu: kbd: Recursively search $LOADKEYS_KEYMAP_PATH.

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 27675 in the body.
You can then email your comments to 27675 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Tobias Geerinckx-Rice <me <at> tobias.gr>
To: guix-patches <at> gnu.org
Subject: [PATCH] gnu: kbd: Recursively search $LOADKEYS_KEYMAP_PATH.
Date: Thu, 13 Jul 2017 02:34:25 +0200

Fix a regression since commit fd7000fe33d3c4188c241cab97e2b891dd4e1268.

* gnu/packages/linux.scm (kbd)[native-search-path]: Add a double asterisk.
---
 gnu/packages/linux.scm | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/gnu/packages/linux.scm b/gnu/packages/linux.scm
index c5fed1a7c..700408cc8 100644
--- a/gnu/packages/linux.scm
+++ b/gnu/packages/linux.scm
@@ -1837,7 +1837,10 @@ system.")
     (native-search-paths
      (list (search-path-specification
             (variable "LOADKEYS_KEYMAP_PATH")
-            (files (list "share/keymaps")))))
+            ;; Append ‘/**’ to recursively search all directories.  One can then
+            ;; run (for example) ‘loadkeys en-latin9’ instead of having to find
+            ;; and type ‘i386/colemak/en-latin9’ on an unfamiliar keyboard.
+            (files (list "share/keymaps/**")))))
     (native-inputs `(("pkg-config" ,pkg-config)))
     (home-page "http://kbd-project.org/")
     (synopsis "Linux keyboard utilities and keyboard maps")
-- 
2.13.1

Message #8 received at 27675 <at> debbugs.gnu.org (full text, mbox):

From: Tobias Geerinckx-Rice <me <at> tobias.gr>
To: 27675 <at> debbugs.gnu.org
Subject: Re: [bug#27675] [PATCH] gnu: kbd: Recursively search
 $LOADKEYS_KEYMAP_PATH.
Date: Thu, 13 Jul 2017 02:41:00 +0200

[Message part 1 (text/plain, inline)]

On 13/07/17 02:34, Tobias Geerinckx-Rice wrote:
> * gnu/packages/linux.scm (kbd)[native-search-path]: Add a double asterisk.

Ach. -paths, of course.

[signature.asc (application/pgp-signature, attachment)]

Message #11 received at 27675 <at> debbugs.gnu.org (full text, mbox):

From: ng0 <ng0 <at> infotropique.org>
To: Tobias Geerinckx-Rice <me <at> tobias.gr>
Cc: 27675 <at> debbugs.gnu.org
Subject: Re: [bug#27675] [PATCH] gnu: kbd: Recursively search
 $LOADKEYS_KEYMAP_PATH.
Date: Thu, 13 Jul 2017 07:41:39 +0000

[Message part 1 (text/plain, inline)]

Tobias Geerinckx-Rice transcribed 1.5K bytes:
> On 13/07/17 02:34, Tobias Geerinckx-Rice wrote:
> > * gnu/packages/linux.scm (kbd)[native-search-path]: Add a double asterisk.
> 
> Ach. -paths, of course.
> 

Oh noes. Thanks for discovering this! I really did not test well enough
when I added this (if it was during the addition of neo-kbd).
-- 
ng0
GnuPG: A88C8ADD129828D7EAC02E52E22F9BBFEE348588
GnuPG: https://n0is.noblogs.org/my-keys
https://www.infotropique.org https://krosos.org

[signature.asc (application/pgp-signature, inline)]

Message #16 received at 27675-done <at> debbugs.gnu.org (full text, mbox):

From: Tobias Geerinckx-Rice <me <at> tobias.gr>
To: 27675-done <at> debbugs.gnu.org
Subject: [PATCH] gnu: kbd: Recursively search $LOADKEYS_KEYMAP_PATH.
Date: Fri, 14 Jul 2017 10:39:40 +0200

[Message part 1 (text/plain, inline)]

ng0 wrote:
> Oh noes. Thanks for discovering this!

With pleasure. Pushed as 674d4e1380a43d83e77f81cbc3a8da8969e70f11.

Odd that it took this long for someone to notice. I'd've hoped we had
more new users/machines than that. Or the poor souls all use qwerties.

Kind regards,

T G-R

[signature.asc (application/pgp-signature, attachment)]

Message #19 received at 27675 <at> debbugs.gnu.org (full text, mbox):

From: ludo <at> gnu.org (Ludovic Courtès)
To: Tobias Geerinckx-Rice <me <at> tobias.gr>
Cc: 27675 <at> debbugs.gnu.org
Subject: Re: [bug#27675] [PATCH] gnu: kbd: Recursively search
 $LOADKEYS_KEYMAP_PATH.
Date: Mon, 17 Jul 2017 11:20:55 +0200

Hello!

Tobias Geerinckx-Rice <me <at> tobias.gr> skribis:

> Fix a regression since commit fd7000fe33d3c4188c241cab97e2b891dd4e1268.
>
> * gnu/packages/linux.scm (kbd)[native-search-path]: Add a double asterisk.
> ---
>  gnu/packages/linux.scm | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/gnu/packages/linux.scm b/gnu/packages/linux.scm
> index c5fed1a7c..700408cc8 100644
> --- a/gnu/packages/linux.scm
> +++ b/gnu/packages/linux.scm
> @@ -1837,7 +1837,10 @@ system.")
>      (native-search-paths
>       (list (search-path-specification
>              (variable "LOADKEYS_KEYMAP_PATH")
> -            (files (list "share/keymaps")))))
> +            ;; Append ‘/**’ to recursively search all directories.  One can then
> +            ;; run (for example) ‘loadkeys en-latin9’ instead of having to find
> +            ;; and type ‘i386/colemak/en-latin9’ on an unfamiliar keyboard.
> +            (files (list "share/keymaps/**")))))

That works if you copy paste the suggested search path in a shell, but
not if we pass it to ‘evaluate-search-paths’ from (guix search-paths)
(it doesn’t recognize the ** syntax.)

What about:

  (search-path-specification
    (variable "LOADKEYS_KEYMAP_PATH")
    (files (list "share/keymaps"))
    (file-type 'regular)
    (file-pattern ".*"))  ;or "\\.map"?

That gives this:

--8<---------------cut here---------------start------------->8---
scheme@(guix search-paths)> (search-path-specification
    (variable "LOADKEYS_KEYMAP_PATH")
    (files (list "share/keymaps"))
    (file-type 'regular)
    (file-pattern ".*"))
$8 = #<<search-path-specification> variable: "LOADKEYS_KEYMAP_PATH" files: ("share/keymaps") separator: ":" file-type: regular file-pattern: ".*">
scheme@(guix search-paths)> (evaluate-search-paths (list $8) (list "/run/current-system/profile"))
$9 = ((#<<search-path-specification> variable: "LOADKEYS_KEYMAP_PATH" files: ("share/keymaps") separator: ":" file-type: regular file-pattern: ".*"> . "/run/current-system/profile/share/keymaps/amiga/amiga-de.map.gz:/run/current-system/profile/share/keymaps/amiga/amiga-us.map.gz:/run/current-system/profile/share/keymaps/atari/atari-de.map.gz:/run/current-system/profile/share/keymaps/atari/atari-se.map.gz:/run/current-system/profile/share/keymaps/atari/atari-uk-falcon.map.gz:/run/current-system/profile/share/keymaps/atari/atari-us.map.gz:/run/current-system/profile/share/key…
--8<---------------cut here---------------end--------------->8---

Is this correct?

Ludo’.

Message #22 received at 27675 <at> debbugs.gnu.org (full text, mbox):

From: Tobias Geerinckx-Rice <me <at> tobias.gr>
To: ludo <at> gnu.org
Cc: 27675 <at> debbugs.gnu.org
Subject: Re: [bug#27675] [PATCH] gnu: kbd: Recursively search
 $LOADKEYS_KEYMAP_PATH.
Date: Mon, 17 Jul 2017 11:35:42 +0200

[Message part 1 (text/plain, inline)]

Ludo',

On 17/07/17 11:20, Ludovic Courtès wrote:
> Tobias Geerinckx-Rice <me <at> tobias.gr> skribis:
> 
>> Fix a regression since commit fd7000fe33d3c4188c241cab97e2b891dd4e1268.

<snip>

> That works if you copy paste the suggested search path in a shell, but
> not if we pass it to ‘evaluate-search-paths’ from (guix search-paths)

I'm confused. What exactly doesn't work? Here, after this patch,
LOADKEYS_KEYMAP_PATH contains "**" like it should.

(I wouldn't have been able to install GuixSD if it didn't! :-) Whoever
designed the Belgian keyboard layout was either on some very cheap
crack, or the expensive good stuff.)

> (it doesn’t recognize the ** syntax.)

Hrm. I don't think it should try to recognise anything, that can only be
done at run-time.

Kind regards,

T G-R

[signature.asc (application/pgp-signature, attachment)]

Message #25 received at 27675 <at> debbugs.gnu.org (full text, mbox):

From: ludo <at> gnu.org (Ludovic Courtès)
To: Tobias Geerinckx-Rice <me <at> tobias.gr>
Cc: 27675 <at> debbugs.gnu.org
Subject: Re: [bug#27675] [PATCH] gnu: kbd: Recursively search
 $LOADKEYS_KEYMAP_PATH.
Date: Mon, 17 Jul 2017 13:00:51 +0200

Heya,

Tobias Geerinckx-Rice <me <at> tobias.gr> skribis:

> On 17/07/17 11:20, Ludovic Courtès wrote:
>> Tobias Geerinckx-Rice <me <at> tobias.gr> skribis:
>> 
>>> Fix a regression since commit fd7000fe33d3c4188c241cab97e2b891dd4e1268.
>
> <snip>
>
>> That works if you copy paste the suggested search path in a shell, but
>> not if we pass it to ‘evaluate-search-paths’ from (guix search-paths)
>
> I'm confused. What exactly doesn't work? Here, after this patch,
> LOADKEYS_KEYMAP_PATH contains "**" like it should.

I mean, it works because it turns out that we pass those ** to Bash,
which does the right thing.

However, a search-path specification is supposed to be understandable
internally by ‘evaluate-search-paths’ (this is what is used in build
environments, when generating ‘etc/profile’, when using --search-paths,
and so on.)  The ** expansion would not happen in contexts where Bash is
not involved, which is not great.

In this case I agree that it’s almost a theoretical problem because in
practice, for LOADKEYS_KEYMAP_PATH, we’re always passing the search path
to the shell.  So maybe it’s not worth fixing after all, dunno.  WDYT?

Ludo’.

Message #28 received at 27675 <at> debbugs.gnu.org (full text, mbox):

From: Tobias Geerinckx-Rice <me <at> tobias.gr>
To: ludo <at> gnu.org, 27675 <at> debbugs.gnu.org
Subject: Re: [bug#27675] [PATCH] gnu: kbd: Recursively search
 $LOADKEYS_KEYMAP_PATH.
Date: Mon, 17 Jul 2017 13:54:48 +0200

[Message part 1 (text/plain, inline)]

[Sent to the list this time. Replying is hard.]

Ludo',

I don't think there's anything to ‘fix’ here.

On 17/07/17 13:00, Ludovic Courtès wrote:
> I mean, it works because it turns out that we pass those ** to Bash, 
> which does the right thing.

But that's not true:

	/* Search a list of directories and directory hierarchies */
	for (i = 0; dirpath[i]; i++) {
		recdepth = 0;
		dl       = strlen(dirpath[i]);

		/* trailing stars denote recursion */
		while (dl && dirpath[i][dl - 1] == '*')
			dl--, recdepth++;

(src/libkeymap/findfile.c:269).

> However, a search-path specification is supposed to be
> understandable internally by ‘evaluate-search-paths’

Erk. So you're saying Guix tries to do clever things (beyond separator
concatenation) to search-paths before exporting them? That won't work.
If that is the case, we'll have to use something other than search-paths
for kbd (and any packages that interpret things like ‘*’ themselves,
without a shell).

But again, at least in the installer image, LOADKEYS_KEYMAP_PATH is
properly untouched as far as I've tested.

> The ** expansion would not happen in contexts where Bash is not
> involved, which is not great.

Bash isn't involved at all in this case: "/**" is a signal to loadkeys
itself to recurse. I think that's where the confusion comes from.

Kind regards,

T G-R

[signature.asc (application/pgp-signature, attachment)]

Message #31 received at 27675 <at> debbugs.gnu.org (full text, mbox):

From: ludo <at> gnu.org (Ludovic Courtès)
To: Tobias Geerinckx-Rice <me <at> tobias.gr>
Cc: 27675 <at> debbugs.gnu.org
Subject: Re: [bug#27675] [PATCH] gnu: kbd: Recursively search
 $LOADKEYS_KEYMAP_PATH.
Date: Mon, 17 Jul 2017 17:46:21 +0200

Hello,

Tobias Geerinckx-Rice <me <at> tobias.gr> skribis:

> On 17/07/17 13:00, Ludovic Courtès wrote:
>> I mean, it works because it turns out that we pass those ** to Bash, 
>> which does the right thing.
>
> But that's not true:
>
> 	/* Search a list of directories and directory hierarchies */
> 	for (i = 0; dirpath[i]; i++) {
> 		recdepth = 0;
> 		dl       = strlen(dirpath[i]);
>
> 		/* trailing stars denote recursion */
> 		while (dl && dirpath[i][dl - 1] == '*')
> 			dl--, recdepth++;
>
> (src/libkeymap/findfile.c:269).

Ah OK, if that’s a libkeymap thing, that’s better (I should know Bash
better!).

>> However, a search-path specification is supposed to be
>> understandable internally by ‘evaluate-search-paths’
>
> Erk. So you're saying Guix tries to do clever things (beyond separator
> concatenation) to search-paths before exporting them? That won't work.
> If that is the case, we'll have to use something other than search-paths
> for kbd (and any packages that interpret things like ‘*’ themselves,
> without a shell).
>
> But again, at least in the installer image, LOADKEYS_KEYMAP_PATH is
> properly untouched as far as I've tested.

Yes, that’s OK.

What I meant is that search-path-specifications have clear semantics
that are interpreted by ‘evaluate-search-paths’.  In this case, what
happens is this:

--8<---------------cut here---------------start------------->8---
scheme@(guile-user)> ,use(guix search-paths)
scheme@(guile-user)> (search-path-specification
            (variable "LOADKEYS_KEYMAP_PATH")
            ;; Append ‘/**’ to recursively search all directories.  One can then
            ;; run (for example) ‘loadkeys en-latin9’ instead of having to find
            ;; and type ‘i386/colemak/en-latin9’ on a mislabelled keyboard.
            (files (list "share/keymaps/**")))
$4 = #<<search-path-specification> variable: "LOADKEYS_KEYMAP_PATH" files: ("share/keymaps/**") separator: ":" file-type: directory file-pattern: #f>
scheme@(guile-user)> (evaluate-search-paths (list $4) (list "/run/current-system/profile"))
--8<---------------cut here---------------end--------------->8---

AFAICS, /run/current-system/profile/etc/profile does not include a
LOADKEYS_KEYMAP_PATH definition because of that.  Or am I missing
something?

Thank you,
Ludo’.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.