From 02c41aba5b5b135bae0de4961fcf4d3a888a9524 Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Fri, 5 Apr 2024 16:08:18 +0200 Subject: [PATCH 1/9] libexpr-c: Add nix_string_realise --- src/libexpr-c/nix_api_expr_internal.h | 6 ++ src/libexpr-c/nix_api_value.cc | 55 ++++++++++++ src/libexpr-c/nix_api_value.h | 60 ++++++++++++- src/libexpr/eval.hh | 8 +- src/libexpr/primops.cc | 21 +++-- tests/unit/libexpr/nix_api_expr.cc | 85 +++++++++++++++++++ .../libutil-support/tests/nix_api_util.hh | 10 +++ 7 files changed, 235 insertions(+), 10 deletions(-) diff --git a/src/libexpr-c/nix_api_expr_internal.h b/src/libexpr-c/nix_api_expr_internal.h index b50a51347..7743849fd 100644 --- a/src/libexpr-c/nix_api_expr_internal.h +++ b/src/libexpr-c/nix_api_expr_internal.h @@ -35,4 +35,10 @@ struct nix_string_context nix::NixStringContext & ctx; }; +struct nix_realised_string +{ + std::string str; + std::vector storePaths; +}; + #endif // NIX_API_EXPR_INTERNAL_H diff --git a/src/libexpr-c/nix_api_value.cc b/src/libexpr-c/nix_api_value.cc index 02bd154b3..fd1bfc165 100644 --- a/src/libexpr-c/nix_api_value.cc +++ b/src/libexpr-c/nix_api_value.cc @@ -2,6 +2,7 @@ #include "config.hh" #include "eval.hh" #include "globals.hh" +#include "path.hh" #include "primops.hh" #include "value.hh" @@ -9,7 +10,9 @@ #include "nix_api_expr_internal.h" #include "nix_api_util.h" #include "nix_api_util_internal.h" +#include "nix_api_store_internal.h" #include "nix_api_value.h" +#include "value/context.hh" #ifdef HAVE_BOEHMGC # include "gc/gc.h" @@ -528,3 +531,55 @@ void nix_bindings_builder_free(BindingsBuilder * bb) delete (nix::BindingsBuilder *) bb; #endif } + +nix_realised_string * nix_string_realise(nix_c_context * context, EvalState * state, Value * value, bool isIFD) +{ + if (context) + context->last_err_code = NIX_OK; + try { + auto &v = check_value_not_null(value); + nix::NixStringContext stringContext; + auto rawStr = state->state.coerceToString(nix::noPos, v, stringContext, "while realising a string").toOwned(); + nix::StorePathSet storePaths; + auto rewrites = state->state.realiseContext(stringContext, &storePaths); + + auto s = nix::rewriteStrings(rawStr, rewrites); + + // Convert to the C API StorePath type and convert to vector for index-based access + std::vector vec; + for (auto &sp : storePaths) { + vec.push_back(StorePath{sp}); + } + + return new nix_realised_string { + .str = s, + .storePaths = vec + }; + } + NIXC_CATCH_ERRS_NULL +} + +void nix_realised_string_free(nix_realised_string * s) +{ + delete s; +} + +size_t nix_realised_string_get_buffer_size(nix_realised_string * s) +{ + return s->str.size(); +} + +const char * nix_realised_string_get_buffer_start(nix_realised_string * s) +{ + return s->str.data(); +} + +size_t nix_realised_string_get_store_path_count(nix_realised_string * s) +{ + return s->storePaths.size(); +} + +const StorePath * nix_realised_string_get_store_path(nix_realised_string * s, size_t i) +{ + return &s->storePaths[i]; +} diff --git a/src/libexpr-c/nix_api_value.h b/src/libexpr-c/nix_api_value.h index 42218188c..c8e85d181 100644 --- a/src/libexpr-c/nix_api_value.h +++ b/src/libexpr-c/nix_api_value.h @@ -9,6 +9,7 @@ */ #include "nix_api_util.h" +#include "nix_api_store.h" #include "stdbool.h" #include "stddef.h" #include "stdint.h" @@ -69,6 +70,10 @@ typedef struct PrimOp PrimOp; */ typedef struct ExternalValue ExternalValue; +/** @brief String without placeholders, and realised store paths + */ +typedef struct nix_realised_string nix_realised_string; + /** @defgroup primops * @brief Create your own primops * @{ @@ -167,7 +172,10 @@ const char * nix_get_typename(nix_c_context * context, const Value * value); */ bool nix_get_bool(nix_c_context * context, const Value * value); -/** @brief Get string +/** @brief Get the raw string + * + * This may contain placeholders. + * * @param[out] context Optional, stores error information * @param[in] value Nix value to inspect * @return string @@ -425,6 +433,56 @@ nix_bindings_builder_insert(nix_c_context * context, BindingsBuilder * builder, void nix_bindings_builder_free(BindingsBuilder * builder); /**@}*/ +/** @brief Realise a string context. + * + * This will + * - realise the store paths referenced by the string's context, and + * - perform the replacement of placeholders. + * - create temporary garbage collection roots for the store paths, for + * the lifetime of the current process. + * - log to stderr + * + * @param[out] context Optional, stores error information + * @param[in] value Nix value, which must be a string + * @param[in] state Nix evaluator state + * @param[in] isIFD If true, disallow derivation outputs if setting `allow-import-from-derivation` is false. + You should set this to true when this call is part of a primop. + You should set this to false when building for your application's purpose. + * @return NULL if failed, are a new nix_realised_string, which must be freed with nix_realised_string_free + */ +nix_realised_string * nix_string_realise(nix_c_context * context, EvalState * state, Value * value, bool isIFD); + +/** @brief Start of the string + * @param[in] realised_string + * @return pointer to the start of the string. It may not be null-terminated. + */ +const char * nix_realised_string_get_buffer_start(nix_realised_string * realised_string); + +/** @brief Length of the string + * @param[in] realised_string + * @return length of the string in bytes + */ +size_t nix_realised_string_get_buffer_size(nix_realised_string * realised_string); + +/** @brief Number of realised store paths + * @param[in] realised_string + * @return number of realised store paths that were referenced by the string via its context + */ +size_t nix_realised_string_get_store_path_count(nix_realised_string * realised_string); + +/** @brief Get a store path. The store paths are stored in an arbitrary order. + * @param[in] realised_string + * @param[in] index index of the store path, must be less than the count + * @return store path + */ +const StorePath * nix_realised_string_get_store_path(nix_realised_string * realised_string, size_t index); + +/** @brief Free a realised string + * @param[in] realised_string + */ +void nix_realised_string_free(nix_realised_string * realised_string); + + // cffi end #ifdef __cplusplus } diff --git a/src/libexpr/eval.hh b/src/libexpr/eval.hh index 4d53bcde6..2741220f1 100644 --- a/src/libexpr/eval.hh +++ b/src/libexpr/eval.hh @@ -733,10 +733,12 @@ public: bool fullGC(); /** - * Realise the given context, and return a mapping from the placeholders - * used to construct the associated value to their final store path + * Realise the given context + * @param[in] context the context to realise + * @param[out] maybePaths if not nullptr, all built or referenced store paths will be added to this set + * @return a mapping from the placeholders used to construct the associated value to their final store path. */ - [[nodiscard]] StringMap realiseContext(const NixStringContext & context); + [[nodiscard]] StringMap realiseContext(const NixStringContext & context, StorePathSet * maybePaths = nullptr, bool isIFD = true); /* Call the binary path filter predicate used builtins.path etc. */ bool callPathFilter( diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index 752176178..e446199ae 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -39,7 +39,7 @@ namespace nix { * Miscellaneous *************************************************************/ -StringMap EvalState::realiseContext(const NixStringContext & context) +StringMap EvalState::realiseContext(const NixStringContext & context, StorePathSet * maybePathsOut, bool isIFD) { std::vector drvs; StringMap res; @@ -61,19 +61,23 @@ StringMap EvalState::realiseContext(const NixStringContext & context) auto ctxS = store->printStorePath(o.path); res.insert_or_assign(ctxS, ctxS); ensureValid(o.path); + if (maybePathsOut) + maybePathsOut->emplace(o.path); }, [&](const NixStringContextElem::DrvDeep & d) { /* Treat same as Opaque */ auto ctxS = store->printStorePath(d.drvPath); res.insert_or_assign(ctxS, ctxS); ensureValid(d.drvPath); + if (maybePathsOut) + maybePathsOut->emplace(d.drvPath); }, }, c.raw); } if (drvs.empty()) return {}; - if (!evalSettings.enableImportFromDerivation) + if (isIFD && !evalSettings.enableImportFromDerivation) error( "cannot build '%1%' during evaluation because the option 'allow-import-from-derivation' is disabled", drvs.begin()->to_string(*store) @@ -90,6 +94,8 @@ StringMap EvalState::realiseContext(const NixStringContext & context) auto outputs = resolveDerivedPath(*buildStore, drv, &*store); for (auto & [outputName, outputPath] : outputs) { outputsToCopyAndAllow.insert(outputPath); + if (maybePathsOut) + maybePathsOut->emplace(outputPath); /* Get all the output paths corresponding to the placeholders we had */ if (experimentalFeatureSettings.isEnabled(Xp::CaDerivations)) { @@ -106,10 +112,13 @@ StringMap EvalState::realiseContext(const NixStringContext & context) } if (store != buildStore) copyClosure(*buildStore, *store, outputsToCopyAndAllow); - for (auto & outputPath : outputsToCopyAndAllow) { - /* Add the output of this derivations to the allowed - paths. */ - allowPath(outputPath); + + if (isIFD) { + for (auto & outputPath : outputsToCopyAndAllow) { + /* Add the output of this derivations to the allowed + paths. */ + allowPath(outputPath); + } } return res; diff --git a/tests/unit/libexpr/nix_api_expr.cc b/tests/unit/libexpr/nix_api_expr.cc index 9d54a62f8..3808bf0eb 100644 --- a/tests/unit/libexpr/nix_api_expr.cc +++ b/tests/unit/libexpr/nix_api_expr.cc @@ -7,6 +7,7 @@ #include "tests/nix_api_expr.hh" +#include "gmock/gmock.h" #include namespace nixC { @@ -97,4 +98,88 @@ TEST_F(nix_api_expr_test, nix_build_drv) nix_store_path_free(drvStorePath); nix_store_path_free(outStorePath); } + +TEST_F(nix_api_expr_test, nix_expr_realise_context_bad_value) +{ + auto expr = "true"; + nix_expr_eval_from_string(ctx, state, expr, ".", value); + assert_ctx_ok(); + auto r = nix_string_realise(ctx, state, value, false); + ASSERT_EQ(nullptr, r); + ASSERT_EQ(ctx->last_err_code, NIX_ERR_NIX_ERROR); + ASSERT_THAT(ctx->last_err, testing::Optional(testing::HasSubstr("cannot coerce"))); } + +TEST_F(nix_api_expr_test, nix_expr_realise_context_bad_build) +{ + auto expr = R"( + derivation { name = "letsbuild"; + system = builtins.currentSystem; + builder = "/bin/sh"; + args = [ "-c" "echo failing a build for testing purposes; exit 1;" ]; + } + )"; + nix_expr_eval_from_string(ctx, state, expr, ".", value); + assert_ctx_ok(); + auto r = nix_string_realise(ctx, state, value, false); + ASSERT_EQ(nullptr, r); + ASSERT_EQ(ctx->last_err_code, NIX_ERR_NIX_ERROR); + ASSERT_THAT(ctx->last_err, testing::Optional(testing::HasSubstr("failed with exit code 1"))); +} + +TEST_F(nix_api_expr_test, nix_expr_realise_context) +{ + // TODO (ca-derivations): add a content-addressed derivation output, which produces a placeholder + auto expr = R"( + '' + a derivation output: ${ + derivation { name = "letsbuild"; + system = builtins.currentSystem; + builder = "/bin/sh"; + args = [ "-c" "echo foo > $out" ]; + }} + a path: ${builtins.toFile "just-a-file" "ooh file good"} + a derivation path by itself: ${ + builtins.unsafeDiscardOutputDependency + (derivation { + name = "not-actually-built-yet"; + system = builtins.currentSystem; + builder = "/bin/sh"; + args = [ "-c" "echo foo > $out" ]; + }).drvPath} + '' + )"; + nix_expr_eval_from_string(ctx, state, expr, ".", value); + assert_ctx_ok(); + auto r = nix_string_realise(ctx, state, value, false); + assert_ctx_ok(); + ASSERT_NE(nullptr, r); + + auto s = std::string(nix_realised_string_get_buffer_start(r), nix_realised_string_get_buffer_size(r)); + + EXPECT_THAT(s, testing::StartsWith("a derivation output:")); + EXPECT_THAT(s, testing::HasSubstr("-letsbuild\n")); + EXPECT_THAT(s, testing::Not(testing::HasSubstr("-letsbuild.drv"))); + EXPECT_THAT(s, testing::HasSubstr("a path:")); + EXPECT_THAT(s, testing::HasSubstr("-just-a-file")); + EXPECT_THAT(s, testing::Not(testing::HasSubstr("-just-a-file.drv"))); + EXPECT_THAT(s, testing::Not(testing::HasSubstr("ooh file good"))); + EXPECT_THAT(s, testing::HasSubstr("a derivation path by itself:")); + EXPECT_THAT(s, testing::EndsWith("-not-actually-built-yet.drv\n")); + + std::vector names; + size_t n = nix_realised_string_get_store_path_count(r); + for (size_t i = 0; i < n; ++i) { + const StorePath * p = nix_realised_string_get_store_path(r, i); + names.push_back(p->path.name()); + } + std::sort(names.begin(), names.end()); + ASSERT_EQ(3, names.size()); + EXPECT_THAT(names[0], testing::StrEq("just-a-file")); + EXPECT_THAT(names[1], testing::StrEq("letsbuild")); + EXPECT_THAT(names[2], testing::StrEq("not-actually-built-yet.drv")); + + nix_realised_string_free(r); +} + +} // namespace nixC diff --git a/tests/unit/libutil-support/tests/nix_api_util.hh b/tests/unit/libutil-support/tests/nix_api_util.hh index 0dfb38f7b..75d302bd6 100644 --- a/tests/unit/libutil-support/tests/nix_api_util.hh +++ b/tests/unit/libutil-support/tests/nix_api_util.hh @@ -23,5 +23,15 @@ protected: } nix_c_context * ctx; + + inline void assert_ctx_ok() { + if (nix_err_code(ctx) == NIX_OK) { + return; + } + unsigned int n; + const char * p = nix_err_msg(nullptr, ctx, &n); + std::string msg(p, n); + FAIL() << "nix_err_code(ctx) != NIX_OK, message: " << msg; + } }; } From c145ce0e1a01f54f7ddf7d38909cf6bd8ec32a88 Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Fri, 5 Apr 2024 16:15:43 +0200 Subject: [PATCH 2/9] realiseContext: Remove no-op replacements A possible use of them might have been to figure out the paths (which can now be retrieved with maybePathsOut), but I have not found evidence that it was used this way, and it would have been broken, because non-CA outputs weren't recorded in the map. --- src/libexpr/primops.cc | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index e446199ae..5fd3256a5 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -59,7 +59,6 @@ StringMap EvalState::realiseContext(const NixStringContext & context, StorePathS }, [&](const NixStringContextElem::Opaque & o) { auto ctxS = store->printStorePath(o.path); - res.insert_or_assign(ctxS, ctxS); ensureValid(o.path); if (maybePathsOut) maybePathsOut->emplace(o.path); @@ -67,7 +66,6 @@ StringMap EvalState::realiseContext(const NixStringContext & context, StorePathS [&](const NixStringContextElem::DrvDeep & d) { /* Treat same as Opaque */ auto ctxS = store->printStorePath(d.drvPath); - res.insert_or_assign(ctxS, ctxS); ensureValid(d.drvPath); if (maybePathsOut) maybePathsOut->emplace(d.drvPath); From ed13cf05a224a08e4f120e0c932289db2d20be84 Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Sun, 7 Apr 2024 16:31:57 +0200 Subject: [PATCH 3/9] build-hook: Allow empty Like always declining; local builds only, as can be inferred from the docs. (Not worth spending too many words on this pretty obvious behavior, I think. Also, plans to remove it? https://github.com/NixOS/nix/issues/1221) --- src/libstore/build/derivation-goal.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libstore/build/derivation-goal.cc b/src/libstore/build/derivation-goal.cc index 29bf2852f..4d4342996 100644 --- a/src/libstore/build/derivation-goal.cc +++ b/src/libstore/build/derivation-goal.cc @@ -1138,7 +1138,7 @@ void DerivationGoal::resolvedFinished() HookReply DerivationGoal::tryBuildHook() { - if (!worker.tryBuildHook || !useDerivation) return rpDecline; + if (settings.buildHook.get().empty() || !worker.tryBuildHook || !useDerivation) return rpDecline; if (!worker.hook) worker.hook = std::make_unique(); From 94d9819bdc9dfa0a62c1e75fc90b2df0409be1ec Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Sun, 7 Apr 2024 16:36:05 +0200 Subject: [PATCH 4/9] tests/unit/libexpr/main: Fix realisation --- tests/unit/libexpr/main.cc | 39 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) create mode 100644 tests/unit/libexpr/main.cc diff --git a/tests/unit/libexpr/main.cc b/tests/unit/libexpr/main.cc new file mode 100644 index 000000000..cf7fcf5a3 --- /dev/null +++ b/tests/unit/libexpr/main.cc @@ -0,0 +1,39 @@ +#include +#include +#include "globals.hh" +#include "logging.hh" + +using namespace nix; + +int main (int argc, char **argv) { + if (argc > 1 && std::string_view(argv[1]) == "__build-remote") { + printError("test-build-remote: not supported in libexpr unit tests"); + return 1; + } + + // Disable build hook. We won't be testing remote builds in these unit tests. If we do, fix the above build hook. + settings.buildHook = {}; + + #if __linux__ // should match the conditional around sandboxBuildDir declaration. + + // When building and testing nix within the host's Nix sandbox, our store dir will be located in the host's sandboxBuildDir, e.g.: + // Host + // storeDir = /nix/store + // sandboxBuildDir = /build + // This process + // storeDir = /build/foo/bar/store + // sandboxBuildDir = /build + // However, we have a rule that the store dir must not be inside the storeDir, so we need to pick a different sandboxBuildDir. + settings.sandboxBuildDir = "/test-build-dir-instead-of-usual-build-dir"; + #endif + + #if __APPLE__ + // Avoid this error, when already running in a sandbox: + // sandbox-exec: sandbox_apply: Operation not permitted + settings.sandboxMode = smDisabled; + setEnv("_NIX_TEST_NO_SANDBOX", "1"); + #endif + + ::testing::InitGoogleTest(&argc, argv); + return RUN_ALL_TESTS(); +} From 48808a53205ceec94bad761266c4325fd28963d1 Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Sun, 7 Apr 2024 21:19:44 +0200 Subject: [PATCH 5/9] tests/unit/libexpr: Enable nix_store_realise test, and add docs --- src/libstore-c/nix_api_store.h | 4 +++- tests/unit/libexpr/nix_api_expr.cc | 11 ++++++----- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/src/libstore-c/nix_api_store.h b/src/libstore-c/nix_api_store.h index 1309f99b7..577b70151 100644 --- a/src/libstore-c/nix_api_store.h +++ b/src/libstore-c/nix_api_store.h @@ -111,7 +111,9 @@ bool nix_store_is_valid_path(nix_c_context * context, Store * store, StorePath * /** * @brief Realise a Nix store path * - * Blocking, calls callback once for each realised output + * Blocking, calls callback once for each realised output. + * + * @note When working with expressions, consider using e.g. nix_string_realise to get the output. `.drvPath` may not be accurate or available in the future. See https://github.com/NixOS/nix/issues/6507 * * @param[out] context Optional, stores error information * @param[in] store Nix Store reference diff --git a/tests/unit/libexpr/nix_api_expr.cc b/tests/unit/libexpr/nix_api_expr.cc index 3808bf0eb..fad078d0c 100644 --- a/tests/unit/libexpr/nix_api_expr.cc +++ b/tests/unit/libexpr/nix_api_expr.cc @@ -74,6 +74,9 @@ TEST_F(nix_api_expr_test, nix_build_drv) std::string pEnd = "-myname.drv"; ASSERT_EQ(pEnd, p.substr(p.size() - pEnd.size())); + // NOTE: .drvPath should be usually be ignored. Output paths are more versatile. + // See https://github.com/NixOS/nix/issues/6507 + // Use e.g. nix_string_realise to realise the output. StorePath * drvStorePath = nix_store_parse_path(ctx, store, drvPath); ASSERT_EQ(true, nix_store_is_valid_path(ctx, store, drvStorePath)); @@ -88,11 +91,9 @@ TEST_F(nix_api_expr_test, nix_build_drv) StorePath * outStorePath = nix_store_parse_path(ctx, store, outPath); ASSERT_EQ(false, nix_store_is_valid_path(ctx, store, outStorePath)); - // TODO figure out why fails. - // `make libexpr-tests_RUN` works, but `nix build .` enters an infinite loop - /* nix_store_realise(ctx, store, drvStorePath, nullptr, nullptr); */ - /* auto is_valid_path = nix_store_is_valid_path(ctx, store, outStorePath); */ - /* ASSERT_EQ(true, is_valid_path); */ + nix_store_realise(ctx, store, drvStorePath, nullptr, nullptr); + auto is_valid_path = nix_store_is_valid_path(ctx, store, outStorePath); + ASSERT_EQ(true, is_valid_path); // Clean up nix_store_path_free(drvStorePath); From 1233bcde37e25a41f2230a64aded9bd5813098cd Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Mon, 8 Apr 2024 10:34:58 +0200 Subject: [PATCH 6/9] libstore-c: Add nix_store_path_clone --- src/libstore-c/nix_api_store.cc | 5 +++++ src/libstore-c/nix_api_store.h | 8 ++++++++ 2 files changed, 13 insertions(+) diff --git a/src/libstore-c/nix_api_store.cc b/src/libstore-c/nix_api_store.cc index d80ba332e..d044ba14f 100644 --- a/src/libstore-c/nix_api_store.cc +++ b/src/libstore-c/nix_api_store.cc @@ -132,3 +132,8 @@ void nix_store_path_free(StorePath * sp) { delete sp; } + +StorePath * nix_store_path_clone(const StorePath * p) +{ + return new StorePath{p->path}; +} diff --git a/src/libstore-c/nix_api_store.h b/src/libstore-c/nix_api_store.h index 577b70151..ab86a81df 100644 --- a/src/libstore-c/nix_api_store.h +++ b/src/libstore-c/nix_api_store.h @@ -90,6 +90,14 @@ nix_err nix_store_get_uri(nix_c_context * context, Store * store, void * callbac */ StorePath * nix_store_parse_path(nix_c_context * context, Store * store, const char * path); +/** + * @brief Copy a StorePath + * + * @param[in] p the path to copy + * @return a new StorePath + */ +StorePath * nix_store_path_clone(const StorePath * p); + /** @brief Deallocate a StorePath * * Does not fail. From 876e70bc9afb15b5ee71d2f6b3090acca315f320 Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Mon, 8 Apr 2024 13:07:36 +0200 Subject: [PATCH 7/9] tests/unit/libexpr/local.mk A proper build system would catch errors like this. --- tests/unit/libexpr/local.mk | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/libexpr/local.mk b/tests/unit/libexpr/local.mk index 8df1a5207..c59191db4 100644 --- a/tests/unit/libexpr/local.mk +++ b/tests/unit/libexpr/local.mk @@ -34,7 +34,7 @@ libexpr-tests_EXTRA_INCLUDES = \ libexpr-tests_CXXFLAGS += $(libexpr-tests_EXTRA_INCLUDES) libexpr-tests_LIBS = \ - libexpr-test-support libstore-test-support libutils-test-support \ + libexpr-test-support libstore-test-support libutil-test-support \ libexpr libexprc libfetchers libstore libstorec libutil libutilc libexpr-tests_LDFLAGS := -lrapidcheck $(GTEST_LIBS) -lgmock From a512f4eebcbfed5db8e8c48fd93d99201267df04 Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Mon, 8 Apr 2024 13:13:02 +0200 Subject: [PATCH 8/9] test/libutil: Add OBSERVE_STRING macro Makes string callback easier to pass, without mistakes. --- tests/unit/libstore/nix_api_store.cc | 12 ++++------- .../libutil-support/tests/string_callback.cc | 9 +++++++++ .../libutil-support/tests/string_callback.hh | 12 +++++++++++ tests/unit/libutil/nix_api_util.cc | 20 ++++++++----------- 4 files changed, 33 insertions(+), 20 deletions(-) create mode 100644 tests/unit/libutil-support/tests/string_callback.cc create mode 100644 tests/unit/libutil-support/tests/string_callback.hh diff --git a/tests/unit/libstore/nix_api_store.cc b/tests/unit/libstore/nix_api_store.cc index a31d66a4c..7c6ec0780 100644 --- a/tests/unit/libstore/nix_api_store.cc +++ b/tests/unit/libstore/nix_api_store.cc @@ -4,14 +4,10 @@ #include "nix_api_store_internal.h" #include "tests/nix_api_store.hh" +#include "tests/string_callback.hh" namespace nixC { -void observe_string_cb(const char * start, unsigned int n, std::string * user_data) -{ - *user_data = std::string(start); -} - std::string PATH_SUFFIX = "/g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-name"; TEST_F(nix_api_util_context, nix_libstore_init) @@ -23,7 +19,7 @@ TEST_F(nix_api_util_context, nix_libstore_init) TEST_F(nix_api_store_test, nix_store_get_uri) { std::string str; - auto ret = nix_store_get_uri(ctx, store, (void *) observe_string_cb, &str); + auto ret = nix_store_get_uri(ctx, store, OBSERVE_STRING(str)); ASSERT_EQ(NIX_OK, ret); ASSERT_STREQ("local", str.c_str()); } @@ -56,7 +52,7 @@ TEST_F(nix_api_store_test, DoesNotCrashWhenContextIsNull) TEST_F(nix_api_store_test, get_version) { std::string str; - auto ret = nix_store_get_version(ctx, store, (void *) observe_string_cb, &str); + auto ret = nix_store_get_version(ctx, store, OBSERVE_STRING(str)); ASSERT_EQ(NIX_OK, ret); ASSERT_STREQ(PACKAGE_VERSION, str.c_str()); } @@ -69,7 +65,7 @@ TEST_F(nix_api_util_context, nix_store_open_dummy) ASSERT_STREQ("dummy", store->ptr->getUri().c_str()); std::string str; - nix_store_get_version(ctx, store, (void *) observe_string_cb, &str); + nix_store_get_version(ctx, store, OBSERVE_STRING(str)); ASSERT_STREQ("", str.c_str()); nix_store_free(store); diff --git a/tests/unit/libutil-support/tests/string_callback.cc b/tests/unit/libutil-support/tests/string_callback.cc new file mode 100644 index 000000000..28ac8b10c --- /dev/null +++ b/tests/unit/libutil-support/tests/string_callback.cc @@ -0,0 +1,9 @@ +#include "string_callback.hh" + +namespace nix::testing { + +void observe_string_cb(const char * start, unsigned int n, std::string * user_data) { + *user_data = std::string(start); +} + +} diff --git a/tests/unit/libutil-support/tests/string_callback.hh b/tests/unit/libutil-support/tests/string_callback.hh new file mode 100644 index 000000000..808fb707b --- /dev/null +++ b/tests/unit/libutil-support/tests/string_callback.hh @@ -0,0 +1,12 @@ +#pragma once +#include + +namespace nix::testing { + +void observe_string_cb(const char * start, unsigned int n, std::string * user_data); +inline void * observe_string_cb_data(std::string & out) { + return (void *) &out; +}; +#define OBSERVE_STRING(str) (void *)nix::testing::observe_string_cb, nix::testing::observe_string_cb_data(str) + +} diff --git a/tests/unit/libutil/nix_api_util.cc b/tests/unit/libutil/nix_api_util.cc index 09f3f3e05..d2999f55b 100644 --- a/tests/unit/libutil/nix_api_util.cc +++ b/tests/unit/libutil/nix_api_util.cc @@ -3,16 +3,12 @@ #include "nix_api_util.h" #include "nix_api_util_internal.h" #include "tests/nix_api_util.hh" +#include "tests/string_callback.hh" #include namespace nixC { -void observe_string_cb(const char * start, unsigned int n, std::string * user_data) -{ - *user_data = std::string(start); -} - TEST_F(nix_api_util_context, nix_context_error) { std::string err_msg_ref; @@ -62,10 +58,10 @@ TEST_F(nix_api_util_context, nix_setting_get) { ASSERT_EQ(ctx->last_err_code, NIX_OK); std::string setting_value; - nix_err result = nix_setting_get(ctx, "invalid-key", (void *) observe_string_cb, &setting_value); + nix_err result = nix_setting_get(ctx, "invalid-key", OBSERVE_STRING(setting_value)); ASSERT_EQ(result, NIX_ERR_KEY); - result = nix_setting_get(ctx, "setting-name", (void *) observe_string_cb, &setting_value); + result = nix_setting_get(ctx, "setting-name", OBSERVE_STRING(setting_value)); ASSERT_EQ(result, NIX_OK); ASSERT_STREQ("empty", setting_value.c_str()); } @@ -79,7 +75,7 @@ TEST_F(nix_api_util_context, nix_setting_set) ASSERT_EQ(result, NIX_OK); std::string setting_value; - result = nix_setting_get(ctx, "setting-name", (void *) observe_string_cb, &setting_value); + result = nix_setting_get(ctx, "setting-name", OBSERVE_STRING(setting_value)); ASSERT_EQ(result, NIX_OK); ASSERT_STREQ("new-value", setting_value.c_str()); } @@ -107,14 +103,14 @@ TEST_F(nix_api_util_context, nix_err_info_msg) std::string err_info; // no error - EXPECT_THROW(nix_err_info_msg(NULL, ctx, (void *) observe_string_cb, &err_info), nix::Error); + EXPECT_THROW(nix_err_info_msg(NULL, ctx, OBSERVE_STRING(err_info)), nix::Error); try { throw nix::Error("testing error"); } catch (...) { nix_context_error(ctx); } - nix_err_info_msg(nix_c_context_create(), ctx, (void *) observe_string_cb, &err_info); + nix_err_info_msg(nix_c_context_create(), ctx, OBSERVE_STRING(err_info)); ASSERT_STREQ("testing error", err_info.c_str()); } @@ -123,7 +119,7 @@ TEST_F(nix_api_util_context, nix_err_name) std::string err_name; // no error - EXPECT_THROW(nix_err_name(NULL, ctx, (void *) observe_string_cb, &err_name), nix::Error); + EXPECT_THROW(nix_err_name(NULL, ctx, OBSERVE_STRING(err_name)), nix::Error); std::string err_msg_ref; try { @@ -131,7 +127,7 @@ TEST_F(nix_api_util_context, nix_err_name) } catch (...) { nix_context_error(ctx); } - nix_err_name(nix_c_context_create(), ctx, (void *) observe_string_cb, &err_name); + nix_err_name(nix_c_context_create(), ctx, OBSERVE_STRING(err_name)); ASSERT_EQ(std::string(err_name), "nix::Error"); } From f2522d4ecdcd693a2f94cd118eb7fa509983a54e Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Mon, 8 Apr 2024 13:14:03 +0200 Subject: [PATCH 9/9] libexpr-c: Add nix_store_path_name --- src/libstore-c/nix_api_store.cc | 7 +++++++ src/libstore-c/nix_api_store.h | 9 +++++++++ tests/unit/libexpr/nix_api_expr.cc | 8 ++++++-- 3 files changed, 22 insertions(+), 2 deletions(-) diff --git a/src/libstore-c/nix_api_store.cc b/src/libstore-c/nix_api_store.cc index d044ba14f..511ba0fad 100644 --- a/src/libstore-c/nix_api_store.cc +++ b/src/libstore-c/nix_api_store.cc @@ -128,6 +128,13 @@ nix_err nix_store_realise( NIXC_CATCH_ERRS } +void nix_store_path_name(const StorePath *store_path, void * callback, void * user_data) +{ + std::string_view name = store_path->path.name(); + ((nix_get_string_callback) callback)(name.data(), name.size(), user_data); +} + + void nix_store_path_free(StorePath * sp) { delete sp; diff --git a/src/libstore-c/nix_api_store.h b/src/libstore-c/nix_api_store.h index ab86a81df..ca8996681 100644 --- a/src/libstore-c/nix_api_store.h +++ b/src/libstore-c/nix_api_store.h @@ -90,6 +90,15 @@ nix_err nix_store_get_uri(nix_c_context * context, Store * store, void * callbac */ StorePath * nix_store_parse_path(nix_c_context * context, Store * store, const char * path); +/** + * @brief Get the path name (e.g. "name" in /nix/store/...-name) + * + * @param[in] store_path the path to get the name from + * @param[in] callback called with the name + * @param[in] user_data arbitrary data, passed to the callback when it's called. + */ +void nix_store_path_name(const StorePath *store_path, void * callback, void * user_data); + /** * @brief Copy a StorePath * diff --git a/tests/unit/libexpr/nix_api_expr.cc b/tests/unit/libexpr/nix_api_expr.cc index fad078d0c..f5c66536d 100644 --- a/tests/unit/libexpr/nix_api_expr.cc +++ b/tests/unit/libexpr/nix_api_expr.cc @@ -6,6 +6,7 @@ #include "nix_api_value.h" #include "tests/nix_api_expr.hh" +#include "tests/string_callback.hh" #include "gmock/gmock.h" #include @@ -168,11 +169,14 @@ TEST_F(nix_api_expr_test, nix_expr_realise_context) EXPECT_THAT(s, testing::HasSubstr("a derivation path by itself:")); EXPECT_THAT(s, testing::EndsWith("-not-actually-built-yet.drv\n")); - std::vector names; + std::vector names; size_t n = nix_realised_string_get_store_path_count(r); for (size_t i = 0; i < n; ++i) { const StorePath * p = nix_realised_string_get_store_path(r, i); - names.push_back(p->path.name()); + ASSERT_NE(nullptr, p); + std::string name; + nix_store_path_name(p, OBSERVE_STRING(name)); + names.push_back(name); } std::sort(names.begin(), names.end()); ASSERT_EQ(3, names.size());