This seems quite natural to me, since boehm is necessarily a global
resource. I still think that it would be best if we don't make it a
requirement to only have a single instance of EvalState per thread, but
specifically for this use-case thread_local is a perfect fit.
- `fun` was used for variable/field names in a number of places.
These have been replaced by similar or more descriptive names,
to avoid name shadowing, which is not allowed in the GCC-based build.
- Use in-place construction instead of assignment.
- `FilteringSourceAccessor::checkAccess` had a fallback for when
`makeNotAllowedError` was null. Since `MakeNotAllowedError` is now
`fun<>`, we've proven the null branch is dead code and have removed it.
- `src/nix/flake.cc`, `src/nix/search.cc`, `src/nix/ls.cc` are left
as-is — the self-referential lambdas there cause too much
reindentation for too little benefit.
Like `ref<T>` guarantees a non-null pointer, `fun<Sig>` guarantees a
non-null callable.
When a callable is nullable, use `std::function` directly — not
`optional<fun<>>`. The two types serve complementary roles:
`fun<Sig>` for non-null, `std::function<Sig>` for nullable.
Construction from callables (lambdas, function pointers) is implicit
since these are inherently non-null. Construction from `std::function`
is explicit, since it may be empty. `fun(nullptr)` is a deleted
overload.
A moved-from `fun<>` violates its non-null invariant, matching the
precedent set by `ref<T>`. Deleting moves was considered but deemed
unnecessary: the goal of this type is to document and enforce an
invariant, not to force non-null at all costs including performance.
`get_fn()` exposes the underlying `std::function` for API that only
exists there, such as `target<>()`.
This commit replaces all usages with their underlying types, that being `std::string` for store
paths, `std::filesystem::path` for filesystem paths, `StringSet` for
path sets, and `std::string_view` for non-owning references.
This allows capturing the current value/result of
`errno`/`GetLastError()` before constructing the error message. Useful
when the error message construction itself might clobber the error code
(e.g., calling `descriptorToPath()`).
Usage:
```
throw NativeSysError([&] { return HintFmt("msg %s", foo()); });
```
The error code is captured when the exception is constructed, then the
lambda is called to produce the `HintFmt`.
Also fix the Windows build
Bug has existed for a long time, but was only recently surfaced by
6733f2e5ce. time_t was being implicitly promoted
to unsigned. Apparently time_t is still 32 bit for i686-linux in nixpkgs.
Fixes https://hydra.nixos.org/build/322992746/nixlog/1
forceStringNoCtx error messages leaked internal string context
representation (e.g. '!out!<hash>-name.drv'). Use DerivedPath
syntax instead: '<store-path>^out' for built, plain path for
opaque, and '<drv-path> (deep)' for deep derivation references.
When the common pattern for store config constructors is to accept
an arbitrary string it's unclear whether it needs pct-decoding or not.
Prior to c436b7a32a the string passed to
the constructors was a mix of encoded authority and decoded path concatenated
with a `/`. After that commit it accidentally started accepting pct-encoded
result of renderAuthorityAndPath, but only in some code paths. This lead to
file:///tmp/a+b to be created on disk in /tmp/a%2Bb directory. Similar issue
affected the less-known variant with local:///tmp/a+b. Regular store references
that are local paths were not affected.
This patch changes the constructors to accept different types to signify what
is actually needed to let the factory method handle this in a consistent way:
- std::filesystem::path - local binary cache store and local store
- ParsedURL::Authority - for ssh stores
- ParsedURL - for http stores
(Some MinGW build fixes by Amaan Qureshi <git@amaanq.com>)
This reflects the fact that it is returning a new, "owned" file
descriptor, that it is the caller's responsibility to close.
Co-authored-by: Amaan Qureshi <git@amaanq.com>
