tools: Do not load names from the environment by default

Our tools are debugging tools and as such we need to have complete
control to be able to reproduce setups. This is not currently the case,
as we do not use `XKB_CONTEXT_NO_ENVIRONMENT_NAMES` by default nor can
we set it.

So it is very easy to forget about the various `XKB_DEFAULT_*`
environement variables for the default RMLVO values, then to get puzzled
by unexpected results.

Added to that, these environment variables do not work correctly in
`xkbcli-compile-xeymap`: calling the tool without RMLVO values will
use these variables only if the RMLVO values are set explicitly empty or
if the various *constants* `DEFAULT_XKB_*` are empty. This is unexpected,
as the environment variables should *always* be used unless:

- `XKB_CONTEXT_NO_ENVIRONMENT_NAMES` is used (not the case here);
- the variable is empty; in this case the constants `DEFAULT_XKB_*` are
  used.

Fixed by the following *breaking change*: make the tools use
`XKB_CONTEXT_NO_ENVIRONMENT_NAMES` *by default*, unless the new
`--enable-environment-names` option is used.

We also make `rmlvo` incompatible with `--enable-environment-names` for
now in the public tool, as else it requires a private API.

Author: wismill

changes/tools/+enable-environment-names.breaking.md

@@ -0,0 +1,3 @@
+The tools do not load the *default* RMLVO (rules, model, layout, variant, options)
+values from the environment anymore. The previous behavior may be restored by using
+the new `--enable-environment-names` option.

test/tool-option-parsing.py

@@ -33,19 +33,6 @@
         file=sys.stderr,
     )
 
-
-# Unset some environment variables, so that tools that rely on them for missing
-# arguments default have the expected behavior.
-for key in (
-    "XKB_DEFAULT_RULES",
-    "XKB_DEFAULT_MODEL",
-    "XKB_DEFAULT_LAYOUT",
-    "XKB_DEFAULT_VARIANT",
-    "XKB_DEFAULT_OPTIONS",
-):
-    if key in os.environ:
-        del os.environ[key]
-
 # Ensure locale is C, so we can check error messages in English
 os.environ["LC_ALL"] = "C.UTF-8"
 

tools/compile-keymap.c

@@ -14,7 +14,7 @@
 #include <string.h>
 
 #include "xkbcommon/xkbcommon.h"
-#if ENABLE_PRIVATE_APIS
+#ifdef ENABLE_PRIVATE_APIS
 #include "xkbcomp/xkbcomp-priv.h"
 #include "xkbcomp/rules.h"
 #endif
@@ -49,7 +49,7 @@ usage(FILE *file, const char *progname)
            "    Enable verbose debugging output\n"
            " --test\n"
            "    Test compilation but do not print the keymap.\n"
-#if ENABLE_PRIVATE_APIS
+#ifdef ENABLE_PRIVATE_APIS
            " --kccgst\n"
            "    Print a keymap which only includes the KcCGST component names instead of the full keymap\n"
 #endif
@@ -59,7 +59,7 @@ usage(FILE *file, const char *progname)
            " --from-xkb <file>\n"
            "    Load the corresponding XKB file, ignore RMLVO options. If <file>\n"
            "    is \"-\" or missing, then load from stdin."
-#if ENABLE_PRIVATE_APIS
+#ifdef ENABLE_PRIVATE_APIS
            "    This option must not be used with --kccgst.\n"
 #endif
            " --include\n"
@@ -84,6 +84,14 @@ usage(FILE *file, const char *progname)
            "    The XKB layout variant (default: '%s')\n"
            " --options <options>\n"
            "    The XKB options (default: '%s')\n"
+           " --enable-environment-names\n"
+           "    Allow to set the default RMLVO values via the following environment variables:\n"
+           "    - XKB_DEFAULT_RULES\n"
+           "    - XKB_DEFAULT_MODEL\n"
+           "    - XKB_DEFAULT_LAYOUT\n"
+           "    - XKB_DEFAULT_VARIANT\n"
+           "    - XKB_DEFAULT_OPTIONS\n"
+           "    Note that this option may affect the default values of the previous options.\n"
            "\n",
            progname, DEFAULT_XKB_RULES,
            DEFAULT_XKB_MODEL, DEFAULT_XKB_LAYOUT,
@@ -92,7 +100,8 @@ usage(FILE *file, const char *progname)
 }
 
 static bool
-parse_options(int argc, char **argv, char **path, struct xkb_rule_names *names)
+parse_options(int argc, char **argv, bool *use_env_names,
+              char **path, struct xkb_rule_names *names)
 {
     enum options {
         OPT_VERBOSE,
@@ -102,6 +111,7 @@ parse_options(int argc, char **argv, char **path, struct xkb_rule_names *names)
         OPT_FROM_XKB,
         OPT_INCLUDE,
         OPT_INCLUDE_DEFAULTS,
+        OPT_ENABLE_ENV_NAMES,
         OPT_RULES,
         OPT_MODEL,
         OPT_LAYOUT,
@@ -112,7 +122,7 @@ parse_options(int argc, char **argv, char **path, struct xkb_rule_names *names)
         {"help",             no_argument,            0, 'h'},
         {"verbose",          no_argument,            0, OPT_VERBOSE},
         {"test",             no_argument,            0, OPT_TEST},
-#if ENABLE_PRIVATE_APIS
+#ifdef ENABLE_PRIVATE_APIS
         {"kccgst",           no_argument,            0, OPT_KCCGST},
 #endif
         {"rmlvo",            no_argument,            0, OPT_RMLVO},
@@ -121,6 +131,7 @@ parse_options(int argc, char **argv, char **path, struct xkb_rule_names *names)
         {"from-xkb",         optional_argument,      0, OPT_FROM_XKB},
         {"include",          required_argument,      0, OPT_INCLUDE},
         {"include-defaults", no_argument,            0, OPT_INCLUDE_DEFAULTS},
+        {"enable-environment-names", no_argument,    0, OPT_ENABLE_ENV_NAMES},
         {"rules",            required_argument,      0, OPT_RULES},
         {"model",            required_argument,      0, OPT_MODEL},
         {"layout",           required_argument,      0, OPT_LAYOUT},
@@ -130,6 +141,7 @@ parse_options(int argc, char **argv, char **path, struct xkb_rule_names *names)
     };
 
     bool has_rmlvo_options = false;
+    *use_env_names = false;
     while (1) {
         int option_index = 0;
         int c = getopt_long(argc, argv, "h", opts, &option_index);
@@ -154,6 +166,10 @@ parse_options(int argc, char **argv, char **path, struct xkb_rule_names *names)
         case OPT_RMLVO:
             if (output_format != FORMAT_KEYMAP_FROM_RMLVO)
                 goto output_format_error;
+#ifndef ENABLE_PRIVATE_APIS
+            if (*use_env_names)
+                goto rmlvo_env_error;
+#endif
             output_format = FORMAT_RMLVO;
             break;
         case OPT_FROM_XKB:
@@ -182,6 +198,13 @@ parse_options(int argc, char **argv, char **path, struct xkb_rule_names *names)
                 goto too_many_includes;
             includes[num_includes++] = DEFAULT_INCLUDE_PATH_PLACEHOLDER;
             break;
+        case OPT_ENABLE_ENV_NAMES:
+#ifndef ENABLE_PRIVATE_APIS
+            if (output_format == FORMAT_RMLVO)
+                goto rmlvo_env_error;
+#endif
+            *use_env_names = true;
+            break;
         case OPT_RULES:
             if (output_format == FORMAT_KEYMAP_FROM_XKB)
                 goto input_format_error;
@@ -243,6 +266,14 @@ parse_options(int argc, char **argv, char **path, struct xkb_rule_names *names)
 
     return true;
 
+#ifndef ENABLE_PRIVATE_APIS
+rmlvo_env_error:
+    /* See comment in print_rmlvo */
+    fprintf(stderr, "ERROR: --rmlvo is not compatible with "
+                    "--enable-environment-names yet\n");
+    exit(EXIT_INVALID_USAGE);
+#endif
+
 output_format_error:
     fprintf(stderr, "ERROR: Cannot mix output formats\n");
     usage(stderr, argv[0]);
@@ -260,9 +291,36 @@ parse_options(int argc, char **argv, char **path, struct xkb_rule_names *names)
 }
 
 static int
-print_rmlvo(struct xkb_context *ctx, const struct xkb_rule_names *rmlvo)
+print_rmlvo(struct xkb_context *ctx, struct xkb_rule_names *rmlvo)
 {
-    printf("rules: \"%s\"\nmodel: \"%s\"\nlayout: \"%s\"\nvariant: \"%s\"\noptions: \"%s\"\n",
+    /* Fill defaults */
+#ifndef ENABLE_PRIVATE_APIS
+    /* FIXME: We should use `xkb_context_sanitize_rule_names`, but this is
+     *        not a public API yet. Instead we just do not support names from
+     *        environment variables. */
+    if (isempty(rmlvo->rules))
+        rmlvo->rules = DEFAULT_XKB_RULES;
+    if (isempty(rmlvo->model))
+        rmlvo->model = DEFAULT_XKB_MODEL;
+    /* Layout and variant are tied together, so we either get user-supplied for
+     * both or default for both */
+    if (isempty(rmlvo->layout)) {
+        if (!isempty(rmlvo->variant)) {
+            fprintf(stderr, "ERROR: a variant requires a layout\n");
+            return EXIT_INVALID_USAGE;
+        }
+        rmlvo->layout = DEFAULT_XKB_LAYOUT;
+        rmlvo->variant = DEFAULT_XKB_VARIANT;
+    }
+    if (isempty(rmlvo->options))
+        rmlvo->options = DEFAULT_XKB_OPTIONS;
+#else
+    /* Resolve default RMLVO values */
+    xkb_context_sanitize_rule_names(ctx, rmlvo);
+#endif
+
+    printf("rules: \"%s\"\nmodel: \"%s\"\nlayout: \"%s\"\nvariant: \"%s\"\n"
+           "options: \"%s\"\n",
            rmlvo->rules, rmlvo->model, rmlvo->layout,
            rmlvo->variant ? rmlvo->variant : "",
            rmlvo->options ? rmlvo->options : "");
@@ -272,7 +330,7 @@ print_rmlvo(struct xkb_context *ctx, const struct xkb_rule_names *rmlvo)
 static int
 print_kccgst(struct xkb_context *ctx, struct xkb_rule_names *rmlvo)
 {
-#if ENABLE_PRIVATE_APIS
+#ifdef ENABLE_PRIVATE_APIS
         struct xkb_component_names kccgst;
 
         /* Resolve default RMLVO values */
@@ -377,36 +435,23 @@ main(int argc, char **argv)
 {
     struct xkb_context *ctx;
     char *keymap_path = NULL;
-    struct xkb_rule_names names = {
-        .rules = DEFAULT_XKB_RULES,
-        .model = DEFAULT_XKB_MODEL,
-        /* layout and variant are tied together, so we either get user-supplied for
-         * both or default for both, see below */
-        .layout = NULL,
-        .variant = NULL,
-        .options = DEFAULT_XKB_OPTIONS,
-    };
+    struct xkb_rule_names names = { 0 };
+    bool use_env_names = false;
     int rc = 1;
 
     if (argc < 1) {
         usage(stderr, argv[0]);
         return EXIT_INVALID_USAGE;
     }
 
-    if (!parse_options(argc, argv, &keymap_path, &names))
+    if (!parse_options(argc, argv, &use_env_names, &keymap_path, &names))
         return EXIT_INVALID_USAGE;
 
-    /* Now fill in the layout */
-    if (!names.layout || !*names.layout) {
-        if (names.variant && *names.variant) {
-            fprintf(stderr, "ERROR: a variant requires a layout\n");
-            return EXIT_INVALID_USAGE;
-        }
-        names.layout = DEFAULT_XKB_LAYOUT;
-        names.variant = DEFAULT_XKB_VARIANT;
-    }
+    enum xkb_context_flags ctx_flags = XKB_CONTEXT_NO_DEFAULT_INCLUDES;
+    if (!use_env_names)
+        ctx_flags |= XKB_CONTEXT_NO_ENVIRONMENT_NAMES;
 
-    ctx = xkb_context_new(XKB_CONTEXT_NO_DEFAULT_INCLUDES);
+    ctx = xkb_context_new(ctx_flags);
     assert(ctx);
 
     if (verbose) {

tools/how-to-type.c

@@ -63,9 +63,10 @@ parse_char_or_codepoint(const char *raw) {
 static void
 usage(const char *argv0, FILE *fp)
 {
-    fprintf(fp, "Usage: %s [--help] [--keysym] [--rules <rules>] [--model <model>] "
-                "[--layout <layout>] [--variant <variant>] [--options <options>]"
-                " <character/codepoint/keysym>\n", argv0);
+    fprintf(fp, "Usage: %s [--help] [--keysym] [--rules <rules>] "
+                "[--model <model>] [--layout <layout>] [--variant <variant>] "
+                "[--options <options>] [--enable-environment-names] "
+                "<character/codepoint/keysym>\n", argv0);
     fprintf(
         fp,
         "\n"
@@ -96,6 +97,14 @@ usage(const char *argv0, FILE *fp)
         "    The XKB layout variant (default: '%s')\n"
         " --options <options>\n"
         "    The XKB options (default: '%s')\n"
+        " --enable-environment-names\n"
+        "    Allow to set the default RMLVO values via the following environment variables:\n"
+        "    - XKB_DEFAULT_RULES\n"
+        "    - XKB_DEFAULT_MODEL\n"
+        "    - XKB_DEFAULT_LAYOUT\n"
+        "    - XKB_DEFAULT_VARIANT\n"
+        "    - XKB_DEFAULT_OPTIONS\n"
+        "    Note that this option may affect the default values of the previous options.\n"
         "\n",
         DEFAULT_XKB_RULES, DEFAULT_XKB_MODEL, DEFAULT_XKB_LAYOUT,
         DEFAULT_XKB_VARIANT ? DEFAULT_XKB_VARIANT : "<none>",
@@ -110,6 +119,7 @@ main(int argc, char *argv[])
     const char *layout_ = NULL;
     const char *variant = NULL;
     const char *options = NULL;
+    bool use_env_names = false;
     bool keysym_mode = false;
     int err = EXIT_FAILURE;
     struct xkb_context *ctx = NULL;
@@ -124,6 +134,7 @@ main(int argc, char *argv[])
     xkb_mod_index_t num_mods;
     enum options {
         OPT_KEYSYM,
+        OPT_ENABLE_ENV_NAMES,
         OPT_RULES,
         OPT_MODEL,
         OPT_LAYOUT,
@@ -133,6 +144,7 @@ main(int argc, char *argv[])
     static struct option opts[] = {
         {"help",                 no_argument,            0, 'h'},
         {"keysym",               no_argument,            0, OPT_KEYSYM},
+        {"enable-environment-names", no_argument,        0, OPT_ENABLE_ENV_NAMES},
         {"rules",                required_argument,      0, OPT_RULES},
         {"model",                required_argument,      0, OPT_MODEL},
         {"layout",               required_argument,      0, OPT_LAYOUT},
@@ -153,6 +165,9 @@ main(int argc, char *argv[])
         case OPT_KEYSYM:
             keysym_mode = true;
             break;
+        case OPT_ENABLE_ENV_NAMES:
+            use_env_names = true;
+            break;
         case OPT_RULES:
             rules = optarg;
             break;
@@ -211,7 +226,12 @@ main(int argc, char *argv[])
         goto err;
     }
 
-    ctx = xkb_context_new(XKB_CONTEXT_NO_FLAGS);
+
+    const enum xkb_context_flags ctx_flags = (use_env_names)
+                                           ? XKB_CONTEXT_NO_FLAGS
+                                           : XKB_CONTEXT_NO_ENVIRONMENT_NAMES;
+
+    ctx = xkb_context_new(ctx_flags);
     if (!ctx) {
         fprintf(stderr, "ERROR: Failed to create XKB context\n");
         goto err;

tools/interactive-evdev.c

@@ -362,7 +362,8 @@ usage(FILE *fp, char *progname)
 {
         fprintf(fp, "Usage: %s [--include=<path>] [--include-defaults] "
                 "[--rules=<rules>] [--model=<model>] [--layout=<layout>] "
-                "[--variant=<variant>] [--options=<options>]\n",
+                "[--variant=<variant>] [--options=<options>] "
+                "[--enable-environment-names]\n",
                 progname);
         fprintf(fp, "   or: %s --keymap <path to keymap file>\n",
                 progname);
@@ -391,6 +392,7 @@ main(int argc, char *argv[])
     struct xkb_compose_table *compose_table = NULL;
     const char *includes[64];
     size_t num_includes = 0;
+    bool use_env_names = false;
     const char *rules = NULL;
     const char *model = NULL;
     const char *layout = NULL;
@@ -403,6 +405,7 @@ main(int argc, char *argv[])
         OPT_VERBOSE,
         OPT_INCLUDE,
         OPT_INCLUDE_DEFAULTS,
+        OPT_ENABLE_ENV_NAMES,
         OPT_RULES,
         OPT_MODEL,
         OPT_LAYOUT,
@@ -423,6 +426,7 @@ main(int argc, char *argv[])
         {"verbose",              no_argument,            0, OPT_VERBOSE},
         {"include",              required_argument,      0, OPT_INCLUDE},
         {"include-defaults",     no_argument,            0, OPT_INCLUDE_DEFAULTS},
+        {"enable-environment-names", no_argument,        0, OPT_ENABLE_ENV_NAMES},
         {"rules",                required_argument,      0, OPT_RULES},
         {"model",                required_argument,      0, OPT_MODEL},
         {"layout",               required_argument,      0, OPT_LAYOUT},
@@ -463,6 +467,9 @@ main(int argc, char *argv[])
                 goto too_many_includes;
             includes[num_includes++] = DEFAULT_INCLUDE_PATH_PLACEHOLDER;
             break;
+        case OPT_ENABLE_ENV_NAMES:
+            use_env_names = true;
+            break;
         case OPT_RULES:
             if (keymap_path)
                 goto input_format_error;
@@ -548,7 +555,11 @@ main(int argc, char *argv[])
         }
     }
 
-    ctx = xkb_context_new(XKB_CONTEXT_NO_DEFAULT_INCLUDES);
+    enum xkb_context_flags ctx_flags = XKB_CONTEXT_NO_DEFAULT_INCLUDES;
+    if (!use_env_names)
+        ctx_flags |= XKB_CONTEXT_NO_ENVIRONMENT_NAMES;
+
+    ctx = xkb_context_new(ctx_flags);
     if (!ctx) {
         fprintf(stderr, "ERROR: Couldn't create xkb context\n");
         goto out;
@@ -584,11 +595,11 @@ main(int argc, char *argv[])
     }
     else {
         struct xkb_rule_names rmlvo = {
-            .rules = (rules == NULL || rules[0] == '\0') ? NULL : rules,
-            .model = (model == NULL || model[0] == '\0') ? NULL : model,
-            .layout = (layout == NULL || layout[0] == '\0') ? NULL : layout,
-            .variant = (variant == NULL || variant[0] == '\0') ? NULL : variant,
-            .options = (options == NULL || options[0] == '\0') ? NULL : options
+            .rules = (isempty(rules)) ? NULL : rules,
+            .model = (isempty(model)) ? NULL : model,
+            .layout = (isempty(layout)) ? NULL : layout,
+            .variant = (isempty(variant)) ? NULL : variant,
+            .options = (isempty(options)) ? NULL : options
         };
 
         if (!rules && !model && !layout && !variant && !options)