[bug#27675] [PATCH] gnu: kbd: Recursively search $LOADKEYS_KEYMAP_PATH.

[bug#27675] [PATCH] gnu: kbd: Recursively search $LOADKEYS_KEYMAP_PATH.

[Tobias Geerinckx-Rice]

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

[bug#27675] [PATCH] gnu: kbd: Recursively search $LOADKEYS_KEYMAP_PATH.

[Tobias Geerinckx-Rice]

[-- Attachment #1.1: Type: text/plain, Size: 156 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.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 504 bytes --]

[bug#27675] [PATCH] gnu: kbd: Recursively search $LOADKEYS_KEYMAP_PATH.

[ng0]

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

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

bug#27675: [PATCH] gnu: kbd: Recursively search $LOADKEYS_KEYMAP_PATH.

[Tobias Geerinckx-Rice]

[-- Attachment #1.1: Type: text/plain, Size: 296 bytes --]

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 504 bytes --]

[bug#27675] [PATCH] gnu: kbd: Recursively search $LOADKEYS_KEYMAP_PATH.

[Ludovic Courtès]

Hello!

Tobias Geerinckx-Rice <me@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’.

[bug#27675] [PATCH] gnu: kbd: Recursively search $LOADKEYS_KEYMAP_PATH.

[Tobias Geerinckx-Rice]

[-- Attachment #1.1: Type: text/plain, Size: 814 bytes --]

Ludo',

On 17/07/17 11:20, Ludovic Courtès wrote:
> Tobias Geerinckx-Rice <me@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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 504 bytes --]

[bug#27675] [PATCH] gnu: kbd: Recursively search $LOADKEYS_KEYMAP_PATH.

[Ludovic Courtès]

Heya,

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

> On 17/07/17 11:20, Ludovic Courtès wrote:
>> Tobias Geerinckx-Rice <me@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’.

[bug#27675] [PATCH] gnu: kbd: Recursively search $LOADKEYS_KEYMAP_PATH.

[Tobias Geerinckx-Rice]

[-- Attachment #1.1: Type: text/plain, Size: 1400 bytes --]

[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




[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 504 bytes --]

[bug#27675] [PATCH] gnu: kbd: Recursively search $LOADKEYS_KEYMAP_PATH.

[Ludovic Courtès]

Hello,

Tobias Geerinckx-Rice <me@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’.