Old code would do very much incorrect reentrancy crimes (trying to do an
erase inside the emplace callback). This would fail miserably with an assertion
in Boost:
terminating due to unexpected unrecoverable internal error: Assertion '(!find(px))&&("reentrancy not allowed")' failed in boost::unordered::detail::foa::entry_trace::entry_trace(const void *) at include/boost/unordered/detail/foa/reentrancy_check.hpp:33
This is trivially reproduced by using any S3 URL with a non-empty profile:
nix-prefetch-url "s3://happy/crash?profile=default"
We now unconditionally compile support for s3:// URLs and stores
without authentication. The whole curl version check can be greatly
simplified by the previous commit, which bumps the minimum required curl
version.
This version has been released a long time ago in 2021 and it's doubtful
that anybody actually uses it still, since it's full of vulnerabilities [^]
[^]: https://curl.se/docs/vuln-7.75.0.html
I realized that we can actually do this thing, even though it is not
what nlohmann expects at all, because the extra parameter has a default
argument so nlohmann doesn't need to care. Sneaky!
Since 3c610df550 this resulted in `getting status of`
errors on paths inside the chroot if a path was already valid. Careful inspection
of the logic shows that if buildMode != bmCheck actualPath gets reassigned to
store.toRealPath(finalDestPath). The only branch that cares about actualPath is
the buildMode == bmCheck case, which doesn't lead to optimisePath anyway.
Instead of the cryptic:
> error: Failed to resolve AWS credentials: error code 6153`
We now get more legible:
> error: AWS authentication error: 'Valid credentials could not be sourced by the IMDS provider' (6153)
This makes it so we don't need to rely on global variables and hacky destructors to
clean up another global variable. Just putting it in the correct order in the class
is more than enough.
This partially reverts commit 5e46df973f,
partially reversing changes made to
8c789db05b.
We do this because Hydra, while using the newer version of the protocol,
still uses this command, even though Nix (as a client) doesn't use it.
On that basis, we don't want to remove it (or consider it only part of
the older versions of the protocol) until Hydra no longer uses the
Legacy SSH Protocol.
Realisations are conceptually key-value pairs, mapping `DrvOutputs` (the
key) to information about that derivation output.
This separate the value type, which will be useful in maps, etc., where
we don't want to denormalize by including the key twice.
This matches similar changes for existing types:
| keyed | unkeyed |
|--------------------|------------------------|
| `ValidPathInfo` | `UnkeyedValidPathInfo` |
| `KeyedBuildResult` | `BuildResult` |
| `Realisation` | `UnkeyedRealisation` |
Co-authored-by: Sergei Zimmerman <sergei@zimmerman.foo>
The macro now accurately reflects its purpose: gating only AWS
authentication code, not all S3 functionality. S3 URL parsing, store
configuration, and public bucket access work regardless of this flag.
This rename clarifies that:
- S3 support is always available (URL parsing, store registration)
- Only AWS credential resolution requires the flag
- The flag controls AWS CRT SDK dependency, not S3 protocol support
Move S3 URL parsing, store configuration, and public bucket support
outside of NIX_WITH_S3_SUPPORT guards. Only AWS credential resolution
remains gated, allowing builds with withAWS = false to:
- Parse s3:// URLs
- Register S3 store types
- Access public S3 buckets (via HTTPS conversion)
- Use S3-compatible services without authentication
The setupForS3() function now always performs URL conversion, with
authentication code conditionally compiled based on NIX_WITH_S3_SUPPORT.
The aws-creds.cc file (only code using AWS CRT SDK) is now conditionally
compiled by meson.
This commit replaces the AWS C++ SDK with a lighter curl-based approach
for S3 binary cache operations.
- Removed dependency on the heavy aws-cpp-sdk-s3 and aws-cpp-sdk-transfer
- Added lightweight aws-crt-cpp for credential resolution only
- Leverages curl's native AWS SigV4 authentication (requires curl >= 7.75.0)
- S3BinaryCacheStore now delegates to HttpBinaryCacheStore
- Function s3ToHttpsUrl converts ParsedS3URL to ParsedURL
- Multipart uploads are no longer supported (may be reimplemented later)
- Build now requires curl >= 7.75.0 for AWS SigV4 support
Fixes: #13084, #12671, #11748, #12403, #5947
We only care about the accessor for a single store object anyway, but
the validity gets ignored. Also `pathExists(store.printStorePath(path))`
is definitely incorrect since it confuses the logical location vs physical
location in case of a chroot store.
This is a simple wrapper around getFSAccessor that throws an InvalidPath
error. This simplifies usage in callsites that only care about getting
a non-null accessor.
Wrap fmt() calls in lambdas to defer string formatting until the
feature check fails. This avoids unnecessary string formatting in
the common case where the feature is enabled.
Addresses performance concern raised by xokdvium in PR review.
The refactor in the last commit fixed the bug it was supposed to fix,
but introduced a new bug in that sometimes we tried to write a resolved
derivation to a store before all its `inputSrcs` were in that store.
The solution is to defer writing the derivation until inside
`DerivationBuildingGoal`, just before we do an actual build. At this
point, we are sure that all inputs in are the store.
This does have the side effect of meaning we don't write down the
resolved derivation in the substituting case, only the building case,
but I think that is actually fine. The store that actually does the
building should make a record of what it built by storing the resolved
derivation. Other stores that just substitute from that store don't
necessary want that derivation however. They can trust the substituter
to keep the record around, or baring that, they can attempt to re
resolve everything, if they need to be audited.
(cherry picked from commit c97b050a6c)
Resolve the derivation before creating a building goal, in a context
where we know what output(s) we want. That way we have a chance just to
download the outputs we want.
Fix#13247
(cherry picked from commit 39f6fd9b46)
std::regex is a really bad tool for parsing things, since
it tends to overflow the stack pretty badly. See the build failure
under ASan in [^].
[^]: https://hydra.nixos.org/build/310077167/nixlog/5
CURL is not very strict about validation of URLs passed to it. We
should reflect this in our handling of URLs that we get from the user
in <nix/fetchurl.nix> or builtins.fetchurl. ValidURL was an attempt to
rectify this, but it turned out to be too strict. The only good way to
resolve this is to pass (in some cases) the user-provided string verbatim
to CURL. Other usages in libfetchers still benefit from using structured
ParsedURL and validation though.
nix store prefetch-file --name foo 'https://cdn.skypack.dev/big.js@^5.2.2'
error: 'https://cdn.skypack.dev/big.js@^5.2.2' is not a valid URL: leftover
Add support for pre-resolving AWS credentials in the parent process
before forking for builtin:fetchurl. This avoids recreating credential
providers in the forked child process.
The previous implementation had a check-then-create race condition where
multiple threads could simultaneously:
1. Check the cache and find no provider (line 122)
2. Create their own providers (lines 126-145)
3. Insert into cache (line 161)
This resulted in multiple credential providers being created when
downloading multiple packages in parallel, as each .narinfo download
would trigger provider creation on its own thread.
Fix by using boost::concurrent_flat_map's try_emplace_and_cvisit, which
provides atomic get-or-create semantics:
- f1 callback: Called atomically during insertion, creates the provider
- f2 callback: Called if key exists, returns cached provider
- Other threads are blocked during f1, so no nullptr is ever visible
