From ea10fe7ab0df085b51189adabdb079fc4442c6be Mon Sep 17 00:00:00 2001 From: Rebecca Turner Date: Thu, 28 Mar 2024 16:26:42 -0700 Subject: [PATCH 1/3] Add `EscapeStringOptions` and `escapeString` tests Change-Id: I86ead2f969c9e03c9edfa51bbc92ee06393fd7d6 --- src/libexpr/print.cc | 8 ++-- src/libutil/escape-string.cc | 57 ++++++++++++++++++--------- src/libutil/escape-string.hh | 60 +++++++++++++++++++++++++---- tests/unit/libutil/escape-string.cc | 35 +++++++++++++++++ tests/unit/meson.build | 1 + 5 files changed, 132 insertions(+), 29 deletions(-) create mode 100644 tests/unit/libutil/escape-string.cc diff --git a/src/libexpr/print.cc b/src/libexpr/print.cc index e5e6b9b21..8d7e2ab34 100644 --- a/src/libexpr/print.cc +++ b/src/libexpr/print.cc @@ -198,12 +198,14 @@ private: void printString(Value & v) { - // NB: Non-printing characters won't be escaped. escapeString( output, v.string.s, - options.maxStringLength, - options.ansiColors + { + .maxLength = options.maxStringLength, + .outputAnsiColors = options.ansiColors, + // NB: Non-printing characters won't be escaped. + } ); } diff --git a/src/libutil/escape-string.cc b/src/libutil/escape-string.cc index 8160403cd..c3cea96d2 100644 --- a/src/libutil/escape-string.cc +++ b/src/libutil/escape-string.cc @@ -11,30 +11,51 @@ namespace nix { std::ostream & -escapeString(std::ostream & str, const std::string_view string, size_t maxLength, bool ansiColors) +escapeString(std::ostream & output, std::string_view string, EscapeStringOptions options) { size_t charsPrinted = 0; - if (ansiColors) - str << ANSI_MAGENTA; - str << "\""; + if (options.outputAnsiColors) { + output << ANSI_MAGENTA; + } + output << "\""; for (auto i = string.begin(); i != string.end(); ++i) { - if (charsPrinted >= maxLength) { - str << "\" "; - printElided(str, string.length() - charsPrinted, "byte", "bytes", ansiColors); - return str; + if (charsPrinted >= options.maxLength) { + output << "\" "; + printElided( + output, string.length() - charsPrinted, "byte", "bytes", options.outputAnsiColors + ); + return output; + } + + if (*i == '\"' || *i == '\\') { + output << "\\" << *i; + } else if (*i == '\n') { + output << "\\n"; + } else if (*i == '\r') { + output << "\\r"; + } else if (*i == '\t') { + output << "\\t"; + } else if (*i == '$' && *(i + 1) == '{') { + output << "\\" << *i; + } else if (options.escapeNonPrinting && !isprint(*i)) { + output << MaybeHexEscapedChar{*i}; + } else { + output << *i; } - if (*i == '\"' || *i == '\\') str << "\\" << *i; - else if (*i == '\n') str << "\\n"; - else if (*i == '\r') str << "\\r"; - else if (*i == '\t') str << "\\t"; - else if (*i == '$' && *(i+1) == '{') str << "\\" << *i; - else str << *i; charsPrinted++; } - str << "\""; - if (ansiColors) - str << ANSI_NORMAL; - return str; + output << "\""; + if (options.outputAnsiColors) { + output << ANSI_NORMAL; + } + return output; +} + +std::string escapeString(std::string_view s, EscapeStringOptions options) +{ + std::ostringstream output; + escapeString(output, s, options); + return output.str(); } }; // namespace nix diff --git a/src/libutil/escape-string.hh b/src/libutil/escape-string.hh index 28c6c8d64..7f0a9e701 100644 --- a/src/libutil/escape-string.hh +++ b/src/libutil/escape-string.hh @@ -5,6 +5,41 @@ namespace nix { +/** + * Options for escaping strings in `escapeString`. + * + * With default optional parameters, the output string will round-trip through + * the Nix evaluator (i.e. you can copy/paste this function's output into the + * REPL and have it evaluate as the string that got passed in). + * + * With non-default optional parameters, the output string will be + * human-readable. + */ +struct EscapeStringOptions +{ + /** + * If `maxLength` is decreased, some trailing portion of the string may be + * omitted with a message like `«123 bytes elided»`. + */ + size_t maxLength = std::numeric_limits::max(); + + /** + * If `outputAnsiColors` is set, the string will be colored the color of literals, using + * ANSI escape codes. + */ + bool outputAnsiColors = false; + + /** + * If `escapeNonPrinting` is set, non-printing ASCII characters (i.e. with + * byte values less than 0x20) will be printed in `\xhh` format, like + * `\x1d` (other than those that Nix supports, like `\n`, `\r`, `\t`). + * Note that this format is not yet supported by the Lix parser/evaluator! + * + * See: https://git.lix.systems/lix-project/lix/issues/149 + */ + bool escapeNonPrinting = false; +}; + /** * Escape a string for output. * @@ -14,21 +49,30 @@ namespace nix { * * With non-default optional parameters, the output string will be * human-readable. + * + * See `EscapeStringOptions` for more details on customizing the output. */ +std::ostream & +escapeString(std::ostream & output, std::string_view s, EscapeStringOptions options = {}); -std::ostream & escapeString( - std::ostream & output, - const std::string_view string, - size_t maxLength = std::numeric_limits::max(), - bool ansiColors = false -); +inline std::ostream & escapeString(std::ostream & output, const char * s) +{ + return escapeString(output, std::string_view(s)); +} + +inline std::ostream & escapeString(std::ostream & output, const std::string & s) +{ + return escapeString(output, std::string_view(s)); +} /** * Escape a string for output, writing the escaped result to a new string. */ -inline std::ostream & escapeString(std::ostream & output, const char * string) +std::string escapeString(std::string_view s, EscapeStringOptions options = {}); + +inline std::string escapeString(const char * s, EscapeStringOptions options = {}) { - return escapeString(output, std::string_view(string)); + return escapeString(std::string_view(s), options); } } // namespace nix diff --git a/tests/unit/libutil/escape-string.cc b/tests/unit/libutil/escape-string.cc new file mode 100644 index 000000000..5ce2b73d8 --- /dev/null +++ b/tests/unit/libutil/escape-string.cc @@ -0,0 +1,35 @@ +#include "escape-string.hh" +#include "ansicolor.hh" +#include + +namespace nix { + +TEST(EscapeString, simple) { + auto escaped = escapeString("puppy"); + ASSERT_EQ(escaped, "\"puppy\""); +} + +TEST(EscapeString, escaping) { + auto escaped = escapeString("\n\r\t \" \\ ${ooga booga}"); + ASSERT_EQ(escaped, R"RAW("\n\r\t \" \\ \${ooga booga}")RAW"); +} + +TEST(EscapeString, maxLength) { + auto escaped = escapeString("puppy", {.maxLength = 5}); + ASSERT_EQ(escaped, "\"puppy\""); + + escaped = escapeString("puppy doggy", {.maxLength = 5}); + ASSERT_EQ(escaped, "\"puppy\" «6 bytes elided»"); +} + +TEST(EscapeString, ansiColors) { + auto escaped = escapeString("puppy doggy", {.maxLength = 5, .outputAnsiColors = true}); + ASSERT_EQ(escaped, ANSI_MAGENTA "\"puppy\" " ANSI_FAINT "«6 bytes elided»" ANSI_NORMAL); +} + +TEST(EscapeString, escapeNonPrinting) { + auto escaped = escapeString("puppy\u0005doggy", {.escapeNonPrinting = true}); + ASSERT_EQ(escaped, "\"puppy\\x05doggy\""); +} + +} // namespace nix diff --git a/tests/unit/meson.build b/tests/unit/meson.build index a5523a813..60bb2de89 100644 --- a/tests/unit/meson.build +++ b/tests/unit/meson.build @@ -39,6 +39,7 @@ libutil_tests_sources = files( 'libutil/closure.cc', 'libutil/compression.cc', 'libutil/config.cc', + 'libutil/escape-string.cc', 'libutil/git.cc', 'libutil/hash.cc', 'libutil/hilite.cc', From 9166babbaf5882ad2cfe1c7c9b30de1c153d70a8 Mon Sep 17 00:00:00 2001 From: Qyriad Date: Thu, 4 Apr 2024 11:57:45 -0600 Subject: [PATCH 2/3] Revert "meson: move nix3 manpage generation into command-ref/" This reverts commit 70954233743a233744787103d3211237a28ddbca. This seems to have broken running ninja on warm build directories, which is not what we want. Reverted until we figure out something better Change-Id: I9623ae078917e7c59a930bf8044a216501d4bb20 --- doc/manual/meson.build | 18 +++++++++++++----- doc/manual/src/command-ref/meson.build | 16 ---------------- 2 files changed, 13 insertions(+), 21 deletions(-) delete mode 100644 doc/manual/src/command-ref/meson.build diff --git a/doc/manual/meson.build b/doc/manual/meson.build index 72294311e..02b707ff3 100644 --- a/doc/manual/meson.build +++ b/doc/manual/meson.build @@ -132,10 +132,19 @@ nix3_cli_json = custom_target( capture : true, output : 'nix.json', ) - -# Creates nix3_cli_files custom target variable, -# which outputs the entire src/command-ref/new-cli directory. -subdir('src/command-ref') +nix3_cli_files = custom_target( + command : nix_eval_for_docs + [ + '--write-to', '@OUTPUT@', + '--expr', + 'import @INPUT1@ true (builtins.readFile @INPUT0@)', + ], + input : [ + nix3_cli_json, + 'generate-manpage.nix', + 'utils.nix', + ], + output : 'new-cli', +) nix3_manpages = [ 'nix3-build', @@ -236,7 +245,6 @@ foreach page : nix3_manpages '@OUTPUT@.tmp', '@OUTPUT@', ], - # nix3_cli_files set in doc/manual/src/command-ref/meson.build. input : nix3_cli_files, output : page + '.1', install : true, diff --git a/doc/manual/src/command-ref/meson.build b/doc/manual/src/command-ref/meson.build deleted file mode 100644 index c4043c882..000000000 --- a/doc/manual/src/command-ref/meson.build +++ /dev/null @@ -1,16 +0,0 @@ -nix3_cli_files = custom_target( - 'command-ref-new-cli', - command : nix_eval_for_docs + [ - '--write-to', '@OUTPUT@', - '--expr', - 'import @INPUT1@ true (builtins.readFile @INPUT0@)', - ], - input : [ - nix3_cli_json, - files( - meson.project_source_root() / 'doc/manual/generate-manpage.nix', - meson.project_source_root() / 'doc/manual/utils.nix', - ), - ], - output : 'new-cli', -) From bd59cbdfc96c82694af92e014cfb1a316a262f66 Mon Sep 17 00:00:00 2001 From: Qyriad Date: Wed, 3 Apr 2024 17:38:09 -0600 Subject: [PATCH 3/3] meson: add missing tests: ca, dyn-drv, plugins, libstoreconsumer Change-Id: I6a74ebaf93697cb99aadd6b51538c2766b0a808a --- tests/functional/meson.build | 47 +++++++++++++++++++ tests/functional/plugins.sh | 11 ++++- tests/functional/plugins/meson.build | 30 ++++++++++++ .../test-libstoreconsumer/meson.build | 13 +++++ 4 files changed, 99 insertions(+), 2 deletions(-) create mode 100644 tests/functional/plugins/meson.build create mode 100644 tests/functional/test-libstoreconsumer/meson.build diff --git a/tests/functional/meson.build b/tests/functional/meson.build index 0ea0e4df5..d90230624 100644 --- a/tests/functional/meson.build +++ b/tests/functional/meson.build @@ -25,9 +25,37 @@ subdir('ca') # Just configures `dyn-drv/config.nix.in`. Same as above. subdir('dyn-drv') +subdir('plugins') +subdir('test-libstoreconsumer') + functional_tests_scripts = [ 'init.sh', 'test-infra.sh', + 'ca/build.sh', + 'ca/build-cache.sh', + 'ca/concurrent-builds.sh', + 'ca/derivation-json.sh', + 'ca/duplicate-realisation-in-closure.sh', + 'ca/eval-store.sh', + 'ca/gc.sh', + 'ca/import-derivation.sh', + 'ca/new-build-cmd.sh', + 'ca/nix-copy.sh', + 'ca/nix-run.sh', + 'ca/nix-shell.sh', + 'ca/post-hook.sh', + 'ca/recursive.sh', + 'ca/repl.sh', + 'ca/selfref-gc.sh', + 'ca/signatures.sh', + 'ca/substitute.sh', + 'ca/why-depends.sh', + 'dyn-drv/text-hashed-output.sh', + 'dyn-drv/recursive-mod-json.sh', + 'dyn-drv/build-built-drv.sh', + 'dyn-drv/eval-outputOf.sh', + 'dyn-drv/dep-built-drv.sh', + 'dyn-drv/old-daemon-error-hack.sh', 'flakes/flakes.sh', 'flakes/develop.sh', 'flakes/develop-r8854.sh', @@ -152,6 +180,8 @@ functional_tests_scripts = [ 'read-only-store.sh', 'nested-sandboxing.sh', 'debugger.sh', + 'plugins.sh', + 'test-libstoreconsumer.sh', ] # TODO(Qyriad): this will hopefully be able to be removed when we remove the autoconf+Make @@ -165,6 +195,21 @@ foreach script : functional_tests_scripts # Turns, e.g., `tests/functional/flakes/show.sh` into a Meson test target called # `functional-flakes-show`. name = 'functional-@0@'.format(fs.replace_suffix(script, '')).replace('/', '-') + + extra_deps = [] + if script == 'plugins.sh' + extra_deps += [ + # Set in tests/functional/plugins/meson.build + libplugintest, + libplugintestfail, + ] + elif script == 'test-libstoreconsumer.sh' + extra_deps += [ + # Set in tests/functional/test-libstoreconsumer/meson.build + libstoreconsumer_tester, + ] + endif + test( name, python, @@ -179,5 +224,7 @@ foreach script : functional_tests_scripts # some tests take 15+ seconds even on an otherwise idle machine, on a loaded machine # this can easily drive them to failure. give them more time, 5min rather than 30sec timeout : 300, + # Used for target dependency/ordering tracking, not adding compiler flags or anything. + depends : extra_deps, ) endforeach diff --git a/tests/functional/plugins.sh b/tests/functional/plugins.sh index 491b933b7..6df10147b 100644 --- a/tests/functional/plugins.sh +++ b/tests/functional/plugins.sh @@ -4,12 +4,19 @@ if [[ $BUILD_SHARED_LIBS != 1 ]]; then skipTest "Plugins are not supported" fi -res=$(nix --option setting-set true --option plugin-files $PWD/plugins/libplugintest.* eval --expr builtins.anotherNull) +# FIXME(Qyriad): this is working around Meson putting `libplugintest.so.p` in the same place +# as `libplugintest.so`, so `libplugintest.*` grabs both. +libext=so +if [[ "$(uname -s)" == "Darwin" ]]; then + libext=dylib +fi + +res=$(nix --option setting-set true --option plugin-files "$PWD/plugins/libplugintest.$libext" eval --expr builtins.anotherNull) [ "$res"x = "nullx" ] # Plugin load failing due to missing symbols -res=$(nix --option plugin-files $PWD/plugins/libplugintestfail.* eval --expr '1234 + 5' 2>&1) +res=$(nix --option plugin-files "$PWD/plugins/libplugintestfail.$libext" eval --expr '1234 + 5' 2>&1) # We expect this to succeed evaluating echo "$res" | grep 1239 >/dev/null # On Linux, we expect this to print some failure of dlopen. diff --git a/tests/functional/plugins/meson.build b/tests/functional/plugins/meson.build new file mode 100644 index 000000000..ba941b103 --- /dev/null +++ b/tests/functional/plugins/meson.build @@ -0,0 +1,30 @@ +libplugintest = library( + 'plugintest', + 'plugintest.cc', + dependencies : [ + liblixutil, + liblixstore, + liblixexpr, + liblixfetchers, + ], + build_by_default : false, +) + +libplugintestfail_link_args = [] +if is_linux + libplugintestfail_link_args = ['-z', 'now'] +endif + +libplugintestfail = shared_module( + 'plugintestfail', + 'plugintestfail.cc', + dependencies : [ + liblixutil, + liblixstore, + liblixexpr, + liblixfetchers, + ], + cpp_args : ['-DMISSING_REFERENCE'], + link_args : libplugintestfail_link_args, + build_by_default : false, +) diff --git a/tests/functional/test-libstoreconsumer/meson.build b/tests/functional/test-libstoreconsumer/meson.build new file mode 100644 index 000000000..ad96aac12 --- /dev/null +++ b/tests/functional/test-libstoreconsumer/meson.build @@ -0,0 +1,13 @@ +libstoreconsumer_tester = executable( + 'test-libstoreconsumer', + 'main.cc', + dependencies : [ + liblixutil, + liblixstore, + sodium, + editline, + boost, + lowdown, + ], + build_by_default : false, +)