nix-super/src/libexpr/primops/context.cc

218 lines
8.3 KiB
C++
Raw Normal View History

#include "primops.hh"
#include "eval-inline.hh"
#include "derivations.hh"
#include "store-api.hh"
namespace nix {
static void prim_unsafeDiscardStringContext(EvalState & state, const PosIdx pos, Value * * args, Value & v)
{
Use `std::set<StringContextElem>` not `PathSet` for string contexts Motivation `PathSet` is not correct because string contexts have other forms (`Built` and `DrvDeep`) that are not rendered as plain store paths. Instead of wrongly using `PathSet`, or "stringly typed" using `StringSet`, use `std::std<StringContextElem>`. ----- In support of this change, `NixStringContext` is now defined as `std::std<StringContextElem>` not `std:vector<StringContextElem>`. The old definition was just used by a `getContext` method which was only used by the eval cache. It can be deleted altogether since the types are now unified and the preexisting `copyContext` function already suffices. Summarizing the previous paragraph: Old: - `value/context.hh`: `NixStringContext = std::vector<StringContextElem>` - `value.hh`: `NixStringContext Value::getContext(...)` - `value.hh`: `copyContext(...)` New: - `value/context.hh`: `NixStringContext = std::set<StringContextElem>` - `value.hh`: `copyContext(...)` ---- The string representation of string context elements no longer contains the store dir. The diff of `src/libexpr/tests/value/context.cc` should make clear what the new representation is, so we recommend reviewing that file first. This was done for two reasons: Less API churn: `Value::mkString` and friends did not take a `Store` before. But if `NixStringContextElem::{parse, to_string}` *do* take a store (as they did before), then we cannot have the `Value` functions use them (in order to work with the fully-structured `NixStringContext`) without adding that argument. That would have been a lot of churn of threading the store, and this diff is already large enough, so the easier and less invasive thing to do was simply make the element `parse` and `to_string` functions not take the `Store` reference, and the easiest way to do that was to simply drop the store dir. Space usage: Dropping the `/nix/store/` (or similar) from the internal representation will safe space in the heap of the Nix programming being interpreted. If the heap contains many strings with non-trivial contexts, the saving could add up to something significant. ---- The eval cache version is bumped. The eval cache serialization uses `NixStringContextElem::{parse, to_string}`, and since those functions are changed per the above, that means the on-disk representation is also changed. This is simply done by changing the name of the used for the eval cache from `eval-cache-v4` to eval-cache-v5`. ---- To avoid some duplication `EvalCache::mkPathString` is added to abstract over the simple case of turning a store path to a string with just that string in the context. Context This PR picks up where #7543 left off. That one introduced the fully structured `NixStringContextElem` data type, but kept `PathSet context` as an awkward middle ground between internal `char[][]` interpreter heap string contexts and `NixStringContext` fully parsed string contexts. The infelicity of `PathSet context` was specifically called out during Nix team group review, but it was agreeing that fixing it could be left as future work. This is that future work. A possible follow-up step would be to get rid of the `char[][]` evaluator heap representation, too, but it is not yet clear how to do that. To use `NixStringContextElem` there we would need to get the STL containers to GC pointers in the GC build, and I am not sure how to do that. ---- PR #7543 effectively is writing the inverse of a `mkPathString`, `mkOutputString`, and one more such function for the `DrvDeep` case. I would like that PR to have property tests ensuring it is actually the inverse as expected. This PR sets things up nicely so that reworking that PR to be in that more elegant and better tested way is possible. Co-authored-by: Théophane Hufschmitt <7226587+thufschmitt@users.noreply.github.com>
2023-01-29 03:31:10 +02:00
NixStringContext context;
auto s = state.coerceToString(pos, *args[0], context, "while evaluating the argument passed to builtins.unsafeDiscardStringContext");
v.mkString(*s);
}
static RegisterPrimOp primop_unsafeDiscardStringContext({
.name = "__unsafeDiscardStringContext",
.arity = 1,
.fun = prim_unsafeDiscardStringContext
});
static void prim_hasContext(EvalState & state, const PosIdx pos, Value * * args, Value & v)
{
Use `std::set<StringContextElem>` not `PathSet` for string contexts Motivation `PathSet` is not correct because string contexts have other forms (`Built` and `DrvDeep`) that are not rendered as plain store paths. Instead of wrongly using `PathSet`, or "stringly typed" using `StringSet`, use `std::std<StringContextElem>`. ----- In support of this change, `NixStringContext` is now defined as `std::std<StringContextElem>` not `std:vector<StringContextElem>`. The old definition was just used by a `getContext` method which was only used by the eval cache. It can be deleted altogether since the types are now unified and the preexisting `copyContext` function already suffices. Summarizing the previous paragraph: Old: - `value/context.hh`: `NixStringContext = std::vector<StringContextElem>` - `value.hh`: `NixStringContext Value::getContext(...)` - `value.hh`: `copyContext(...)` New: - `value/context.hh`: `NixStringContext = std::set<StringContextElem>` - `value.hh`: `copyContext(...)` ---- The string representation of string context elements no longer contains the store dir. The diff of `src/libexpr/tests/value/context.cc` should make clear what the new representation is, so we recommend reviewing that file first. This was done for two reasons: Less API churn: `Value::mkString` and friends did not take a `Store` before. But if `NixStringContextElem::{parse, to_string}` *do* take a store (as they did before), then we cannot have the `Value` functions use them (in order to work with the fully-structured `NixStringContext`) without adding that argument. That would have been a lot of churn of threading the store, and this diff is already large enough, so the easier and less invasive thing to do was simply make the element `parse` and `to_string` functions not take the `Store` reference, and the easiest way to do that was to simply drop the store dir. Space usage: Dropping the `/nix/store/` (or similar) from the internal representation will safe space in the heap of the Nix programming being interpreted. If the heap contains many strings with non-trivial contexts, the saving could add up to something significant. ---- The eval cache version is bumped. The eval cache serialization uses `NixStringContextElem::{parse, to_string}`, and since those functions are changed per the above, that means the on-disk representation is also changed. This is simply done by changing the name of the used for the eval cache from `eval-cache-v4` to eval-cache-v5`. ---- To avoid some duplication `EvalCache::mkPathString` is added to abstract over the simple case of turning a store path to a string with just that string in the context. Context This PR picks up where #7543 left off. That one introduced the fully structured `NixStringContextElem` data type, but kept `PathSet context` as an awkward middle ground between internal `char[][]` interpreter heap string contexts and `NixStringContext` fully parsed string contexts. The infelicity of `PathSet context` was specifically called out during Nix team group review, but it was agreeing that fixing it could be left as future work. This is that future work. A possible follow-up step would be to get rid of the `char[][]` evaluator heap representation, too, but it is not yet clear how to do that. To use `NixStringContextElem` there we would need to get the STL containers to GC pointers in the GC build, and I am not sure how to do that. ---- PR #7543 effectively is writing the inverse of a `mkPathString`, `mkOutputString`, and one more such function for the `DrvDeep` case. I would like that PR to have property tests ensuring it is actually the inverse as expected. This PR sets things up nicely so that reworking that PR to be in that more elegant and better tested way is possible. Co-authored-by: Théophane Hufschmitt <7226587+thufschmitt@users.noreply.github.com>
2023-01-29 03:31:10 +02:00
NixStringContext context;
state.forceString(*args[0], context, pos, "while evaluating the argument passed to builtins.hasContext");
2022-01-04 19:40:39 +02:00
v.mkBool(!context.empty());
}
static RegisterPrimOp primop_hasContext({
.name = "__hasContext",
.arity = 1,
.fun = prim_hasContext
});
/* Sometimes we want to pass a derivation path (i.e. pkg.drvPath) to a
builder without causing the derivation to be built (for instance,
in the derivation that builds NARs in nix-push, when doing
source-only deployment). This primop marks the string context so
that builtins.derivation adds the path to drv.inputSrcs rather than
drv.inputDrvs. */
static void prim_unsafeDiscardOutputDependency(EvalState & state, const PosIdx pos, Value * * args, Value & v)
{
Use `std::set<StringContextElem>` not `PathSet` for string contexts Motivation `PathSet` is not correct because string contexts have other forms (`Built` and `DrvDeep`) that are not rendered as plain store paths. Instead of wrongly using `PathSet`, or "stringly typed" using `StringSet`, use `std::std<StringContextElem>`. ----- In support of this change, `NixStringContext` is now defined as `std::std<StringContextElem>` not `std:vector<StringContextElem>`. The old definition was just used by a `getContext` method which was only used by the eval cache. It can be deleted altogether since the types are now unified and the preexisting `copyContext` function already suffices. Summarizing the previous paragraph: Old: - `value/context.hh`: `NixStringContext = std::vector<StringContextElem>` - `value.hh`: `NixStringContext Value::getContext(...)` - `value.hh`: `copyContext(...)` New: - `value/context.hh`: `NixStringContext = std::set<StringContextElem>` - `value.hh`: `copyContext(...)` ---- The string representation of string context elements no longer contains the store dir. The diff of `src/libexpr/tests/value/context.cc` should make clear what the new representation is, so we recommend reviewing that file first. This was done for two reasons: Less API churn: `Value::mkString` and friends did not take a `Store` before. But if `NixStringContextElem::{parse, to_string}` *do* take a store (as they did before), then we cannot have the `Value` functions use them (in order to work with the fully-structured `NixStringContext`) without adding that argument. That would have been a lot of churn of threading the store, and this diff is already large enough, so the easier and less invasive thing to do was simply make the element `parse` and `to_string` functions not take the `Store` reference, and the easiest way to do that was to simply drop the store dir. Space usage: Dropping the `/nix/store/` (or similar) from the internal representation will safe space in the heap of the Nix programming being interpreted. If the heap contains many strings with non-trivial contexts, the saving could add up to something significant. ---- The eval cache version is bumped. The eval cache serialization uses `NixStringContextElem::{parse, to_string}`, and since those functions are changed per the above, that means the on-disk representation is also changed. This is simply done by changing the name of the used for the eval cache from `eval-cache-v4` to eval-cache-v5`. ---- To avoid some duplication `EvalCache::mkPathString` is added to abstract over the simple case of turning a store path to a string with just that string in the context. Context This PR picks up where #7543 left off. That one introduced the fully structured `NixStringContextElem` data type, but kept `PathSet context` as an awkward middle ground between internal `char[][]` interpreter heap string contexts and `NixStringContext` fully parsed string contexts. The infelicity of `PathSet context` was specifically called out during Nix team group review, but it was agreeing that fixing it could be left as future work. This is that future work. A possible follow-up step would be to get rid of the `char[][]` evaluator heap representation, too, but it is not yet clear how to do that. To use `NixStringContextElem` there we would need to get the STL containers to GC pointers in the GC build, and I am not sure how to do that. ---- PR #7543 effectively is writing the inverse of a `mkPathString`, `mkOutputString`, and one more such function for the `DrvDeep` case. I would like that PR to have property tests ensuring it is actually the inverse as expected. This PR sets things up nicely so that reworking that PR to be in that more elegant and better tested way is possible. Co-authored-by: Théophane Hufschmitt <7226587+thufschmitt@users.noreply.github.com>
2023-01-29 03:31:10 +02:00
NixStringContext context;
auto s = state.coerceToString(pos, *args[0], context, "while evaluating the argument passed to builtins.unsafeDiscardOutputDependency");
Use `std::set<StringContextElem>` not `PathSet` for string contexts Motivation `PathSet` is not correct because string contexts have other forms (`Built` and `DrvDeep`) that are not rendered as plain store paths. Instead of wrongly using `PathSet`, or "stringly typed" using `StringSet`, use `std::std<StringContextElem>`. ----- In support of this change, `NixStringContext` is now defined as `std::std<StringContextElem>` not `std:vector<StringContextElem>`. The old definition was just used by a `getContext` method which was only used by the eval cache. It can be deleted altogether since the types are now unified and the preexisting `copyContext` function already suffices. Summarizing the previous paragraph: Old: - `value/context.hh`: `NixStringContext = std::vector<StringContextElem>` - `value.hh`: `NixStringContext Value::getContext(...)` - `value.hh`: `copyContext(...)` New: - `value/context.hh`: `NixStringContext = std::set<StringContextElem>` - `value.hh`: `copyContext(...)` ---- The string representation of string context elements no longer contains the store dir. The diff of `src/libexpr/tests/value/context.cc` should make clear what the new representation is, so we recommend reviewing that file first. This was done for two reasons: Less API churn: `Value::mkString` and friends did not take a `Store` before. But if `NixStringContextElem::{parse, to_string}` *do* take a store (as they did before), then we cannot have the `Value` functions use them (in order to work with the fully-structured `NixStringContext`) without adding that argument. That would have been a lot of churn of threading the store, and this diff is already large enough, so the easier and less invasive thing to do was simply make the element `parse` and `to_string` functions not take the `Store` reference, and the easiest way to do that was to simply drop the store dir. Space usage: Dropping the `/nix/store/` (or similar) from the internal representation will safe space in the heap of the Nix programming being interpreted. If the heap contains many strings with non-trivial contexts, the saving could add up to something significant. ---- The eval cache version is bumped. The eval cache serialization uses `NixStringContextElem::{parse, to_string}`, and since those functions are changed per the above, that means the on-disk representation is also changed. This is simply done by changing the name of the used for the eval cache from `eval-cache-v4` to eval-cache-v5`. ---- To avoid some duplication `EvalCache::mkPathString` is added to abstract over the simple case of turning a store path to a string with just that string in the context. Context This PR picks up where #7543 left off. That one introduced the fully structured `NixStringContextElem` data type, but kept `PathSet context` as an awkward middle ground between internal `char[][]` interpreter heap string contexts and `NixStringContext` fully parsed string contexts. The infelicity of `PathSet context` was specifically called out during Nix team group review, but it was agreeing that fixing it could be left as future work. This is that future work. A possible follow-up step would be to get rid of the `char[][]` evaluator heap representation, too, but it is not yet clear how to do that. To use `NixStringContextElem` there we would need to get the STL containers to GC pointers in the GC build, and I am not sure how to do that. ---- PR #7543 effectively is writing the inverse of a `mkPathString`, `mkOutputString`, and one more such function for the `DrvDeep` case. I would like that PR to have property tests ensuring it is actually the inverse as expected. This PR sets things up nicely so that reworking that PR to be in that more elegant and better tested way is possible. Co-authored-by: Théophane Hufschmitt <7226587+thufschmitt@users.noreply.github.com>
2023-01-29 03:31:10 +02:00
NixStringContext context2;
for (auto && c : context) {
if (auto * ptr = std::get_if<NixStringContextElem::DrvDeep>(&c)) {
Use `std::set<StringContextElem>` not `PathSet` for string contexts Motivation `PathSet` is not correct because string contexts have other forms (`Built` and `DrvDeep`) that are not rendered as plain store paths. Instead of wrongly using `PathSet`, or "stringly typed" using `StringSet`, use `std::std<StringContextElem>`. ----- In support of this change, `NixStringContext` is now defined as `std::std<StringContextElem>` not `std:vector<StringContextElem>`. The old definition was just used by a `getContext` method which was only used by the eval cache. It can be deleted altogether since the types are now unified and the preexisting `copyContext` function already suffices. Summarizing the previous paragraph: Old: - `value/context.hh`: `NixStringContext = std::vector<StringContextElem>` - `value.hh`: `NixStringContext Value::getContext(...)` - `value.hh`: `copyContext(...)` New: - `value/context.hh`: `NixStringContext = std::set<StringContextElem>` - `value.hh`: `copyContext(...)` ---- The string representation of string context elements no longer contains the store dir. The diff of `src/libexpr/tests/value/context.cc` should make clear what the new representation is, so we recommend reviewing that file first. This was done for two reasons: Less API churn: `Value::mkString` and friends did not take a `Store` before. But if `NixStringContextElem::{parse, to_string}` *do* take a store (as they did before), then we cannot have the `Value` functions use them (in order to work with the fully-structured `NixStringContext`) without adding that argument. That would have been a lot of churn of threading the store, and this diff is already large enough, so the easier and less invasive thing to do was simply make the element `parse` and `to_string` functions not take the `Store` reference, and the easiest way to do that was to simply drop the store dir. Space usage: Dropping the `/nix/store/` (or similar) from the internal representation will safe space in the heap of the Nix programming being interpreted. If the heap contains many strings with non-trivial contexts, the saving could add up to something significant. ---- The eval cache version is bumped. The eval cache serialization uses `NixStringContextElem::{parse, to_string}`, and since those functions are changed per the above, that means the on-disk representation is also changed. This is simply done by changing the name of the used for the eval cache from `eval-cache-v4` to eval-cache-v5`. ---- To avoid some duplication `EvalCache::mkPathString` is added to abstract over the simple case of turning a store path to a string with just that string in the context. Context This PR picks up where #7543 left off. That one introduced the fully structured `NixStringContextElem` data type, but kept `PathSet context` as an awkward middle ground between internal `char[][]` interpreter heap string contexts and `NixStringContext` fully parsed string contexts. The infelicity of `PathSet context` was specifically called out during Nix team group review, but it was agreeing that fixing it could be left as future work. This is that future work. A possible follow-up step would be to get rid of the `char[][]` evaluator heap representation, too, but it is not yet clear how to do that. To use `NixStringContextElem` there we would need to get the STL containers to GC pointers in the GC build, and I am not sure how to do that. ---- PR #7543 effectively is writing the inverse of a `mkPathString`, `mkOutputString`, and one more such function for the `DrvDeep` case. I would like that PR to have property tests ensuring it is actually the inverse as expected. This PR sets things up nicely so that reworking that PR to be in that more elegant and better tested way is possible. Co-authored-by: Théophane Hufschmitt <7226587+thufschmitt@users.noreply.github.com>
2023-01-29 03:31:10 +02:00
context2.emplace(NixStringContextElem::Opaque {
.path = ptr->drvPath
});
} else {
/* Can reuse original item */
Use `std::set<StringContextElem>` not `PathSet` for string contexts Motivation `PathSet` is not correct because string contexts have other forms (`Built` and `DrvDeep`) that are not rendered as plain store paths. Instead of wrongly using `PathSet`, or "stringly typed" using `StringSet`, use `std::std<StringContextElem>`. ----- In support of this change, `NixStringContext` is now defined as `std::std<StringContextElem>` not `std:vector<StringContextElem>`. The old definition was just used by a `getContext` method which was only used by the eval cache. It can be deleted altogether since the types are now unified and the preexisting `copyContext` function already suffices. Summarizing the previous paragraph: Old: - `value/context.hh`: `NixStringContext = std::vector<StringContextElem>` - `value.hh`: `NixStringContext Value::getContext(...)` - `value.hh`: `copyContext(...)` New: - `value/context.hh`: `NixStringContext = std::set<StringContextElem>` - `value.hh`: `copyContext(...)` ---- The string representation of string context elements no longer contains the store dir. The diff of `src/libexpr/tests/value/context.cc` should make clear what the new representation is, so we recommend reviewing that file first. This was done for two reasons: Less API churn: `Value::mkString` and friends did not take a `Store` before. But if `NixStringContextElem::{parse, to_string}` *do* take a store (as they did before), then we cannot have the `Value` functions use them (in order to work with the fully-structured `NixStringContext`) without adding that argument. That would have been a lot of churn of threading the store, and this diff is already large enough, so the easier and less invasive thing to do was simply make the element `parse` and `to_string` functions not take the `Store` reference, and the easiest way to do that was to simply drop the store dir. Space usage: Dropping the `/nix/store/` (or similar) from the internal representation will safe space in the heap of the Nix programming being interpreted. If the heap contains many strings with non-trivial contexts, the saving could add up to something significant. ---- The eval cache version is bumped. The eval cache serialization uses `NixStringContextElem::{parse, to_string}`, and since those functions are changed per the above, that means the on-disk representation is also changed. This is simply done by changing the name of the used for the eval cache from `eval-cache-v4` to eval-cache-v5`. ---- To avoid some duplication `EvalCache::mkPathString` is added to abstract over the simple case of turning a store path to a string with just that string in the context. Context This PR picks up where #7543 left off. That one introduced the fully structured `NixStringContextElem` data type, but kept `PathSet context` as an awkward middle ground between internal `char[][]` interpreter heap string contexts and `NixStringContext` fully parsed string contexts. The infelicity of `PathSet context` was specifically called out during Nix team group review, but it was agreeing that fixing it could be left as future work. This is that future work. A possible follow-up step would be to get rid of the `char[][]` evaluator heap representation, too, but it is not yet clear how to do that. To use `NixStringContextElem` there we would need to get the STL containers to GC pointers in the GC build, and I am not sure how to do that. ---- PR #7543 effectively is writing the inverse of a `mkPathString`, `mkOutputString`, and one more such function for the `DrvDeep` case. I would like that PR to have property tests ensuring it is actually the inverse as expected. This PR sets things up nicely so that reworking that PR to be in that more elegant and better tested way is possible. Co-authored-by: Théophane Hufschmitt <7226587+thufschmitt@users.noreply.github.com>
2023-01-29 03:31:10 +02:00
context2.emplace(std::move(c));
}
}
v.mkString(*s, context2);
}
static RegisterPrimOp primop_unsafeDiscardOutputDependency({
.name = "__unsafeDiscardOutputDependency",
.arity = 1,
.fun = prim_unsafeDiscardOutputDependency
});
/* Extract the context of a string as a structured Nix value.
The context is represented as an attribute set whose keys are the
paths in the context set and whose values are attribute sets with
the following keys:
path: True if the relevant path is in the context as a plain store
path (i.e. the kind of context you get when interpolating
a Nix path (e.g. ./.) into a string). False if missing.
allOutputs: True if the relevant path is a derivation and it is
in the context as a drv file with all of its outputs
(i.e. the kind of context you get when referencing
.drvPath of some derivation). False if missing.
outputs: If a non-empty list, the relevant path is a derivation
and the provided outputs are referenced in the context
(i.e. the kind of context you get when referencing
.outPath of some derivation). Empty list if missing.
Note that for a given path any combination of the above attributes
may be present.
*/
static void prim_getContext(EvalState & state, const PosIdx pos, Value * * args, Value & v)
{
struct ContextInfo {
bool path = false;
bool allOutputs = false;
Strings outputs;
};
Use `std::set<StringContextElem>` not `PathSet` for string contexts Motivation `PathSet` is not correct because string contexts have other forms (`Built` and `DrvDeep`) that are not rendered as plain store paths. Instead of wrongly using `PathSet`, or "stringly typed" using `StringSet`, use `std::std<StringContextElem>`. ----- In support of this change, `NixStringContext` is now defined as `std::std<StringContextElem>` not `std:vector<StringContextElem>`. The old definition was just used by a `getContext` method which was only used by the eval cache. It can be deleted altogether since the types are now unified and the preexisting `copyContext` function already suffices. Summarizing the previous paragraph: Old: - `value/context.hh`: `NixStringContext = std::vector<StringContextElem>` - `value.hh`: `NixStringContext Value::getContext(...)` - `value.hh`: `copyContext(...)` New: - `value/context.hh`: `NixStringContext = std::set<StringContextElem>` - `value.hh`: `copyContext(...)` ---- The string representation of string context elements no longer contains the store dir. The diff of `src/libexpr/tests/value/context.cc` should make clear what the new representation is, so we recommend reviewing that file first. This was done for two reasons: Less API churn: `Value::mkString` and friends did not take a `Store` before. But if `NixStringContextElem::{parse, to_string}` *do* take a store (as they did before), then we cannot have the `Value` functions use them (in order to work with the fully-structured `NixStringContext`) without adding that argument. That would have been a lot of churn of threading the store, and this diff is already large enough, so the easier and less invasive thing to do was simply make the element `parse` and `to_string` functions not take the `Store` reference, and the easiest way to do that was to simply drop the store dir. Space usage: Dropping the `/nix/store/` (or similar) from the internal representation will safe space in the heap of the Nix programming being interpreted. If the heap contains many strings with non-trivial contexts, the saving could add up to something significant. ---- The eval cache version is bumped. The eval cache serialization uses `NixStringContextElem::{parse, to_string}`, and since those functions are changed per the above, that means the on-disk representation is also changed. This is simply done by changing the name of the used for the eval cache from `eval-cache-v4` to eval-cache-v5`. ---- To avoid some duplication `EvalCache::mkPathString` is added to abstract over the simple case of turning a store path to a string with just that string in the context. Context This PR picks up where #7543 left off. That one introduced the fully structured `NixStringContextElem` data type, but kept `PathSet context` as an awkward middle ground between internal `char[][]` interpreter heap string contexts and `NixStringContext` fully parsed string contexts. The infelicity of `PathSet context` was specifically called out during Nix team group review, but it was agreeing that fixing it could be left as future work. This is that future work. A possible follow-up step would be to get rid of the `char[][]` evaluator heap representation, too, but it is not yet clear how to do that. To use `NixStringContextElem` there we would need to get the STL containers to GC pointers in the GC build, and I am not sure how to do that. ---- PR #7543 effectively is writing the inverse of a `mkPathString`, `mkOutputString`, and one more such function for the `DrvDeep` case. I would like that PR to have property tests ensuring it is actually the inverse as expected. This PR sets things up nicely so that reworking that PR to be in that more elegant and better tested way is possible. Co-authored-by: Théophane Hufschmitt <7226587+thufschmitt@users.noreply.github.com>
2023-01-29 03:31:10 +02:00
NixStringContext context;
state.forceString(*args[0], context, pos, "while evaluating the argument passed to builtins.getContext");
auto contextInfos = std::map<StorePath, ContextInfo>();
Use `std::set<StringContextElem>` not `PathSet` for string contexts Motivation `PathSet` is not correct because string contexts have other forms (`Built` and `DrvDeep`) that are not rendered as plain store paths. Instead of wrongly using `PathSet`, or "stringly typed" using `StringSet`, use `std::std<StringContextElem>`. ----- In support of this change, `NixStringContext` is now defined as `std::std<StringContextElem>` not `std:vector<StringContextElem>`. The old definition was just used by a `getContext` method which was only used by the eval cache. It can be deleted altogether since the types are now unified and the preexisting `copyContext` function already suffices. Summarizing the previous paragraph: Old: - `value/context.hh`: `NixStringContext = std::vector<StringContextElem>` - `value.hh`: `NixStringContext Value::getContext(...)` - `value.hh`: `copyContext(...)` New: - `value/context.hh`: `NixStringContext = std::set<StringContextElem>` - `value.hh`: `copyContext(...)` ---- The string representation of string context elements no longer contains the store dir. The diff of `src/libexpr/tests/value/context.cc` should make clear what the new representation is, so we recommend reviewing that file first. This was done for two reasons: Less API churn: `Value::mkString` and friends did not take a `Store` before. But if `NixStringContextElem::{parse, to_string}` *do* take a store (as they did before), then we cannot have the `Value` functions use them (in order to work with the fully-structured `NixStringContext`) without adding that argument. That would have been a lot of churn of threading the store, and this diff is already large enough, so the easier and less invasive thing to do was simply make the element `parse` and `to_string` functions not take the `Store` reference, and the easiest way to do that was to simply drop the store dir. Space usage: Dropping the `/nix/store/` (or similar) from the internal representation will safe space in the heap of the Nix programming being interpreted. If the heap contains many strings with non-trivial contexts, the saving could add up to something significant. ---- The eval cache version is bumped. The eval cache serialization uses `NixStringContextElem::{parse, to_string}`, and since those functions are changed per the above, that means the on-disk representation is also changed. This is simply done by changing the name of the used for the eval cache from `eval-cache-v4` to eval-cache-v5`. ---- To avoid some duplication `EvalCache::mkPathString` is added to abstract over the simple case of turning a store path to a string with just that string in the context. Context This PR picks up where #7543 left off. That one introduced the fully structured `NixStringContextElem` data type, but kept `PathSet context` as an awkward middle ground between internal `char[][]` interpreter heap string contexts and `NixStringContext` fully parsed string contexts. The infelicity of `PathSet context` was specifically called out during Nix team group review, but it was agreeing that fixing it could be left as future work. This is that future work. A possible follow-up step would be to get rid of the `char[][]` evaluator heap representation, too, but it is not yet clear how to do that. To use `NixStringContextElem` there we would need to get the STL containers to GC pointers in the GC build, and I am not sure how to do that. ---- PR #7543 effectively is writing the inverse of a `mkPathString`, `mkOutputString`, and one more such function for the `DrvDeep` case. I would like that PR to have property tests ensuring it is actually the inverse as expected. This PR sets things up nicely so that reworking that PR to be in that more elegant and better tested way is possible. Co-authored-by: Théophane Hufschmitt <7226587+thufschmitt@users.noreply.github.com>
2023-01-29 03:31:10 +02:00
for (auto && i : context) {
std::visit(overloaded {
Use `std::set<StringContextElem>` not `PathSet` for string contexts Motivation `PathSet` is not correct because string contexts have other forms (`Built` and `DrvDeep`) that are not rendered as plain store paths. Instead of wrongly using `PathSet`, or "stringly typed" using `StringSet`, use `std::std<StringContextElem>`. ----- In support of this change, `NixStringContext` is now defined as `std::std<StringContextElem>` not `std:vector<StringContextElem>`. The old definition was just used by a `getContext` method which was only used by the eval cache. It can be deleted altogether since the types are now unified and the preexisting `copyContext` function already suffices. Summarizing the previous paragraph: Old: - `value/context.hh`: `NixStringContext = std::vector<StringContextElem>` - `value.hh`: `NixStringContext Value::getContext(...)` - `value.hh`: `copyContext(...)` New: - `value/context.hh`: `NixStringContext = std::set<StringContextElem>` - `value.hh`: `copyContext(...)` ---- The string representation of string context elements no longer contains the store dir. The diff of `src/libexpr/tests/value/context.cc` should make clear what the new representation is, so we recommend reviewing that file first. This was done for two reasons: Less API churn: `Value::mkString` and friends did not take a `Store` before. But if `NixStringContextElem::{parse, to_string}` *do* take a store (as they did before), then we cannot have the `Value` functions use them (in order to work with the fully-structured `NixStringContext`) without adding that argument. That would have been a lot of churn of threading the store, and this diff is already large enough, so the easier and less invasive thing to do was simply make the element `parse` and `to_string` functions not take the `Store` reference, and the easiest way to do that was to simply drop the store dir. Space usage: Dropping the `/nix/store/` (or similar) from the internal representation will safe space in the heap of the Nix programming being interpreted. If the heap contains many strings with non-trivial contexts, the saving could add up to something significant. ---- The eval cache version is bumped. The eval cache serialization uses `NixStringContextElem::{parse, to_string}`, and since those functions are changed per the above, that means the on-disk representation is also changed. This is simply done by changing the name of the used for the eval cache from `eval-cache-v4` to eval-cache-v5`. ---- To avoid some duplication `EvalCache::mkPathString` is added to abstract over the simple case of turning a store path to a string with just that string in the context. Context This PR picks up where #7543 left off. That one introduced the fully structured `NixStringContextElem` data type, but kept `PathSet context` as an awkward middle ground between internal `char[][]` interpreter heap string contexts and `NixStringContext` fully parsed string contexts. The infelicity of `PathSet context` was specifically called out during Nix team group review, but it was agreeing that fixing it could be left as future work. This is that future work. A possible follow-up step would be to get rid of the `char[][]` evaluator heap representation, too, but it is not yet clear how to do that. To use `NixStringContextElem` there we would need to get the STL containers to GC pointers in the GC build, and I am not sure how to do that. ---- PR #7543 effectively is writing the inverse of a `mkPathString`, `mkOutputString`, and one more such function for the `DrvDeep` case. I would like that PR to have property tests ensuring it is actually the inverse as expected. This PR sets things up nicely so that reworking that PR to be in that more elegant and better tested way is possible. Co-authored-by: Théophane Hufschmitt <7226587+thufschmitt@users.noreply.github.com>
2023-01-29 03:31:10 +02:00
[&](NixStringContextElem::DrvDeep && d) {
contextInfos[std::move(d.drvPath)].allOutputs = true;
},
Use `std::set<StringContextElem>` not `PathSet` for string contexts Motivation `PathSet` is not correct because string contexts have other forms (`Built` and `DrvDeep`) that are not rendered as plain store paths. Instead of wrongly using `PathSet`, or "stringly typed" using `StringSet`, use `std::std<StringContextElem>`. ----- In support of this change, `NixStringContext` is now defined as `std::std<StringContextElem>` not `std:vector<StringContextElem>`. The old definition was just used by a `getContext` method which was only used by the eval cache. It can be deleted altogether since the types are now unified and the preexisting `copyContext` function already suffices. Summarizing the previous paragraph: Old: - `value/context.hh`: `NixStringContext = std::vector<StringContextElem>` - `value.hh`: `NixStringContext Value::getContext(...)` - `value.hh`: `copyContext(...)` New: - `value/context.hh`: `NixStringContext = std::set<StringContextElem>` - `value.hh`: `copyContext(...)` ---- The string representation of string context elements no longer contains the store dir. The diff of `src/libexpr/tests/value/context.cc` should make clear what the new representation is, so we recommend reviewing that file first. This was done for two reasons: Less API churn: `Value::mkString` and friends did not take a `Store` before. But if `NixStringContextElem::{parse, to_string}` *do* take a store (as they did before), then we cannot have the `Value` functions use them (in order to work with the fully-structured `NixStringContext`) without adding that argument. That would have been a lot of churn of threading the store, and this diff is already large enough, so the easier and less invasive thing to do was simply make the element `parse` and `to_string` functions not take the `Store` reference, and the easiest way to do that was to simply drop the store dir. Space usage: Dropping the `/nix/store/` (or similar) from the internal representation will safe space in the heap of the Nix programming being interpreted. If the heap contains many strings with non-trivial contexts, the saving could add up to something significant. ---- The eval cache version is bumped. The eval cache serialization uses `NixStringContextElem::{parse, to_string}`, and since those functions are changed per the above, that means the on-disk representation is also changed. This is simply done by changing the name of the used for the eval cache from `eval-cache-v4` to eval-cache-v5`. ---- To avoid some duplication `EvalCache::mkPathString` is added to abstract over the simple case of turning a store path to a string with just that string in the context. Context This PR picks up where #7543 left off. That one introduced the fully structured `NixStringContextElem` data type, but kept `PathSet context` as an awkward middle ground between internal `char[][]` interpreter heap string contexts and `NixStringContext` fully parsed string contexts. The infelicity of `PathSet context` was specifically called out during Nix team group review, but it was agreeing that fixing it could be left as future work. This is that future work. A possible follow-up step would be to get rid of the `char[][]` evaluator heap representation, too, but it is not yet clear how to do that. To use `NixStringContextElem` there we would need to get the STL containers to GC pointers in the GC build, and I am not sure how to do that. ---- PR #7543 effectively is writing the inverse of a `mkPathString`, `mkOutputString`, and one more such function for the `DrvDeep` case. I would like that PR to have property tests ensuring it is actually the inverse as expected. This PR sets things up nicely so that reworking that PR to be in that more elegant and better tested way is possible. Co-authored-by: Théophane Hufschmitt <7226587+thufschmitt@users.noreply.github.com>
2023-01-29 03:31:10 +02:00
[&](NixStringContextElem::Built && b) {
contextInfos[std::move(b.drvPath)].outputs.emplace_back(std::move(b.output));
},
Use `std::set<StringContextElem>` not `PathSet` for string contexts Motivation `PathSet` is not correct because string contexts have other forms (`Built` and `DrvDeep`) that are not rendered as plain store paths. Instead of wrongly using `PathSet`, or "stringly typed" using `StringSet`, use `std::std<StringContextElem>`. ----- In support of this change, `NixStringContext` is now defined as `std::std<StringContextElem>` not `std:vector<StringContextElem>`. The old definition was just used by a `getContext` method which was only used by the eval cache. It can be deleted altogether since the types are now unified and the preexisting `copyContext` function already suffices. Summarizing the previous paragraph: Old: - `value/context.hh`: `NixStringContext = std::vector<StringContextElem>` - `value.hh`: `NixStringContext Value::getContext(...)` - `value.hh`: `copyContext(...)` New: - `value/context.hh`: `NixStringContext = std::set<StringContextElem>` - `value.hh`: `copyContext(...)` ---- The string representation of string context elements no longer contains the store dir. The diff of `src/libexpr/tests/value/context.cc` should make clear what the new representation is, so we recommend reviewing that file first. This was done for two reasons: Less API churn: `Value::mkString` and friends did not take a `Store` before. But if `NixStringContextElem::{parse, to_string}` *do* take a store (as they did before), then we cannot have the `Value` functions use them (in order to work with the fully-structured `NixStringContext`) without adding that argument. That would have been a lot of churn of threading the store, and this diff is already large enough, so the easier and less invasive thing to do was simply make the element `parse` and `to_string` functions not take the `Store` reference, and the easiest way to do that was to simply drop the store dir. Space usage: Dropping the `/nix/store/` (or similar) from the internal representation will safe space in the heap of the Nix programming being interpreted. If the heap contains many strings with non-trivial contexts, the saving could add up to something significant. ---- The eval cache version is bumped. The eval cache serialization uses `NixStringContextElem::{parse, to_string}`, and since those functions are changed per the above, that means the on-disk representation is also changed. This is simply done by changing the name of the used for the eval cache from `eval-cache-v4` to eval-cache-v5`. ---- To avoid some duplication `EvalCache::mkPathString` is added to abstract over the simple case of turning a store path to a string with just that string in the context. Context This PR picks up where #7543 left off. That one introduced the fully structured `NixStringContextElem` data type, but kept `PathSet context` as an awkward middle ground between internal `char[][]` interpreter heap string contexts and `NixStringContext` fully parsed string contexts. The infelicity of `PathSet context` was specifically called out during Nix team group review, but it was agreeing that fixing it could be left as future work. This is that future work. A possible follow-up step would be to get rid of the `char[][]` evaluator heap representation, too, but it is not yet clear how to do that. To use `NixStringContextElem` there we would need to get the STL containers to GC pointers in the GC build, and I am not sure how to do that. ---- PR #7543 effectively is writing the inverse of a `mkPathString`, `mkOutputString`, and one more such function for the `DrvDeep` case. I would like that PR to have property tests ensuring it is actually the inverse as expected. This PR sets things up nicely so that reworking that PR to be in that more elegant and better tested way is possible. Co-authored-by: Théophane Hufschmitt <7226587+thufschmitt@users.noreply.github.com>
2023-01-29 03:31:10 +02:00
[&](NixStringContextElem::Opaque && o) {
contextInfos[std::move(o.path)].path = true;
},
Use `std::set<StringContextElem>` not `PathSet` for string contexts Motivation `PathSet` is not correct because string contexts have other forms (`Built` and `DrvDeep`) that are not rendered as plain store paths. Instead of wrongly using `PathSet`, or "stringly typed" using `StringSet`, use `std::std<StringContextElem>`. ----- In support of this change, `NixStringContext` is now defined as `std::std<StringContextElem>` not `std:vector<StringContextElem>`. The old definition was just used by a `getContext` method which was only used by the eval cache. It can be deleted altogether since the types are now unified and the preexisting `copyContext` function already suffices. Summarizing the previous paragraph: Old: - `value/context.hh`: `NixStringContext = std::vector<StringContextElem>` - `value.hh`: `NixStringContext Value::getContext(...)` - `value.hh`: `copyContext(...)` New: - `value/context.hh`: `NixStringContext = std::set<StringContextElem>` - `value.hh`: `copyContext(...)` ---- The string representation of string context elements no longer contains the store dir. The diff of `src/libexpr/tests/value/context.cc` should make clear what the new representation is, so we recommend reviewing that file first. This was done for two reasons: Less API churn: `Value::mkString` and friends did not take a `Store` before. But if `NixStringContextElem::{parse, to_string}` *do* take a store (as they did before), then we cannot have the `Value` functions use them (in order to work with the fully-structured `NixStringContext`) without adding that argument. That would have been a lot of churn of threading the store, and this diff is already large enough, so the easier and less invasive thing to do was simply make the element `parse` and `to_string` functions not take the `Store` reference, and the easiest way to do that was to simply drop the store dir. Space usage: Dropping the `/nix/store/` (or similar) from the internal representation will safe space in the heap of the Nix programming being interpreted. If the heap contains many strings with non-trivial contexts, the saving could add up to something significant. ---- The eval cache version is bumped. The eval cache serialization uses `NixStringContextElem::{parse, to_string}`, and since those functions are changed per the above, that means the on-disk representation is also changed. This is simply done by changing the name of the used for the eval cache from `eval-cache-v4` to eval-cache-v5`. ---- To avoid some duplication `EvalCache::mkPathString` is added to abstract over the simple case of turning a store path to a string with just that string in the context. Context This PR picks up where #7543 left off. That one introduced the fully structured `NixStringContextElem` data type, but kept `PathSet context` as an awkward middle ground between internal `char[][]` interpreter heap string contexts and `NixStringContext` fully parsed string contexts. The infelicity of `PathSet context` was specifically called out during Nix team group review, but it was agreeing that fixing it could be left as future work. This is that future work. A possible follow-up step would be to get rid of the `char[][]` evaluator heap representation, too, but it is not yet clear how to do that. To use `NixStringContextElem` there we would need to get the STL containers to GC pointers in the GC build, and I am not sure how to do that. ---- PR #7543 effectively is writing the inverse of a `mkPathString`, `mkOutputString`, and one more such function for the `DrvDeep` case. I would like that PR to have property tests ensuring it is actually the inverse as expected. This PR sets things up nicely so that reworking that PR to be in that more elegant and better tested way is possible. Co-authored-by: Théophane Hufschmitt <7226587+thufschmitt@users.noreply.github.com>
2023-01-29 03:31:10 +02:00
}, ((NixStringContextElem &&) i).raw());
}
auto attrs = state.buildBindings(contextInfos.size());
auto sPath = state.symbols.create("path");
auto sAllOutputs = state.symbols.create("allOutputs");
for (const auto & info : contextInfos) {
auto infoAttrs = state.buildBindings(3);
if (info.second.path)
2022-01-04 19:40:39 +02:00
infoAttrs.alloc(sPath).mkBool(true);
if (info.second.allOutputs)
2022-01-04 19:40:39 +02:00
infoAttrs.alloc(sAllOutputs).mkBool(true);
if (!info.second.outputs.empty()) {
auto & outputsVal = infoAttrs.alloc(state.sOutputs);
state.mkList(outputsVal, info.second.outputs.size());
for (const auto & [i, output] : enumerate(info.second.outputs))
(outputsVal.listElems()[i] = state.allocValue())->mkString(output);
}
attrs.alloc(state.store->printStorePath(info.first)).mkAttrs(infoAttrs);
}
v.mkAttrs(attrs);
}
static RegisterPrimOp primop_getContext({
.name = "__getContext",
.arity = 1,
.fun = prim_getContext
});
/* Append the given context to a given string.
See the commentary above unsafeGetContext for details of the
context representation.
*/
static void prim_appendContext(EvalState & state, const PosIdx pos, Value * * args, Value & v)
{
Use `std::set<StringContextElem>` not `PathSet` for string contexts Motivation `PathSet` is not correct because string contexts have other forms (`Built` and `DrvDeep`) that are not rendered as plain store paths. Instead of wrongly using `PathSet`, or "stringly typed" using `StringSet`, use `std::std<StringContextElem>`. ----- In support of this change, `NixStringContext` is now defined as `std::std<StringContextElem>` not `std:vector<StringContextElem>`. The old definition was just used by a `getContext` method which was only used by the eval cache. It can be deleted altogether since the types are now unified and the preexisting `copyContext` function already suffices. Summarizing the previous paragraph: Old: - `value/context.hh`: `NixStringContext = std::vector<StringContextElem>` - `value.hh`: `NixStringContext Value::getContext(...)` - `value.hh`: `copyContext(...)` New: - `value/context.hh`: `NixStringContext = std::set<StringContextElem>` - `value.hh`: `copyContext(...)` ---- The string representation of string context elements no longer contains the store dir. The diff of `src/libexpr/tests/value/context.cc` should make clear what the new representation is, so we recommend reviewing that file first. This was done for two reasons: Less API churn: `Value::mkString` and friends did not take a `Store` before. But if `NixStringContextElem::{parse, to_string}` *do* take a store (as they did before), then we cannot have the `Value` functions use them (in order to work with the fully-structured `NixStringContext`) without adding that argument. That would have been a lot of churn of threading the store, and this diff is already large enough, so the easier and less invasive thing to do was simply make the element `parse` and `to_string` functions not take the `Store` reference, and the easiest way to do that was to simply drop the store dir. Space usage: Dropping the `/nix/store/` (or similar) from the internal representation will safe space in the heap of the Nix programming being interpreted. If the heap contains many strings with non-trivial contexts, the saving could add up to something significant. ---- The eval cache version is bumped. The eval cache serialization uses `NixStringContextElem::{parse, to_string}`, and since those functions are changed per the above, that means the on-disk representation is also changed. This is simply done by changing the name of the used for the eval cache from `eval-cache-v4` to eval-cache-v5`. ---- To avoid some duplication `EvalCache::mkPathString` is added to abstract over the simple case of turning a store path to a string with just that string in the context. Context This PR picks up where #7543 left off. That one introduced the fully structured `NixStringContextElem` data type, but kept `PathSet context` as an awkward middle ground between internal `char[][]` interpreter heap string contexts and `NixStringContext` fully parsed string contexts. The infelicity of `PathSet context` was specifically called out during Nix team group review, but it was agreeing that fixing it could be left as future work. This is that future work. A possible follow-up step would be to get rid of the `char[][]` evaluator heap representation, too, but it is not yet clear how to do that. To use `NixStringContextElem` there we would need to get the STL containers to GC pointers in the GC build, and I am not sure how to do that. ---- PR #7543 effectively is writing the inverse of a `mkPathString`, `mkOutputString`, and one more such function for the `DrvDeep` case. I would like that PR to have property tests ensuring it is actually the inverse as expected. This PR sets things up nicely so that reworking that PR to be in that more elegant and better tested way is possible. Co-authored-by: Théophane Hufschmitt <7226587+thufschmitt@users.noreply.github.com>
2023-01-29 03:31:10 +02:00
NixStringContext context;
auto orig = state.forceString(*args[0], context, noPos, "while evaluating the first argument passed to builtins.appendContext");
state.forceAttrs(*args[1], pos, "while evaluating the second argument passed to builtins.appendContext");
auto sPath = state.symbols.create("path");
auto sAllOutputs = state.symbols.create("allOutputs");
for (auto & i : *args[1]->attrs) {
const auto & name = state.symbols[i.name];
if (!state.store->isStorePath(name))
throw EvalError({
.msg = hintfmt("context key '%s' is not a store path", name),
.errPos = state.positions[i.pos]
});
Use `std::set<StringContextElem>` not `PathSet` for string contexts Motivation `PathSet` is not correct because string contexts have other forms (`Built` and `DrvDeep`) that are not rendered as plain store paths. Instead of wrongly using `PathSet`, or "stringly typed" using `StringSet`, use `std::std<StringContextElem>`. ----- In support of this change, `NixStringContext` is now defined as `std::std<StringContextElem>` not `std:vector<StringContextElem>`. The old definition was just used by a `getContext` method which was only used by the eval cache. It can be deleted altogether since the types are now unified and the preexisting `copyContext` function already suffices. Summarizing the previous paragraph: Old: - `value/context.hh`: `NixStringContext = std::vector<StringContextElem>` - `value.hh`: `NixStringContext Value::getContext(...)` - `value.hh`: `copyContext(...)` New: - `value/context.hh`: `NixStringContext = std::set<StringContextElem>` - `value.hh`: `copyContext(...)` ---- The string representation of string context elements no longer contains the store dir. The diff of `src/libexpr/tests/value/context.cc` should make clear what the new representation is, so we recommend reviewing that file first. This was done for two reasons: Less API churn: `Value::mkString` and friends did not take a `Store` before. But if `NixStringContextElem::{parse, to_string}` *do* take a store (as they did before), then we cannot have the `Value` functions use them (in order to work with the fully-structured `NixStringContext`) without adding that argument. That would have been a lot of churn of threading the store, and this diff is already large enough, so the easier and less invasive thing to do was simply make the element `parse` and `to_string` functions not take the `Store` reference, and the easiest way to do that was to simply drop the store dir. Space usage: Dropping the `/nix/store/` (or similar) from the internal representation will safe space in the heap of the Nix programming being interpreted. If the heap contains many strings with non-trivial contexts, the saving could add up to something significant. ---- The eval cache version is bumped. The eval cache serialization uses `NixStringContextElem::{parse, to_string}`, and since those functions are changed per the above, that means the on-disk representation is also changed. This is simply done by changing the name of the used for the eval cache from `eval-cache-v4` to eval-cache-v5`. ---- To avoid some duplication `EvalCache::mkPathString` is added to abstract over the simple case of turning a store path to a string with just that string in the context. Context This PR picks up where #7543 left off. That one introduced the fully structured `NixStringContextElem` data type, but kept `PathSet context` as an awkward middle ground between internal `char[][]` interpreter heap string contexts and `NixStringContext` fully parsed string contexts. The infelicity of `PathSet context` was specifically called out during Nix team group review, but it was agreeing that fixing it could be left as future work. This is that future work. A possible follow-up step would be to get rid of the `char[][]` evaluator heap representation, too, but it is not yet clear how to do that. To use `NixStringContextElem` there we would need to get the STL containers to GC pointers in the GC build, and I am not sure how to do that. ---- PR #7543 effectively is writing the inverse of a `mkPathString`, `mkOutputString`, and one more such function for the `DrvDeep` case. I would like that PR to have property tests ensuring it is actually the inverse as expected. This PR sets things up nicely so that reworking that PR to be in that more elegant and better tested way is possible. Co-authored-by: Théophane Hufschmitt <7226587+thufschmitt@users.noreply.github.com>
2023-01-29 03:31:10 +02:00
auto namePath = state.store->parseStorePath(name);
if (!settings.readOnlyMode)
Use `std::set<StringContextElem>` not `PathSet` for string contexts Motivation `PathSet` is not correct because string contexts have other forms (`Built` and `DrvDeep`) that are not rendered as plain store paths. Instead of wrongly using `PathSet`, or "stringly typed" using `StringSet`, use `std::std<StringContextElem>`. ----- In support of this change, `NixStringContext` is now defined as `std::std<StringContextElem>` not `std:vector<StringContextElem>`. The old definition was just used by a `getContext` method which was only used by the eval cache. It can be deleted altogether since the types are now unified and the preexisting `copyContext` function already suffices. Summarizing the previous paragraph: Old: - `value/context.hh`: `NixStringContext = std::vector<StringContextElem>` - `value.hh`: `NixStringContext Value::getContext(...)` - `value.hh`: `copyContext(...)` New: - `value/context.hh`: `NixStringContext = std::set<StringContextElem>` - `value.hh`: `copyContext(...)` ---- The string representation of string context elements no longer contains the store dir. The diff of `src/libexpr/tests/value/context.cc` should make clear what the new representation is, so we recommend reviewing that file first. This was done for two reasons: Less API churn: `Value::mkString` and friends did not take a `Store` before. But if `NixStringContextElem::{parse, to_string}` *do* take a store (as they did before), then we cannot have the `Value` functions use them (in order to work with the fully-structured `NixStringContext`) without adding that argument. That would have been a lot of churn of threading the store, and this diff is already large enough, so the easier and less invasive thing to do was simply make the element `parse` and `to_string` functions not take the `Store` reference, and the easiest way to do that was to simply drop the store dir. Space usage: Dropping the `/nix/store/` (or similar) from the internal representation will safe space in the heap of the Nix programming being interpreted. If the heap contains many strings with non-trivial contexts, the saving could add up to something significant. ---- The eval cache version is bumped. The eval cache serialization uses `NixStringContextElem::{parse, to_string}`, and since those functions are changed per the above, that means the on-disk representation is also changed. This is simply done by changing the name of the used for the eval cache from `eval-cache-v4` to eval-cache-v5`. ---- To avoid some duplication `EvalCache::mkPathString` is added to abstract over the simple case of turning a store path to a string with just that string in the context. Context This PR picks up where #7543 left off. That one introduced the fully structured `NixStringContextElem` data type, but kept `PathSet context` as an awkward middle ground between internal `char[][]` interpreter heap string contexts and `NixStringContext` fully parsed string contexts. The infelicity of `PathSet context` was specifically called out during Nix team group review, but it was agreeing that fixing it could be left as future work. This is that future work. A possible follow-up step would be to get rid of the `char[][]` evaluator heap representation, too, but it is not yet clear how to do that. To use `NixStringContextElem` there we would need to get the STL containers to GC pointers in the GC build, and I am not sure how to do that. ---- PR #7543 effectively is writing the inverse of a `mkPathString`, `mkOutputString`, and one more such function for the `DrvDeep` case. I would like that PR to have property tests ensuring it is actually the inverse as expected. This PR sets things up nicely so that reworking that PR to be in that more elegant and better tested way is possible. Co-authored-by: Théophane Hufschmitt <7226587+thufschmitt@users.noreply.github.com>
2023-01-29 03:31:10 +02:00
state.store->ensurePath(namePath);
state.forceAttrs(*i.value, i.pos, "while evaluating the value of a string context");
auto iter = i.value->attrs->find(sPath);
if (iter != i.value->attrs->end()) {
if (state.forceBool(*iter->value, iter->pos, "while evaluating the `path` attribute of a string context"))
Use `std::set<StringContextElem>` not `PathSet` for string contexts Motivation `PathSet` is not correct because string contexts have other forms (`Built` and `DrvDeep`) that are not rendered as plain store paths. Instead of wrongly using `PathSet`, or "stringly typed" using `StringSet`, use `std::std<StringContextElem>`. ----- In support of this change, `NixStringContext` is now defined as `std::std<StringContextElem>` not `std:vector<StringContextElem>`. The old definition was just used by a `getContext` method which was only used by the eval cache. It can be deleted altogether since the types are now unified and the preexisting `copyContext` function already suffices. Summarizing the previous paragraph: Old: - `value/context.hh`: `NixStringContext = std::vector<StringContextElem>` - `value.hh`: `NixStringContext Value::getContext(...)` - `value.hh`: `copyContext(...)` New: - `value/context.hh`: `NixStringContext = std::set<StringContextElem>` - `value.hh`: `copyContext(...)` ---- The string representation of string context elements no longer contains the store dir. The diff of `src/libexpr/tests/value/context.cc` should make clear what the new representation is, so we recommend reviewing that file first. This was done for two reasons: Less API churn: `Value::mkString` and friends did not take a `Store` before. But if `NixStringContextElem::{parse, to_string}` *do* take a store (as they did before), then we cannot have the `Value` functions use them (in order to work with the fully-structured `NixStringContext`) without adding that argument. That would have been a lot of churn of threading the store, and this diff is already large enough, so the easier and less invasive thing to do was simply make the element `parse` and `to_string` functions not take the `Store` reference, and the easiest way to do that was to simply drop the store dir. Space usage: Dropping the `/nix/store/` (or similar) from the internal representation will safe space in the heap of the Nix programming being interpreted. If the heap contains many strings with non-trivial contexts, the saving could add up to something significant. ---- The eval cache version is bumped. The eval cache serialization uses `NixStringContextElem::{parse, to_string}`, and since those functions are changed per the above, that means the on-disk representation is also changed. This is simply done by changing the name of the used for the eval cache from `eval-cache-v4` to eval-cache-v5`. ---- To avoid some duplication `EvalCache::mkPathString` is added to abstract over the simple case of turning a store path to a string with just that string in the context. Context This PR picks up where #7543 left off. That one introduced the fully structured `NixStringContextElem` data type, but kept `PathSet context` as an awkward middle ground between internal `char[][]` interpreter heap string contexts and `NixStringContext` fully parsed string contexts. The infelicity of `PathSet context` was specifically called out during Nix team group review, but it was agreeing that fixing it could be left as future work. This is that future work. A possible follow-up step would be to get rid of the `char[][]` evaluator heap representation, too, but it is not yet clear how to do that. To use `NixStringContextElem` there we would need to get the STL containers to GC pointers in the GC build, and I am not sure how to do that. ---- PR #7543 effectively is writing the inverse of a `mkPathString`, `mkOutputString`, and one more such function for the `DrvDeep` case. I would like that PR to have property tests ensuring it is actually the inverse as expected. This PR sets things up nicely so that reworking that PR to be in that more elegant and better tested way is possible. Co-authored-by: Théophane Hufschmitt <7226587+thufschmitt@users.noreply.github.com>
2023-01-29 03:31:10 +02:00
context.emplace(NixStringContextElem::Opaque {
.path = namePath,
});
}
iter = i.value->attrs->find(sAllOutputs);
if (iter != i.value->attrs->end()) {
if (state.forceBool(*iter->value, iter->pos, "while evaluating the `allOutputs` attribute of a string context")) {
if (!isDerivation(name)) {
throw EvalError({
.msg = hintfmt("tried to add all-outputs context of %s, which is not a derivation, to a string", name),
.errPos = state.positions[i.pos]
});
}
Use `std::set<StringContextElem>` not `PathSet` for string contexts Motivation `PathSet` is not correct because string contexts have other forms (`Built` and `DrvDeep`) that are not rendered as plain store paths. Instead of wrongly using `PathSet`, or "stringly typed" using `StringSet`, use `std::std<StringContextElem>`. ----- In support of this change, `NixStringContext` is now defined as `std::std<StringContextElem>` not `std:vector<StringContextElem>`. The old definition was just used by a `getContext` method which was only used by the eval cache. It can be deleted altogether since the types are now unified and the preexisting `copyContext` function already suffices. Summarizing the previous paragraph: Old: - `value/context.hh`: `NixStringContext = std::vector<StringContextElem>` - `value.hh`: `NixStringContext Value::getContext(...)` - `value.hh`: `copyContext(...)` New: - `value/context.hh`: `NixStringContext = std::set<StringContextElem>` - `value.hh`: `copyContext(...)` ---- The string representation of string context elements no longer contains the store dir. The diff of `src/libexpr/tests/value/context.cc` should make clear what the new representation is, so we recommend reviewing that file first. This was done for two reasons: Less API churn: `Value::mkString` and friends did not take a `Store` before. But if `NixStringContextElem::{parse, to_string}` *do* take a store (as they did before), then we cannot have the `Value` functions use them (in order to work with the fully-structured `NixStringContext`) without adding that argument. That would have been a lot of churn of threading the store, and this diff is already large enough, so the easier and less invasive thing to do was simply make the element `parse` and `to_string` functions not take the `Store` reference, and the easiest way to do that was to simply drop the store dir. Space usage: Dropping the `/nix/store/` (or similar) from the internal representation will safe space in the heap of the Nix programming being interpreted. If the heap contains many strings with non-trivial contexts, the saving could add up to something significant. ---- The eval cache version is bumped. The eval cache serialization uses `NixStringContextElem::{parse, to_string}`, and since those functions are changed per the above, that means the on-disk representation is also changed. This is simply done by changing the name of the used for the eval cache from `eval-cache-v4` to eval-cache-v5`. ---- To avoid some duplication `EvalCache::mkPathString` is added to abstract over the simple case of turning a store path to a string with just that string in the context. Context This PR picks up where #7543 left off. That one introduced the fully structured `NixStringContextElem` data type, but kept `PathSet context` as an awkward middle ground between internal `char[][]` interpreter heap string contexts and `NixStringContext` fully parsed string contexts. The infelicity of `PathSet context` was specifically called out during Nix team group review, but it was agreeing that fixing it could be left as future work. This is that future work. A possible follow-up step would be to get rid of the `char[][]` evaluator heap representation, too, but it is not yet clear how to do that. To use `NixStringContextElem` there we would need to get the STL containers to GC pointers in the GC build, and I am not sure how to do that. ---- PR #7543 effectively is writing the inverse of a `mkPathString`, `mkOutputString`, and one more such function for the `DrvDeep` case. I would like that PR to have property tests ensuring it is actually the inverse as expected. This PR sets things up nicely so that reworking that PR to be in that more elegant and better tested way is possible. Co-authored-by: Théophane Hufschmitt <7226587+thufschmitt@users.noreply.github.com>
2023-01-29 03:31:10 +02:00
context.emplace(NixStringContextElem::DrvDeep {
.drvPath = namePath,
});
}
}
iter = i.value->attrs->find(state.sOutputs);
if (iter != i.value->attrs->end()) {
state.forceList(*iter->value, iter->pos, "while evaluating the `outputs` attribute of a string context");
if (iter->value->listSize() && !isDerivation(name)) {
throw EvalError({
.msg = hintfmt("tried to add derivation output context of %s, which is not a derivation, to a string", name),
.errPos = state.positions[i.pos]
});
}
for (auto elem : iter->value->listItems()) {
auto outputName = state.forceStringNoCtx(*elem, iter->pos, "while evaluating an output name within a string context");
Use `std::set<StringContextElem>` not `PathSet` for string contexts Motivation `PathSet` is not correct because string contexts have other forms (`Built` and `DrvDeep`) that are not rendered as plain store paths. Instead of wrongly using `PathSet`, or "stringly typed" using `StringSet`, use `std::std<StringContextElem>`. ----- In support of this change, `NixStringContext` is now defined as `std::std<StringContextElem>` not `std:vector<StringContextElem>`. The old definition was just used by a `getContext` method which was only used by the eval cache. It can be deleted altogether since the types are now unified and the preexisting `copyContext` function already suffices. Summarizing the previous paragraph: Old: - `value/context.hh`: `NixStringContext = std::vector<StringContextElem>` - `value.hh`: `NixStringContext Value::getContext(...)` - `value.hh`: `copyContext(...)` New: - `value/context.hh`: `NixStringContext = std::set<StringContextElem>` - `value.hh`: `copyContext(...)` ---- The string representation of string context elements no longer contains the store dir. The diff of `src/libexpr/tests/value/context.cc` should make clear what the new representation is, so we recommend reviewing that file first. This was done for two reasons: Less API churn: `Value::mkString` and friends did not take a `Store` before. But if `NixStringContextElem::{parse, to_string}` *do* take a store (as they did before), then we cannot have the `Value` functions use them (in order to work with the fully-structured `NixStringContext`) without adding that argument. That would have been a lot of churn of threading the store, and this diff is already large enough, so the easier and less invasive thing to do was simply make the element `parse` and `to_string` functions not take the `Store` reference, and the easiest way to do that was to simply drop the store dir. Space usage: Dropping the `/nix/store/` (or similar) from the internal representation will safe space in the heap of the Nix programming being interpreted. If the heap contains many strings with non-trivial contexts, the saving could add up to something significant. ---- The eval cache version is bumped. The eval cache serialization uses `NixStringContextElem::{parse, to_string}`, and since those functions are changed per the above, that means the on-disk representation is also changed. This is simply done by changing the name of the used for the eval cache from `eval-cache-v4` to eval-cache-v5`. ---- To avoid some duplication `EvalCache::mkPathString` is added to abstract over the simple case of turning a store path to a string with just that string in the context. Context This PR picks up where #7543 left off. That one introduced the fully structured `NixStringContextElem` data type, but kept `PathSet context` as an awkward middle ground between internal `char[][]` interpreter heap string contexts and `NixStringContext` fully parsed string contexts. The infelicity of `PathSet context` was specifically called out during Nix team group review, but it was agreeing that fixing it could be left as future work. This is that future work. A possible follow-up step would be to get rid of the `char[][]` evaluator heap representation, too, but it is not yet clear how to do that. To use `NixStringContextElem` there we would need to get the STL containers to GC pointers in the GC build, and I am not sure how to do that. ---- PR #7543 effectively is writing the inverse of a `mkPathString`, `mkOutputString`, and one more such function for the `DrvDeep` case. I would like that PR to have property tests ensuring it is actually the inverse as expected. This PR sets things up nicely so that reworking that PR to be in that more elegant and better tested way is possible. Co-authored-by: Théophane Hufschmitt <7226587+thufschmitt@users.noreply.github.com>
2023-01-29 03:31:10 +02:00
context.emplace(NixStringContextElem::Built {
.drvPath = namePath,
.output = std::string { outputName },
});
}
}
}
2022-01-04 19:24:42 +02:00
v.mkString(orig, context);
}
static RegisterPrimOp primop_appendContext({
.name = "__appendContext",
.arity = 2,
.fun = prim_appendContext
});
}