From a60c7004ca2422294aea87b65ae6dab90eed44d3 Mon Sep 17 00:00:00 2001 From: Pierre Le Marre Date: Fri, 14 Feb 2025 10:35:49 +0100 Subject: [PATCH] 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. --- .../+enable-environment-names.breaking.md | 3 + test/tool-option-parsing.py | 13 --- tools/compile-keymap.c | 101 +++++++++++++----- tools/how-to-type.c | 28 ++++- tools/interactive-evdev.c | 25 +++-- tools/xkbcli-compile-keymap.1 | 17 +++ tools/xkbcli-how-to-type.1 | 16 +++ tools/xkbcli-interactive-evdev.1 | 16 +++ 8 files changed, 167 insertions(+), 52 deletions(-) create mode 100644 changes/tools/+enable-environment-names.breaking.md diff --git a/changes/tools/+enable-environment-names.breaking.md b/changes/tools/+enable-environment-names.breaking.md new file mode 100644 index 000000000..4864eebf2 --- /dev/null +++ b/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. diff --git a/test/tool-option-parsing.py b/test/tool-option-parsing.py index 66d699a42..e671da918 100755 --- a/test/tool-option-parsing.py +++ b/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" diff --git a/tools/compile-keymap.c b/tools/compile-keymap.c index ce8b71d10..e1798aad7 100644 --- a/tools/compile-keymap.c +++ b/tools/compile-keymap.c @@ -14,7 +14,7 @@ #include #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 \n" " Load the corresponding XKB file, ignore RMLVO options. If \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 \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,15 +435,8 @@ 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) { @@ -393,20 +444,14 @@ main(int argc, char **argv) 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) { diff --git a/tools/how-to-type.c b/tools/how-to-type.c index f74d4e739..8ce2c7bfd 100644 --- a/tools/how-to-type.c +++ b/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 ] [--model ] " - "[--layout ] [--variant ] [--options ]" - " \n", argv0); + fprintf(fp, "Usage: %s [--help] [--keysym] [--rules ] " + "[--model ] [--layout ] [--variant ] " + "[--options ] [--enable-environment-names] " + "\n", argv0); fprintf( fp, "\n" @@ -96,6 +97,14 @@ usage(const char *argv0, FILE *fp) " The XKB layout variant (default: '%s')\n" " --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 : "", @@ -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; diff --git a/tools/interactive-evdev.c b/tools/interactive-evdev.c index a515ab062..5c8dd9eca 100644 --- a/tools/interactive-evdev.c +++ b/tools/interactive-evdev.c @@ -362,7 +362,8 @@ usage(FILE *fp, char *progname) { fprintf(fp, "Usage: %s [--include=] [--include-defaults] " "[--rules=] [--model=] [--layout=] " - "[--variant=] [--options=]\n", + "[--variant=] [--options=] " + "[--enable-environment-names]\n", progname); fprintf(fp, " or: %s --keymap \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) diff --git a/tools/xkbcli-compile-keymap.1 b/tools/xkbcli-compile-keymap.1 index e56456041..0d395f358 100644 --- a/tools/xkbcli-compile-keymap.1 +++ b/tools/xkbcli-compile-keymap.1 @@ -66,6 +66,23 @@ The XKB layout variant . .It Fl \-options Ar options The XKB options +. +.It Fl \-enable\-environment\-names +Allow to set the default RMLVO values via the following environment variables: +.Bl -dash -compact -hang +.It +.Ev XKB_DEFAULT_RULES +.It +.Ev XKB_DEFAULT_MODEL +.It +.Ev XKB_DEFAULT_LAYOUT +.It +.Ev XKB_DEFAULT_VARIANT +.It +.Ev XKB_DEFAULT_OPTIONS +.El +Note that this option may affect the default values of the previous options. +. .El . .Sh SEE ALSO diff --git a/tools/xkbcli-how-to-type.1 b/tools/xkbcli-how-to-type.1 index 26d46373a..2aeadcefa 100644 --- a/tools/xkbcli-how-to-type.1 +++ b/tools/xkbcli-how-to-type.1 @@ -89,6 +89,22 @@ The XKB layout variant .It Fl \-options Ar options The XKB options . +.It Fl \-enable\-environment\-names +Allow to set the default RMLVO values via the following environment variables: +.Bl -dash -compact -hang +.It +.Ev XKB_DEFAULT_RULES +.It +.Ev XKB_DEFAULT_MODEL +.It +.Ev XKB_DEFAULT_LAYOUT +.It +.Ev XKB_DEFAULT_VARIANT +.It +.Ev XKB_DEFAULT_OPTIONS +.El +Note that this option may affect the default values of the previous options. +. .It Fl \-help Print a help message and exit. .El diff --git a/tools/xkbcli-interactive-evdev.1 b/tools/xkbcli-interactive-evdev.1 index 9f779f91f..59d4c8918 100644 --- a/tools/xkbcli-interactive-evdev.1 +++ b/tools/xkbcli-interactive-evdev.1 @@ -70,6 +70,22 @@ The XKB layout variant .It Fl \-option Ar options The XKB options . +.It Fl \-enable\-environment\-names +Allow to set the default RMLVO values via the following environment variables: +.Bl -dash -compact -hang +.It +.Ev XKB_DEFAULT_RULES +.It +.Ev XKB_DEFAULT_MODEL +.It +.Ev XKB_DEFAULT_LAYOUT +.It +.Ev XKB_DEFAULT_VARIANT +.It +.Ev XKB_DEFAULT_OPTIONS +.El +Note that this option may affect the default values of the previous options. +. .It Fl \-keymap Ar file Specify a keymap path. This option is mutually exclusive with the RMLVO options.