nixos/xserver: Properly validate XKB options

Checking the keyboard layout has been a long set of hurdles so far, with several attempts. Originally, the checking was introduced by @lheckemann in #23709. The initial implementation just was trying to check whether the symbols/ directory contained the layout name. Unfortunately, that wasn't enough and keyboard variants weren't recognized, so if you set layout to eg. "dvorak" it will fail with an error (#25526). So my improvement on that was to use sed to filter rules/base.lst and match the layout against that. I fucked up twice with this, first because layout can be a comma-separated list which I didn't account for and second because I ran into a Nix issue (NixOS/nix#1426). After fixing this, it still wasn't enough (and this is btw. what localectl also does), because we were *only* matching rules but not symbols, so using "eu" as a layout won't work either. I decided now it's the time to actually use libxkbcommon to try compiling the keyboard options and see whether it succeeds. This comes in the form of a helper tool called xkbvalidate. IMHO this approach is a lot less error-prone and we can be sure that we don't forget about anything because that's what the X server itself uses to compile the keymap. Another advantage of this is that we now validate the full set of XKB options rather than just the layout. Tested this against a variety of wrong and correct keyboard configurations and against the "keymap" NixOS VM tests. Signed-off-by: aszlig <aszlig@redmoonstudios.org> Cc: @lheckemann, @peti, @7c6f434c, @tohl, @vcunat, @lluchs Fixes: #27597
NixOS · Jul 28, 2017 · 6e5d2f8 · 6e5d2f8 · vcunat · Jul 30, 2017
1 parent 805467b
commit 6e5d2f8
Show file tree

Hide file tree

Showing 4 changed files with 156 additions and 44 deletions.
diff --git a/nixos/modules/services/x11/xserver.nix b/nixos/modules/services/x11/xserver.nix
@@ -648,51 +648,11 @@ in
 
     services.xserver.xkbDir = mkDefault "${pkgs.xkeyboard_config}/etc/X11/xkb";
 
-    system.extraDependencies = singleton (pkgs.runCommand "xkb-layouts-exist" {
-      inherit (cfg) layout xkbDir;
+    system.extraDependencies = singleton (pkgs.runCommand "xkb-validated" {
+      inherit (cfg) xkbModel layout xkbVariant xkbOptions;
+      nativeBuildInputs = [ pkgs.xkbvalidate ];
     } ''
-      # We can use the default IFS here, because the layouts won't contain
-      # spaces or tabs and are ruled out by the sed expression below.
-      availableLayouts="$(
-        sed -n -e ':i /^! \(layout\|variant\) *$/ {
-          # Loop through all of the layouts/variants until we hit another ! at
-          # the start of the line or the line is empty ('t' branches only if
-          # the last substitution was successful, so if the line is empty the
-          # substition will fail).
-          :l; n; /^!/bi; s/^ *\([^ ]\+\).*/\1/p; tl
-        }' "$xkbDir/rules/base.lst" | sort -u
-      )"
-
-      layoutNotFound() {
-        echo >&2
-        echo "The following layouts and variants are available:" >&2
-        echo >&2
-
-        # While an output width of 80 is more desirable for small terminals, we
-        # really don't know the amount of columns of the terminal from within
-        # the builder. The content in $availableLayouts however is pretty
-        # large, so let's opt for a larger width here, because it will print a
-        # smaller amount of lines on modern KMS/framebuffer terminals and won't
-        # lose information even in smaller terminals (it only will look a bit
-        # ugly).
-        echo "$availableLayouts" | ${pkgs.utillinux}/bin/column -c 150 >&2
-
-        echo >&2
-        echo "However, the keyboard layout definition in" \
-             "\`services.xserver.layout' contains the layout \`$1', which" \
-             "isn't a valid layout or variant." >&2
-        echo >&2
-        exit 1
-      }
-
-      # Again, we don't need to take care of IFS, see the comment for
-      # $availableLayouts.
-      for l in ''${layout//,/ }; do
-        if ! echo "$availableLayouts" | grep -qxF "$l"; then
-          layoutNotFound "$l"
-        fi
-      done
-
+      validate "$xkbModel" "$layout" "$xkbVariant" "$xkbOptions"
       touch "$out"
     '');
 

diff --git a/pkgs/tools/X11/xkbvalidate/default.nix b/pkgs/tools/X11/xkbvalidate/default.nix
@@ -0,0 +1,15 @@
+{ lib, runCommandCC, libxkbcommon }:
+
+runCommandCC "xkbvalidate" {
+  buildInputs = [ libxkbcommon ];
+  meta = {
+    description = "NixOS tool to validate X keyboard configuration";
+    license = lib.licenses.mit;
+    platforms = lib.platforms.linux;
+    maintainers = [ lib.maintainers.aszlig ];
+  };
+} ''
+  mkdir -p "$out/bin"
+  gcc -std=gnu11 -Wall -pedantic -lxkbcommon ${./xkbvalidate.c} \
+    -o "$out/bin/validate"
+''
diff --git a/pkgs/tools/X11/xkbvalidate/xkbvalidate.c b/pkgs/tools/X11/xkbvalidate/xkbvalidate.c
@@ -0,0 +1,135 @@
+#define _GNU_SOURCE
+#include <stdarg.h>
+#include <stdbool.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+
+#include <xkbcommon/xkbcommon.h>
+
+static char **log_buffer = NULL;
+static int log_buffer_size = 0;
+static bool log_alloc_success = true;
+
+static void add_log(struct xkb_context *ctx, enum xkb_log_level level,
+                    const char *fmt, va_list args)
+{
+    log_buffer_size++;
+
+    if (log_buffer == NULL)
+        log_buffer = malloc(sizeof(char *));
+    else
+        log_buffer = realloc(log_buffer, sizeof(char *) * log_buffer_size);
+
+    if (log_buffer == NULL) {
+        perror("buffer alloc");
+        log_alloc_success = false;
+        log_buffer_size--;
+        return;
+    }
+
+    if (vasprintf(&log_buffer[log_buffer_size - 1], fmt, args) == -1) {
+        perror("log line alloc");
+        log_alloc_success = false;
+        return;
+    }
+}
+
+static void print_logs(void)
+{
+    for (int i = 0; i < log_buffer_size; ++i)
+        fprintf(stderr, "    %s", log_buffer[i]);
+}
+
+static void free_logs(void)
+{
+    if (log_buffer == NULL)
+        return;
+    for (int i = 0; i < log_buffer_size; ++i)
+        free(log_buffer[i]);
+    free(log_buffer);
+    log_buffer = NULL;
+    log_buffer_size = 0;
+}
+
+static bool try_keymap(struct xkb_context *ctx, struct xkb_rule_names *rdef)
+{
+    struct xkb_keymap *keymap;
+    bool result = true;
+
+    if ((keymap = xkb_keymap_new_from_names(ctx, rdef, 0)) == NULL)
+        result = false;
+    else
+        xkb_keymap_unref(keymap);
+
+    return result;
+}
+
+static void print_error(const char *name, const char *value,
+                        const char *nixos_option)
+{
+    fprintf(stderr, "\nThe value `%s' for keyboard %s is invalid.\n\n"
+                    "Please check the definition in `services.xserver.%s'.\n",
+            value, name, nixos_option);
+    fputs("\nDetailed XKB compiler errors:\n\n", stderr);
+    print_logs();
+    putc('\n', stderr);
+}
+
+#define TRY_KEYMAP(name, value, nixos_option) \
+    *rdef = (struct xkb_rule_names) {0}; \
+    free_logs(); \
+    rdef->name = value; \
+    result = try_keymap(ctx, rdef); \
+    if (!log_alloc_success) \
+        goto out; \
+    if (!result) { \
+        print_error(#name, value, nixos_option); \
+        exit_code = EXIT_FAILURE; \
+        goto out; \
+    }
+
+int main(int argc, char **argv)
+{
+    int exit_code = EXIT_SUCCESS;
+    bool result;
+    struct xkb_context *ctx;
+    struct xkb_rule_names *rdef;
+
+    if (argc != 5) {
+        fprintf(stderr, "Usage: %s model layout variant options\n", argv[0]);
+        return EXIT_FAILURE;
+    }
+
+    ctx = xkb_context_new(XKB_CONTEXT_NO_ENVIRONMENT_NAMES);
+    xkb_context_set_log_fn(ctx, add_log);
+
+    rdef = malloc(sizeof(struct xkb_rule_names));
+
+    TRY_KEYMAP(model,   argv[1], "xkbModel");
+    TRY_KEYMAP(layout,  argv[2], "layout");
+    TRY_KEYMAP(variant, argv[3], "xkbVariant");
+    TRY_KEYMAP(options, argv[4], "xkbOptions");
+
+    free_logs();
+    rdef->model = argv[1];
+    rdef->layout = argv[2];
+    rdef->variant = argv[3];
+    rdef->options = argv[4];
+
+    result = try_keymap(ctx, rdef);
+    if (!log_alloc_success)
+        goto out;
+
+    if (!result) {
+        fputs("The XKB keyboard definition failed to compile:\n", stderr);
+        print_logs();
+        exit_code = EXIT_FAILURE;
+    }
+
+out:
+    free_logs();
+    free(rdef);
+    xkb_context_unref(ctx);
+    return exit_code;
+}
diff --git a/pkgs/top-level/all-packages.nix b/pkgs/top-level/all-packages.nix
@@ -4805,6 +4805,8 @@ with pkgs;
 
   xbrightness = callPackage ../tools/X11/xbrightness { };
 
+  xkbvalidate = callPackage ../tools/X11/xkbvalidate { };
+
   xfstests = callPackage ../tools/misc/xfstests { };
 
   xprintidle-ng = callPackage ../tools/X11/xprintidle-ng {};