From 88a7f65c834ec09ce9df0c23c9935630e17a0c4a Mon Sep 17 00:00:00 2001 From: Profpatsch Date: Wed, 22 Jan 2020 23:22:23 +0100 Subject: [PATCH 1/7] lib/cli: unexport symbols & sort with generators MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit lib/cli is very similar to generators, so it should follow largely the same interface. Similar to how generators isn’t exported, we should also namespace cli by default (plus “cli” is only three characters to type). --- lib/default.nix | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/lib/default.nix b/lib/default.nix index 5abafe1b2ac..45530d47a53 100644 --- a/lib/default.nix +++ b/lib/default.nix @@ -37,11 +37,13 @@ let licenses = callLibs ./licenses.nix; systems = callLibs ./systems; + # serialization + cli = callLibs ./cli.nix; + generators = callLibs ./generators.nix; + # misc asserts = callLibs ./asserts.nix; - cli = callLibs ./cli.nix; debug = callLibs ./debug.nix; - generators = callLibs ./generators.nix; misc = callLibs ./deprecated.nix; # domain-specific @@ -121,7 +123,6 @@ let isOptionType mkOptionType; inherit (asserts) assertMsg assertOneOf; - inherit (cli) encodeGNUCommandLine toGNUCommandLine; inherit (debug) addErrorContextToAttrs traceIf traceVal traceValFn traceXMLVal traceXMLValMarked traceSeq traceSeqN traceValSeq traceValSeqFn traceValSeqN traceValSeqNFn traceShowVal From 582354d3b6161384cf69a90f6c00d7e29382950a Mon Sep 17 00:00:00 2001 From: Profpatsch Date: Wed, 22 Jan 2020 23:24:06 +0100 Subject: [PATCH 2/7] lib/cli: encodeGNUCommandLine -> toGNUCommandLineShell MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The semantic difference between `encode` and `to` is not apparent. Users are likely to confuse both functions (which leads to unexpected error messages about the wrong types). Like in `generators.nix`, all functions should be prefixed by `to`. Furthermore, converting to a string depends on the target context. In this case, it’s a POSIX shell, so we should name it that (compare `escapeShellArg` in `strings.nix`). We can later add versions that escape for embedding in e.g. python scripts or similar. --- lib/cli.nix | 26 ++++++++++++++++++++++++-- lib/tests/misc.nix | 29 +++++++++++++++++++++++++++-- 2 files changed, 51 insertions(+), 4 deletions(-) diff --git a/lib/cli.nix b/lib/cli.nix index f47625d2f53..32d24a00ceb 100644 --- a/lib/cli.nix +++ b/lib/cli.nix @@ -6,8 +6,29 @@ rec { This helps protect against malformed command lines and also to reduce boilerplate related to command-line construction for simple use cases. + `toGNUCommandLine` returns a list of nix strings. + `toGNUCommandLineShell` returns an escaped shell string. + Example: - encodeGNUCommandLine + toGNUCommandLine + { } + { data = builtins.toJSON { id = 0; }; + + X = "PUT"; + + retry = 3; + + retry-delay = null; + + url = [ "https://example.com/foo" "https://example.com/bar" ]; + + silent = false; + + verbose = true; + }; + => [ "-X" "PUT" "--data" "{\"id\":0}" "--retry" "3" "--url" "https://example.com/foo" "--url" "https://example.com/bar" "--verbose" ] + + toGNUCommandLineShell { } { data = builtins.toJSON { id = 0; }; @@ -24,8 +45,9 @@ rec { verbose = true; }; => "'-X' 'PUT' '--data' '{\"id\":0}' '--retry' '3' '--url' 'https://example.com/foo' '--url' 'https://example.com/bar' '--verbose'" + */ - encodeGNUCommandLine = + toGNUCommandLineShell = options: attrs: lib.escapeShellArgs (toGNUCommandLine options attrs); toGNUCommandLine = diff --git a/lib/tests/misc.nix b/lib/tests/misc.nix index e47b48b5017..b320839b2ac 100644 --- a/lib/tests/misc.nix +++ b/lib/tests/misc.nix @@ -441,9 +441,34 @@ runTests { expected = "«foo»"; }; - testRenderOptions = { + +# CLI + + testToGNUCommandLine = { expr = - encodeGNUCommandLine + cli.toGNUCommandLine + { } + { data = builtins.toJSON { id = 0; }; + + X = "PUT"; + + retry = 3; + + retry-delay = null; + + url = [ "https://example.com/foo" "https://example.com/bar" ]; + + silent = false; + + verbose = true; + }; + + expected = [ "-X" "PUT" "--data" "{\"id\":0}" "--retry" "3" "--url" "https://example.com/foo" "--url" "https://example.com/bar" "--verbose" ]; + }; + + testToGNUCommandLineShell = { + expr = + cli.toGNUCommandLineShell { } { data = builtins.toJSON { id = 0; }; From 6841f408cc7f42e3eaa59b06351008da2b1aa270 Mon Sep 17 00:00:00 2001 From: Profpatsch Date: Wed, 22 Jan 2020 23:31:58 +0100 Subject: [PATCH 3/7] CODEOWNERS: add Profpatsch to /lib/cli.nix and /lib/asserts.nix --- .github/CODEOWNERS | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index 0c06c59f378..f3ac8ad96b7 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -14,7 +14,9 @@ /lib @edolstra @nbp @infinisil /lib/systems @nbp @ericson2314 @matthewbauer /lib/generators.nix @edolstra @nbp @Profpatsch +/lib/cli.nix @edolstra @nbp @Profpatsch /lib/debug.nix @edolstra @nbp @Profpatsch +/lib/asserts.nix @edolstra @nbp @Profpatsch # Nixpkgs Internals /default.nix @nbp From b2654c226a83aa4cf5948f04ea6370796a2c7055 Mon Sep 17 00:00:00 2001 From: Profpatsch Date: Wed, 22 Jan 2020 23:37:10 +0100 Subject: [PATCH 4/7] lib/cli,lib/tests/misc: somewhat more standard formatting --- lib/cli.nix | 76 +++++++++++++++++++++------------------------- lib/tests/misc.nix | 61 ++++++++++++++++--------------------- 2 files changed, 61 insertions(+), 76 deletions(-) diff --git a/lib/cli.nix b/lib/cli.nix index 32d24a00ceb..a0e85c39605 100644 --- a/lib/cli.nix +++ b/lib/cli.nix @@ -10,49 +10,44 @@ rec { `toGNUCommandLineShell` returns an escaped shell string. Example: - toGNUCommandLine - { } - { data = builtins.toJSON { id = 0; }; - - X = "PUT"; - - retry = 3; - - retry-delay = null; - - url = [ "https://example.com/foo" "https://example.com/bar" ]; - - silent = false; - - verbose = true; - }; - => [ "-X" "PUT" "--data" "{\"id\":0}" "--retry" "3" "--url" "https://example.com/foo" "--url" "https://example.com/bar" "--verbose" ] - - toGNUCommandLineShell - { } - { data = builtins.toJSON { id = 0; }; - - X = "PUT"; - - retry = 3; - - retry-delay = null; - - url = [ "https://example.com/foo" "https://example.com/bar" ]; - - silent = false; - - verbose = true; - }; - => "'-X' 'PUT' '--data' '{\"id\":0}' '--retry' '3' '--url' 'https://example.com/foo' '--url' 'https://example.com/bar' '--verbose'" + cli.toGNUCommandLine {} { + data = builtins.toJSON { id = 0; }; + X = "PUT"; + retry = 3; + retry-delay = null; + url = [ "https://example.com/foo" "https://example.com/bar" ]; + silent = false; + verbose = true; + } + => [ + "-X" "PUT" + "--data" "{\"id\":0}" + "--retry" "3" + "--url" "https://example.com/foo" + "--url" "https://example.com/bar" + "--verbose" + ] + cli.toGNUCommandLineShell {} { + data = builtins.toJSON { id = 0; }; + X = "PUT"; + retry = 3; + retry-delay = null; + url = [ "https://example.com/foo" "https://example.com/bar" ]; + silent = false; + verbose = true; + } + => "'-X' 'PUT' '--data' '{\"id\":0}' '--retry' '3' '--url' 'https://example.com/foo' '--url' 'https://example.com/bar' '--verbose'"; */ toGNUCommandLineShell = options: attrs: lib.escapeShellArgs (toGNUCommandLine options attrs); toGNUCommandLine = { renderKey ? - key: if builtins.stringLength key == 1 then "-${key}" else "--${key}" + key: + if builtins.stringLength key == 1 + then "-${key}" + else "--${key}" , renderOption ? key: value: @@ -66,11 +61,10 @@ rec { }: options: let - render = key: value: - if builtins.isBool value - then renderBool key value - else if builtins.isList value - then renderList key value + render = + key: value: + if builtins.isBool value then renderBool key value + else if builtins.isList value then renderList key value else renderOption key value; in diff --git a/lib/tests/misc.nix b/lib/tests/misc.nix index b320839b2ac..59ed1e507e2 100644 --- a/lib/tests/misc.nix +++ b/lib/tests/misc.nix @@ -445,45 +445,36 @@ runTests { # CLI testToGNUCommandLine = { - expr = - cli.toGNUCommandLine - { } - { data = builtins.toJSON { id = 0; }; + expr = cli.toGNUCommandLine {} { + data = builtins.toJSON { id = 0; }; + X = "PUT"; + retry = 3; + retry-delay = null; + url = [ "https://example.com/foo" "https://example.com/bar" ]; + silent = false; + verbose = true; + }; - X = "PUT"; - - retry = 3; - - retry-delay = null; - - url = [ "https://example.com/foo" "https://example.com/bar" ]; - - silent = false; - - verbose = true; - }; - - expected = [ "-X" "PUT" "--data" "{\"id\":0}" "--retry" "3" "--url" "https://example.com/foo" "--url" "https://example.com/bar" "--verbose" ]; + expected = [ + "-X" "PUT" + "--data" "{\"id\":0}" + "--retry" "3" + "--url" "https://example.com/foo" + "--url" "https://example.com/bar" + "--verbose" + ]; }; testToGNUCommandLineShell = { - expr = - cli.toGNUCommandLineShell - { } - { data = builtins.toJSON { id = 0; }; - - X = "PUT"; - - retry = 3; - - retry-delay = null; - - url = [ "https://example.com/foo" "https://example.com/bar" ]; - - silent = false; - - verbose = true; - }; + expr = cli.toGNUCommandLineShell {} { + data = builtins.toJSON { id = 0; }; + X = "PUT"; + retry = 3; + retry-delay = null; + url = [ "https://example.com/foo" "https://example.com/bar" ]; + silent = false; + verbose = true; + }; expected = "'-X' 'PUT' '--data' '{\"id\":0}' '--retry' '3' '--url' 'https://example.com/foo' '--url' 'https://example.com/bar' '--verbose'"; }; From e71e1be8590e0f0dcac66e8b1e09695b94362fbd Mon Sep 17 00:00:00 2001 From: Profpatsch Date: Wed, 22 Jan 2020 23:55:42 +0100 Subject: [PATCH 5/7] lib/cli: rename `renderX` options to `mkX` Mirrors the naming scheme in `generators.nix`, for consistency. Also rename `key` to `k` and value to `v` to aid readability to the code structure. --- lib/cli.nix | 31 ++++++++++++++----------------- 1 file changed, 14 insertions(+), 17 deletions(-) diff --git a/lib/cli.nix b/lib/cli.nix index a0e85c39605..067ee21696a 100644 --- a/lib/cli.nix +++ b/lib/cli.nix @@ -43,29 +43,26 @@ rec { options: attrs: lib.escapeShellArgs (toGNUCommandLine options attrs); toGNUCommandLine = - { renderKey ? - key: - if builtins.stringLength key == 1 - then "-${key}" - else "--${key}" + { mkKey ? + k: if builtins.stringLength k == 1 + then "-${k}" + else "--${k}" - , renderOption ? - key: value: - if value == null - then [] - else [ (renderKey key) (builtins.toString value) ] + , mkOption ? + k: v: if v == null + then [] + else [ (mkKey k) (builtins.toString v) ] - , renderBool ? key: value: lib.optional value (renderKey key) + , mkBool ? k: v: lib.optional v (mkKey k) - , renderList ? key: value: lib.concatMap (renderOption key) value + , mkList ? k: v: lib.concatMap (mkOption k) v }: options: let - render = - key: value: - if builtins.isBool value then renderBool key value - else if builtins.isList value then renderList key value - else renderOption key value; + render = k: v: + if builtins.isBool v then mkBool k v + else if builtins.isList v then mkList k v + else mkOption k v; in builtins.concatLists (lib.mapAttrsToList render options); From 18520b7f3658f2007c7cbfcaea43edf2becdbb86 Mon Sep 17 00:00:00 2001 From: Profpatsch Date: Thu, 23 Jan 2020 01:07:02 +0100 Subject: [PATCH 6/7] lib/generators: floats are not supported in mkValueStringDefault They are cut off after a few decimal places; we cannot in good faith define a default string representation with that. --- lib/generators.nix | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/generators.nix b/lib/generators.nix index a71654bec6c..a64e94bd5cb 100644 --- a/lib/generators.nix +++ b/lib/generators.nix @@ -46,7 +46,10 @@ rec { else if isList v then err "lists" v # same as for lists, might want to replace else if isAttrs v then err "attrsets" v + # functions can’t be printed of course else if isFunction v then err "functions" v + # let’s not talk about floats. There is no sensible `toString` for them. + else if isFloat v then err "floats" v else err "this value is" (toString v); From 7228a3c0bcd80e24a18881b11048d3713042f256 Mon Sep 17 00:00:00 2001 From: Profpatsch Date: Thu, 23 Jan 2020 01:09:55 +0100 Subject: [PATCH 7/7] lib/cli: mkKey -> mkOptionName, use generators.mkValueStringDefault MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Let’s call them by what they are, option names. `generators.mkValueStringDefault` is a better value string renderer than plain `toString`. Also add docs to all options. --- lib/cli.nix | 36 +++++++++++++++++++++++++----------- 1 file changed, 25 insertions(+), 11 deletions(-) diff --git a/lib/cli.nix b/lib/cli.nix index 067ee21696a..c96d4dbb043 100644 --- a/lib/cli.nix +++ b/lib/cli.nix @@ -42,20 +42,34 @@ rec { toGNUCommandLineShell = options: attrs: lib.escapeShellArgs (toGNUCommandLine options attrs); - toGNUCommandLine = - { mkKey ? - k: if builtins.stringLength k == 1 - then "-${k}" - else "--${k}" + toGNUCommandLine = { + # how to string-format the option name; + # by default one character is a short option (`-`), + # more than one characters a long option (`--`). + mkOptionName ? + k: if builtins.stringLength k == 1 + then "-${k}" + else "--${k}", - , mkOption ? - k: v: if v == null - then [] - else [ (mkKey k) (builtins.toString v) ] + # how to format a boolean value to a command list; + # by default it’s a flag option + # (only the option name if true, left out completely if false). + mkBool ? k: v: lib.optional v (mkOptionName k), - , mkBool ? k: v: lib.optional v (mkKey k) + # how to format a list value to a command list; + # by default the option name is repeated for each value + # and `mkOption` is applied to the values themselves. + mkList ? k: v: lib.concatMap (mkOption k) v, - , mkList ? k: v: lib.concatMap (mkOption k) v + # how to format any remaining value to a command list; + # on the toplevel, booleans and lists are handled by `mkBool` and `mkList`, + # though they can still appear as values of a list. + # By default, everything is printed verbatim and complex types + # are forbidden (lists, attrsets, functions). `null` values are omitted. + mkOption ? + k: v: if v == null + then [] + else [ (mkOptionName k) (lib.generators.mkValueStringDefault {} v) ] }: options: let