[PATCH] gnu: kbd: Recursively search $LOADKEYS_KEYMAP_PATH.

  • Done
  • quality assurance status badge
Details
3 participants
  • Ludovic Courtès
  • Tobias Geerinckx-Rice
  • ng0
Owner
unassigned
Submitted by
Tobias Geerinckx-Rice
Severity
normal
T
T
Tobias Geerinckx-Rice wrote on 13 Jul 2017 02:34
(address . guix-patches@gnu.org)
20170713003425.31282-1-me@tobias.gr
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(-)

Toggle diff (18 lines)
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
T
T
Tobias Geerinckx-Rice wrote on 13 Jul 2017 02:41
(address . 27675@debbugs.gnu.org)
10a001fa-5412-4795-3509-f8ed160a3f3e@tobias.gr
On 13/07/17 02:34, Tobias Geerinckx-Rice wrote:
Toggle quote (2 lines)
> * gnu/packages/linux.scm (kbd)[native-search-path]: Add a double asterisk.

Ach. -paths, of course.
Attachment: signature.asc
N
N
ng0 wrote on 13 Jul 2017 09:41
(name . Tobias Geerinckx-Rice)(address . me@tobias.gr)(address . 27675@debbugs.gnu.org)
20170713074139.bu7ccp5xkbqhphpq@abyayala
Tobias Geerinckx-Rice transcribed 1.5K bytes:
Toggle quote (6 lines)
> 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
-----BEGIN PGP SIGNATURE-----

iQIzBAABCgAdFiEEqIyK3RKYKNfqwC5S4i+bv+40hYgFAllnJDMACgkQ4i+bv+40
hYgrQA//bhuE77x0xs0yyHwJFJU2yMQDx07y1nZfr0byttGEgsjRn2XEQRfCmvwi
+Dr/vXHMs/+Y8/zF3FDiJhM9/nwg+k2awvKF5ecjm8AKhmeHEkQwGjr1iB4Jwm9e
mzOMtWVydohgz3uufNoYCqrMBjiA/cBG+EXLKoTmUfoliF2SNCC/BlCqpAsC+EUq
GMulUdJo+C7FzDvBbj/TRX4Q14l19zcXieW5rc3CQecB43t8HQABxetiw4H/B05g
GU5m17iuiqLm9xZTqaJseUcybKxRrnLDAle1I+JzknoGpsoJutDTCgKnhvqBocXX
OwmvpI5nANMm0WvDuSDKbSySvaAnq1ziZL1tL2r3n4NFNtQ7A/8mHZvDPpVWo4Fy
MsEU1pgvxRmXxAnkwlQ02btiagCG8N1fopMOCsZZ8SPqXAiRs4SQqmNkhwpCB3kn
fV3ZcnZ96Jmv9ml0iK+AUQ+v2LS98bv1Y+kQGbV/5JPGvKcjDllMBIMqPde3TJac
w/0l6q0kBfOOwlfCGCqEMC4kt47lRoZb4rS7GJ3J3dz9SvIn23//9s3YsdTIJqRJ
Dhud/SxnFPTKa2XJI+zmym4FB9DfuYOwvnM7Q+JvsctwpInCKuoZdpCqdd0l7lq2
h4GsNzr4paaAnSKW6UCfuoeFdmJjMDmiVW/qjJBVOKGVeYxrvGo=
=fBB6
-----END PGP SIGNATURE-----


T
T
Tobias Geerinckx-Rice wrote on 14 Jul 2017 10:39
[PATCH] gnu: kbd: Recursively search $LOADKEYS_KEYMAP_PATH.
(address . 27675-done@debbugs.gnu.org)
8d04dcc5-63ac-c376-b2a1-f0ca576b0269@tobias.gr
ng0 wrote:
Toggle quote (2 lines)
> 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: signature.asc
Closed
L
L
Ludovic Courtès wrote on 17 Jul 2017 11:20
(name . Tobias Geerinckx-Rice)(address . me@tobias.gr)(address . 27675@debbugs.gnu.org)
87poczctc8.fsf@gnu.org
Hello!

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

Toggle quote (21 lines)
> 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:

Toggle snippet (10 lines)
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…

Is this correct?

Ludo’.
T
T
Tobias Geerinckx-Rice wrote on 17 Jul 2017 11:35
(address . ludo@gnu.org)(address . 27675@debbugs.gnu.org)
383b1065-2c96-4982-2fc1-92b356a50838@tobias.gr
Ludo',

On 17/07/17 11:20, Ludovic Courtès wrote:
Toggle quote (4 lines)
> Tobias Geerinckx-Rice <me@tobias.gr> skribis:
>
>> Fix a regression since commit fd7000fe33d3c4188c241cab97e2b891dd4e1268.

<snip>

Toggle quote (3 lines)
> 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.)

Toggle quote (2 lines)
> (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: signature.asc
L
L
Ludovic Courtès wrote on 17 Jul 2017 13:00
(name . Tobias Geerinckx-Rice)(address . me@tobias.gr)(address . 27675@debbugs.gnu.org)
87d18z9vks.fsf@gnu.org
Heya,

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

Toggle quote (13 lines)
> 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’.
T
T
Tobias Geerinckx-Rice wrote on 17 Jul 2017 13:54
193d8037-1817-bca7-231b-b58a8cbddb99@tobias.gr
[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:
Toggle quote (3 lines)
> 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).

Toggle quote (3 lines)
> 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.

Toggle quote (3 lines)
> 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: signature.asc
L
L
Ludovic Courtès wrote on 17 Jul 2017 17:46
(name . Tobias Geerinckx-Rice)(address . me@tobias.gr)(address . 27675@debbugs.gnu.org)
87d18z5anm.fsf@gnu.org
Hello,

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

Toggle quote (17 lines)
> 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!).

Toggle quote (12 lines)
>> 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:

Toggle snippet (11 lines)
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"))

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’.
?