it's only used by HttpBinaryCacheStore, and even there used in only on
place. this one place can set the header explicitly, which it now does
Change-Id: Id89228150669e25e7f59a3d6bd939e46059ce29e
it only sets the one field anyway (and the parent activity as a side
effect that does not depend on the exact location of the constructor
call). when FileTransferRequest goes away we would need this anyway.
Change-Id: I35cf2ed3533239181449a62cf34cd282b395e5db
* changes:
treewide: make more settings conditionally available
libstore/build: only send overridden settings to the build hook
treewide: consistently mark overridden settings as such
a first little step into pushing the event loops up, up and away.
eventually we will want them to be instantiated only at the roots
of every thread (since kj binds loops to threads), but not today.
Change-Id: Ic97f1debba382a5a3f46daeaf2d6d434ee42569f
goals should be considered internal to the worker architecture due to
the tight coupling of the two, and we can finally do that. doing this
is also a prerequisite for turning Worker::run() into a real promise.
Change-Id: I7cf273d4a6fdb75b8d192fce1af07c6265ff6980
previously it was possible to fetchurl a dict server, or an ldap server,
or an imap server. this is a bit of a problem, both because rare schemes
may not be available on all systems, and because some schemes (e.g. scp)
are inherently insecure in potentially surprising ways we needn't allow.
Change-Id: I18fc567c6f58c3221b5ea8ce927f4da780057828
we already have a results type for the entire worker call, we may as
well put this bit of info in there. we do keep the assumption of the
old code that the field will only be read if some goals have failed;
fixing that is a very different mess, and not immediately necessary.
Change-Id: If3fc32649dcd88e1987cdd1758c6c5743e3b35ac
without this derivations do not show as completely processed in the
internal-json logs (or the newer multiline output). the former also
breaks external tools like nix-output-monitor which, like multiline
output, grow vertically until at least some goals are finally freed
Change-Id: I55758daf526ba29ae15fb82e0d88da8afb45bf5c
optimise-store.cc used std::regex on darwin, but forgot to include the
header. This probably compiled due to the precompiled headers file, but
it caused errors in the editor.
Change-Id: I23297c08cb66d44e4d4f303560f46e4adc7d5a43
It's not clear to me if `proc_pidinfo()` or `proc_pidfdinfo()` can
actually return negative values, the syscall wrappers convert `-1` into
zero and the semantics suggest that negative values don't make sense,
but just to be safe we'll preserve the int type until we've checked that
it's a positive value.
Fixes: #548
Change-Id: If575aec6b1e27dba63091c7a0316c7b3788747cd
some promises capture `this`. we could also allocate a shared state,
but this thing doesn't really need to ever be moved anyway. so there.
Change-Id: I50b5c44684a8ab4e984b1323de21f97ace4a864a
not doing this can freeze slots until the goal that occupied them is
freed (rather than simply complete), and then can freeze the system.
fixes#549
Change-Id: I042df04222f8ffbaa18ef4a4eae6cbd6f89b679e
The old behavior results in lots of concatenations happening for no good
reason and is an artifact of the technical limitations of the old parser
(combined with some lack of care for such details).
Change-Id: I0d78d6220ca6aeaa10bc437e48e08bf7922e0bb3
This is only a minor semantical distinction, but we should be able to
properly test it, and the parser tests rely on show for that.
Change-Id: I25e868cf9544e30cdff17deb5fd50a434e0f367e
This commit should faithfully reproduce the old behavior down to the
bugs. The new code is a lot more readable, all quirks are well
documented, and it is overall much more maintainable.
Change-Id: I629585918e4f2b7d296b6b8330235cdc90b7bade
Some settings only make sense on particular platforms, or only when a certain
experimental feature is enabled. Several of those were already conditionally
available. Do the same for a bunch more instead of silently ignoring them.
Exceptionally, the use-case-hack setting is not made conditional because it is
included in the test suite.
Change-Id: I29e66ad8ee6178a7c0eff9efb55c3410fae32514
The build hook is still running locally, so it will run with the same default
settings. Hence, just as with the daemon, it is enough to send it only the
overridden settings. This will prevent warnings like
warning: Ignoring setting 'auto-allocate-uids' because experimental feature 'auto-allocate-uids' is not enabled
when the user didn't actually set those settings.
This is inspired by and an alternative to [0].
[0] https://github.com/NixOS/nix/pull/10049
Change-Id: I77ea62cd017614b16b55979dd30e75f09f860d21
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
A better fix than in 104448e75d, hence a
revert + the fix.
It turns out that this commit has the side-effect that when having e.g.
`StrictHostKeyChecking=accept-new` for a remote builder, the warnings à la
Warning: Permanently added 'builder' (ED25519) to the list of known hosts.
actually end up in the derivation's log whereas hostkey verification
errors don't, but only in the stderr of the `nix-build` invocation
(which was the motivation for the patch).
This change writes the stderr from the build-hook to
* the daemon's stderr, so that the SSH errors appear in the journal
(which was the case before 104448e75d)
* the client's stderr, as a log message
* NOT to the drv log (this is handled via `handleJSONLogMessage`)
I tried to fix the issue for legacy-ssh as well, but failed and
ultimately decided to not bother.
I know that we'll sooner or later replace the entire component, however
this is the part of the patch I have working for a while, so I figured I
might still submit it for the time being.
Change-Id: I21ca1aa0d8ae281d2eacddf26e0aa825272707e5
While debugging something else I observed that latest `main` ignores
`Control-C` on `sudo nix-build`.
After reading through the capnproto docs, it seems as if the promise
must be fulfilled to actually terminate the `promise.wait()` below.
This also applies to scenarios such as stopping the client
(`nix-build`), but the builders on the daemon-side are still running,
i.e. closes#540
Co-authored-by: eldritch horrors <pennae@lix.systems>
Change-Id: I9634d14df4909fc1b65d05654aad0309bcca8a0a
So we received a report that the thread pool crashed due to an
Interrupted exception.
Relevant log tail:
copying path '/nix/store/0kal2k73inviikxv9f1ciaj39lkl9a87-etc-os-release' to 'ssh://192.168.0.27'...
Lix crashed. This is a bug. We would appreciate if you report it along with what caused it at https://git.lix.systems/lix-project/lix/issues with the following information included:
error (ignored): error: interrupted by the user
Exception: nix::Interrupted: error: interrupted by the user
Relevant stack trace:
4# __cxa_rethrow in /nix/store/22nxhmsfcv2q2rpkmfvzwg2w5z1l231z-gcc-13.3.0-lib/lib/libstdc++.so.6
5# nix::ignoreExceptionExceptInterrupt(nix::Verbosity) in /nix/store/ghxr2ykqc3rrfcy8rzdys0rzx9ah5fqj-lix-2.92.0-dev-pre20241005-ed9b7f4/lib/liblixutil.so
6# nix::ThreadPool::doWork(bool) in /nix/store/ghxr2ykqc3rrfcy8rzdys0rzx9ah5fqj-lix-2.92.0-dev-pre20241005-ed9b7f4/lib/liblixutil.so
7# 0x00007FA7A00E86D3 in /nix/store/22nxhmsfcv2q2rpkmfvzwg2w5z1l231z-gcc-13.3.0-lib/lib/libstdc++.so.6
8# 0x00007FA79FE99A42 in /nix/store/3dyw8dzj9ab4m8hv5dpyx7zii8d0w6fi-glibc-2.39-52/lib/libc.so.6
9# 0x00007FA79FF1905C in /nix/store/3dyw8dzj9ab4m8hv5dpyx7zii8d0w6fi-glibc-2.39-52/lib/libc.so.6
Notably, this is *not* in the main thread, so this implies that the
thread didn't get joined properly before their destructors got called.
That, in turn, should have only possibly happened because join() threw
on a previous iteration of the loop joining threads, I think. Or if it
threw while in the ThreadPool destructor. Either way we had better stop
letting Interrupted fall out of our child threads!
If:
- Interrupted was thrown inside the action in the main thread: it would
have fallen out of doWork if state->exception was already set and got
caught by ThreadPool::process, calling shutdown() and the join loop
which would crash the process entirely.
- Interrupted was thrown inside the action on a secondary thread: it
would have been caught and put into the exception field and then
possibly rethrown to fall out of the thread (since it was previously
ignoreExceptionExceptInterrupt).
The one possible hole in this hypothesis is that there is an "error
(ignored)" line in there implying that at least one Interrupted got
eaten by an ignoreExceptionInDestructor. It's also unclear whether this
got reordered because of stderr buffering.
Fixes: #542
Change-Id: I322cf050da660af78f5cb0e08ec6e6d27d09ac76
I followed @pennae's advice and moved the constructor definition of
`AttrName` from the header file `nixexpr.hh` to `nixexpr.cc`.
Change-Id: I733f56c25635b366b11ba332ccec38dd7444e793
The approach that was taken here was to add default values to the type
definitions rather than specify them whenever they are missing.
Now the only remaining warning is '-Wunused-parameter' which @jade said
is usually counterproductive and that we can just disable it:
#456 (comment)
So this change adds the flags '-Wall', '-Wextra' and
'-Wno-unused-parameter', so that all warnings are enabled except for
'-Wunused-parameter'.
Change-Id: Ic223a964d67ab429e8da804c0721ba5e25d53012
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
these two functions are now nearly trivial and much better inline into
makeGoalCommon. keeping them separate also separates information about
goal completion flows and how failure information ends up in `Worker`.
Change-Id: I6af86996e4a2346583371186595e3013c88fb082
we can use our newfound powers of Goal::work Is A Real Promise to remove
completed goals from continuation promises. apart from being much easier
to follow it's also a lot more efficient because we have the iterator to
the item we are trying to remove, skipping a linear search of the cache.
Change-Id: Ie0190d051c5f4b81304d98db478348b20c209df5
Goal::work() is a fully usable promise that does not rely on the worker
to report completion conditions. as such we no longer need the `notify`
field that enabled this interplay. we do have to clear goal caches when
destroying the worker though, otherwise goal promises may (incorrectly)
keep goals alive due to strong shared pointers created by childStarted.
Change-Id: Ie607209aafec064dbdf3464fe207d70ba9ee158a
derivation goals still hold a BuildResult member variable since parts of
these results of accumulated in different places, but the Goal class now
no longer has such a field. substitution goals don't need it at all, and
derivation goals should also be refactored to not drop their buildResult
Change-Id: Ic6d3d471cdbe790a6e09a43445e25bedec6ed446
the field is simply duplicated between the two, and now that we can
return WorkResults from Worker::run we no longer need both of them.
Change-Id: I82fc47d050b39b7bb7d1656445630d271f6c9830
this will be needed to move all interesting result fields out of Goal
proper and into WorkResult. once that is done we can treat goals as a
totally internal construct of the worker mechanism, which also allows
us to fully stop exposing unclear intermediate state to Worker users.
Change-Id: I98d7778a4b5b2590b7b070bdfc164a22a0ef7190
since we now propagate goal exceptions properly we no longer need to
check topGoals for a reason to abort early. any early abort reasons,
whether by exception or a clean top goal failure, can now be handled
by inspecting the goal result in the main loop. this greatly reduces
goal-to-goal interactions that do not happen at the main loop level.
since the underscore-free name is now available for use as variables
we'll migrate to that where we currently use `_topGoals` for locals.
Change-Id: I5727c5ea7799647c0a69ab76975b1a03a6558aa6
drop childException since it's no longer needed. also makes
waitForInput, childFinished, and childTerminated redundant.
Change-Id: I05d88ffd323c5b5c909ac21056162f69ffb0eb9f
there's no reason to have the worker set information on goals that the
goals themselves return from their entry point. doing this in the goal
`work()` function is much cleaner, and a prerequisite to removing more
implicit strong shared references to goals that are currently running.
Change-Id: Ibb3e953ab8482a6a21ce2ed659d5023a991e7923
this simplifies the worker loop, and lets us remove it entirely later.
note that ideally only one promise waiting for interrupts should exist
in the entire system. not one per event loop, one per *process*. extra
interrupt waiters make interrupt response nondeterministic and as such
aren't great for user experience. if anything wants to react to aborts
caused by explicit interruptions, or anything else, those things would
be much better served using RAII guards such as Finally (or KJ_DEFER).
Change-Id: I41d035ff40172d536e098153c7375b0972110d51
this was a triumph. i'm making a note here: huge success. it's hard to
overstate my satisfaction! i'm not even angry. i'm being so sincere ri
actually, no. we *are* angry. this was one dumbass odyssey. nobody has
asked for this. but not doing it would have locked us into old, broken
protocols forever or (possibly worse) forced us to write our own async
framework building on the old did-you-mean-continuations in Worker. if
we had done that we'd be locked into ever more, and ever more complex,
manual state management all over the place. this just could not stand.
Change-Id: I43a6de1035febff59d2eff83be9ad52af4659871
eldritch horrors
alois31
Lily Ballard
piegames
jade
Maximilian Bosch
Lulu