the very slight speedup in config setting access is not worth the
maintenance overhead of conflating concers like this. the virtual
inheritance scheme used for configs requires too much duplication
of base class constructor arguments to be worth doing. perhaps we
should get rid of all virtual inheritance of data-membered bases?
Change-Id: I4acf5ceaedb4ed7476efe1114c2e065ec72d2c6d
Ever read gdb output and you just kinda get a headache because you have
to infer what a thread is by reading the stack trace? It's not hard, but
we could also just never have to do that again, which is also not hard.
Sample:
(gdb) info thr
Id Target Id Frame
* 1 LWP 3719283 "nix-daemon" 0x00007e558587da0f in accept ()
from target:/nix/store/c10zhkbp6jmyh0xc5kd123ga8yy2p4hk-glibc-2.39-52/lib/libc.so.6
2 LWP 3719284 "signal handler" 0x00007e55857b2bea in sigtimedwait ()
from target:/nix/store/c10zhkbp6jmyh0xc5kd123ga8yy2p4hk-glibc-2.39-52/lib/libc.so.6
The API design for this is forced by the macOS pthread_setname_np only
being able to change the current thread's name, but if we just conform
everything to that, it works everywhere.
Change-Id: I2b1d6ed41e3c94170cb0b4e73ad66f239ebd9c88
file:// urls are no longer handled by curl itself, and this is supposed
to test the curl wrapper. use http to force the wrapper to be involved.
Change-Id: Ib13087db07b3b2f1ae44ce8e3ec7a96d935b1bab
this only ever worked for empty uploads, and there it worked only by
complete accident: curl was asked to send more data than the wrapper
would provide, which curl would not like and report as an error. the
error would cause a retry with even less data to send, until finally
failing by running into the retry limit. let's just forbid all this.
Change-Id: I229a94b3b8b33e2c6cdb8ea19edd57cd6740e6c6
This adds a new temp-dir setting for controlling the temporary directory
without having to change the TMPDIR env var. This can be used to e.g.
use a path on a case-sensitive store on macOS for temporary files
without changing the TMPDIR var used by interactive shells or commands
invoked with `nix run`.
This also stops unsetting `TMPDIR` on darwin when the env var value
starts with `/var/folders/`, preferring instead to just do the check
when reading `TMPDIR`. This way the inherited `TMPDIR` env var is
preserved for child processes (such as interactive shells).
As a side effect this changes the behavior of `nix-build -o ''` to act
like `nix-build --no-out-link` instead of failing with an error caused
by trying to create a symlink at the cwd.
Fixes: #253
Fixes: #112
Change-Id: I9ee826323f2deca62854715a77ca7a373a948a29
don't pause the entire curl thread. we have multiple consumer threads
after all, not just one, so stalling all of them is likely not great.
note that libcurl advises against using transfer pauses if compressed
encodings are allowed and automatically decoded. this should not lead
to problems in practice because our data is usually not compressed to
such a degree that curl buffering *uncompressed* data matters. should
this cause problems we can reintroduce the whole-thread pause, but we
will probably get away with this until the entire file transfer class
is made kj::Promise-using async (and *then* curl can be hardpaused if
it cannot get rid of its data, solving the problem once and for all).
Change-Id: I218e41bfa5a27c7454eafb0bdb54f2a29a7f6493
it's no longer needed. `download` can do everything `enqueueDownload`
did, and a lot more. e.g. not block the calling thread, for instance.
Change-Id: I4b36235ed707c92d117b4c33efa3db50d26f9a84
return it as a separate item in a pair instead. this will let us remove
enqueueDownload() in favor of returning metadata from download() itself
Change-Id: I74fad2ca15f920da1eefabc950c2baa2c360f2ba
it's just a uri and some headers now. those can be function arguments
with no loss of clarity. *actual* additional arguments, for example a
TLS context with additional certificates, could be added on a new and
improved FileTransfer class that carries not just a backend reference
but some real, visible context for its transfers. curl not being very
multi-threading-friendly when using multi handles will make sharing a
bit hard anyway once we drop the single global download worker thread
Change-Id: Id2112c95cbd118c6d920488f38d272d7da926460
we don't even need this outside of tests. maybe we should not do
automatic retries at this level at all and use retrying wrappers
instead? at some point we may have to do this, but not just yet.
Change-Id: If0088aa55215be81f1770c25b3bb1b5268c65cf8
Only overridden settings are sent to the daemon, and we're going to do the same
for the build hook to. It needs to be ensured that overridden settings are in
fact consistently marked as such, so that they actually get sent.
Change-Id: I7cd58d925702f86cf2c35ad121eb191ceb62a355
There was a bug report about a potential call to `memcpy` with a null
pointer which is not reproducible:
#492
This occurred in `src/libstore/filetransfer.cc` in `InnerSource::read`.
To ensure that this doesn't happen, an early return is added before
calling `memcpy` if the length of the data to be copied is 0.
This change also adds a test that ensures that when `InnerSource::read`
is called with an empty file, it throws an `EndOfFile` exception.
Change-Id: Ia18149bee9a3488576c864f28475a3a0c9eadfbb
This:
- Converts a bunch of C style casts into C++ casts.
- Removes some very silly pointer subtraction code (which is no more or
less busted on i686 than it began)
- Fixes some "technically UB" that never had to be UB in the first
place.
- Makes finally follow the noexcept status of the inner function. Maybe
in the future we should ban the function from not being noexcept, but
that is not today.
- Makes various locally-used exceptions inherit from std::exception.
Change-Id: I22e66972602604989b5e494fd940b93e0e6e9297
this is cursed. deeply and profoundly cursed. under NO CIRCUMSTANCES
must protocol serializer helpers be applied to temporaries! doing so
will inevitably cause dangling references and cause the entire thing
to crash. we need to do this even so to get rid of boost coroutines,
and likewise to encapsulate the serializers we suffer today at least
a little bit to allow a gradual migration to an actual IPC protocol.
(this isn't a problem that's unique to generators. c++ coroutines in
general cannot safely take references to arbitrary temporaries since
c++ does not have a lifetime system that can make this safe. -sigh-)
Change-Id: I2921ba451e04d86798752d140885d3c5cc08e146
without this we will not be able to get rid of makeDecompressionSink,
which in turn will be necessary to get rid of sourceToSink (since the
libarchive archive wrapper *must* be a Source due to api limitations)
Change-Id: Iccd3d333ba2cbcab49cb5a1d3125624de16bce27
even the transfer function is not all that necessary since there aren't
that many users, but we'll keep it for now. we could've kept both names
but we also kind of want to use `download` for something else very soon
Change-Id: I005e403ee59de433e139e37aa2045c26a523ccbf
We don't want to deal with these at all, let's stop doing so.
(marking this one as the fix commit since its immediate predecessors
aren't the complete fix)
Fixes: #325
Change-Id: Ieea1b0b8ac0f903d1e24e5b3e63cfe12eeec119d
The UnkeyedValidPathInfo test was testing an ancient version but not the
current version. Doesn't make much sense to me.
Change-Id: Ib476a4297d9075f2dcd31a073b3e7b149b2189af
This reverts commit 285bc67318.
Reason for revert: #364
For some reason this broke `main` even though the change we are reverting passed CI! Mysterious, haunted, etc. Needs more debugging, let's turn it off for now.
Change-Id: Ica4819d61cd35b83eb52985bfcb657e858f025a9
* changes:
util.hh: Delete remaining file and clean up headers
util.hh: Move nativeSystem to local-derivation-goal.cc
util.hh: Move stuff to types.hh
util.cc: Delete remaining file
util.{hh,cc}: Move ignoreException to error.{hh,cc}
util.{hh,cc}: Split out namespaces.{hh,cc}
util.{hh,cc}: Split out users.{hh,cc}
util.{hh,cc}: Split out strings.{hh,cc}
util.{hh,cc}: Split out unix-domain-socket.{hh,cc}
util.{hh,cc}: Split out child.{hh,cc}
util.{hh,cc}: Split out current-process.{hh,cc}
util.{hh,cc}: Split out processes.{hh,cc}
util.{hh,cc}: Split out file-descriptor.{hh,cc}
util.{hh,cc}: Split out file-system.{hh,cc}
util.{hh,cc}: Split out terminal.{hh,cc}
util.{hh,cc}: Split out environment-variables.{hh,cc}
while refactoring the curl wrapper we inadvertently broken the immutable
flake protocol, because the immutable flake protocol accumulates headers
across the entire redirect chain instead of using only the headers given
in the final response of the chain. this is a problem because Some Known
Providers Of Flake Infrastructure set rel=immutable link headers only in
the penultimate entry of the redirect chain, and curl does not regard it
as worth returning to us via its response header enumeration mechanisms.
fixes #358
Change-Id: I645c3932b465cde848bd6a3565925a1e3cbcdda0
Example: /nix/store/dr53sp25hyfsnzjpm8mh3r3y36vrw3ng-neovim-0.9.5^out
This is nonsensical since selecting outputs can only be done for a
buildable derivation, not for a realised store path. The build worker
side of things ends up crashing with an assertion when trying to handle
such malformed paths.
Change-Id: Ia3587c71fe3da5bea45d4e506e1be4dd62291ddf
Very basic behavior test to ensure that gzip data gets internally
decompressed by the file transfer pipeline.
Change a std::string_view return value in the test harness to
std::string. I wouldn't call myself a C++ beginner and I still managed
to shoot myself in the foot like three times with the lifetime
managements there (e.g. [&] { return an_std_string; } ends up with a
dangling string_view!).
Change-Id: I1360750d4181ce1ca2a3aa4dc0e97e131351c469
also add a few more tests for exception propagation behavior. using
packaged_tasks and futures (which only allow a single call to a few
of their methods) introduces error paths that weren't there before.
Change-Id: I42ca5236f156fefec17df972f6e9be45989cf805
not doing this will cause transfers that had their readers disappear to
linger. with lingering transfers the curl thread can't shut down, which
will cause nix itself to not shut down until the transfer finishes some
other way (most likely network timeouts). also add a new test for this.
Change-Id: Id2401b3ac85731c824db05918d4079125be25b57
As discussed in the maintainer meeting on 2024-01-29.
Mainly this is to avoid a situation where the name is parsed and
treated as a file name, mostly to protect users.
.-* and ..-* are also considered invalid because they might strip
on that separator to remove versions. Doesn't really work, but that's
what we decided, and I won't argue with it, because .-* probably
doesn't seem to have a real world application anyway.
We do still permit a 1-character name that's just "-", which still
poses a similar risk in such a situation. We can't start disallowing
trailing -, because a non-zero number of users will need it and we've
seen how annoying and painful such a change is.
What matters most is preventing a situation where . or .. can be
injected, and to just get this done.
(cherry picked from commit f1b4663805a9dbcb1ace64ec110092d17c9155e0)
Change-Id: I900a8509933cee662f888c3c76fa8986b0058839
* some things that can throw are marked noexcept
yet the linter seems to think not. Maybe they can't throw in practice.
I would rather not have the UB possibility in pretty obvious cold
paths.
* various default-case-missing complaints
* a fair pile of casts from integer to character, which are in fact
deliberate.
* an instance of <https://clang.llvm.org/extra/clang-tidy/checks/bugprone/move-forwarding-reference.html>
* bugprone-not-null-terminated-result on handing a string to curl in
chunks of bytes. our usage is fine.
* reassigning a unique_ptr by CRIMES instead of using release(), then
using release() and ignoring the result. wild. let's use release() for
its intended purpose.
Change-Id: Ic3e7affef12383576213a8a7c8145c27e662513d
this lets us set per-test-program environment variables rather than only
a single, global default. this was supported in nix originally but
might've gone partially missing in the upstream backports process?
Change-Id: Iad0919841b1b6d11e0b7ebd3920449a62f544e77
This has some Flaws for sure (like, it is going to be a bit stretched to
use for repl characterization), but it is a start.
Change-Id: I258c8beb3aee236f45818a03be83bcda858120c9
Factor out `ServeProto::Serialiser<UnkeyedValidPathInfo>` and test
(cherry picked from commit 139982997eec493a0f74105c427953f6be77da6d)
Change-Id: I28e4ba5a681a90d81915a56e6dbaa5456d64f96d
When reviewing old PRs, I found that #9997 adds some code to ensure one
particular assert is always present. But, removing asserts isn't
something we do in our own release builds either in the flake here or in
nixpkgs, and is plainly a bad idea that increases support burden,
especially if other distros make bad choices of build flags in their Nix
packaging.
For context, the assert macro in the C standard is defined to do nothing
if NDEBUG is set.
There is no way in our build system to set -DNDEBUG without manually
adding it to CFLAGS, so this is simply a configuration we do not use.
Let's ban it at compile time.
I put this preprocessor directive in src/libutil.cc because it is not
obvious where else to put it, and it seems like the most logical file
since you are not getting a usable nix without it.
Upstream-PR: https://github.com/NixOS/nix/pull/10126
Original-Change-Id: I513cceaac1371decb3d96231e6ef9181c910c218
Change-Id: I531a51f6348a746e8e41d88203b08f614898356c
Convert `Machine::speedFactor` from a non-neg int to a non-neg float
(cherry picked from commit 69d0ae27e376e7c7c4f237716b0149223b8a805a)
Change-Id: I2afb5cf9e4fe1384985c58353946135c3d102b42
Turn derivation unit tests into unit characterization tests
(cherry picked from commit a6e587923c9d5d716fe0f0049bed96d1cc210bff)
Change-Id: Ia2a2e65aabfee8d5d52142b8fdaacbae4a27242c
Give `nix daemon` and `nix-store --serve` protocols separate serializers with version info
(cherry picked from commit 8b68bbb77745fda0d14939b6c23d31cc89da41ce)
Change-Id: Ia3d3b9fbaf9f0ae62ab225020b7d14790e793655