aboutsummaryrefslogtreecommitdiff
path: root/src/libstore
AgeCommit message (Collapse)Author
2024-08-03fix: warn and document when advanced attributes will have no impact due to ↵Tom Bereknyei
__structuredAttrs Backport of https://github.com/NixOS/nix/pull/10884. Change-Id: I82cc2794730ae9f4a9b7df0185ed0aea83efb65a
2024-08-03libstore: move Goal::waiteeDone into Worker::goalFinishedeldritch horrors
this begins a long and arduous journey to remove all result state from Goal, to eventually drop the std::enable_shared_from_this base, and to completely eliminate all unsynchronized modification of states of both Goal and Worker. by the end of this we will hopefully be able to start and reap multiple derivation builds in parallel, which should speed up the process quite a bit (at least for short local builds, others might not notice a large difference. the build hooks will remain a problem.) Change-Id: I57dcd9b2cab4636ed4aa24cdec67124fef883345
2024-08-02Merge "libstore/ssh: only resume the logger when we paused it" into mainalois31
2024-08-02libstore/ssh: only resume the logger when we paused itAlois Wohlschlager
In the SSH code, the logger was conditionally paused, but unconditionally resumed. This was fine as long as resuming the logger was idempotent. Starting with 0dd1d8ca1cdccfc620644a7f690ed35bcd2d1e74, it isn't any more, and the behaviour of the code in question was missed. Consequently, an assertion failure is triggered for example when performing builds against an "SSH" store on localhost. Fix the issue by only resuming the logger when it has actually been paused. Fixes: https://git.lix.systems/lix-project/lix/issues/458 Change-Id: Ib1e4d047744a129f15730b7216f9c9368c2f4211
2024-08-02libstore: move Goal::amDone to Workereldritch horrors
we still mutate goal state to store the results of any given goal run, but now we also have that information in Worker and could in theory do something else with it. we could return a map of goal to goal results, which would also let us better diagnose failures of subgoals (at all). Change-Id: I1df956bbd9fa8cc9485fb6df32918d68dda3ff48
2024-08-02libstore: return finishedness from Goal methodseldritch horrors
this is the first step towards removing all result-related mutation of Goal state from goal implementations themselves, and into Worker state instead. once that is done we can treat all non-const Goal fields like private state of the goal itself, and make threading of goals possible Change-Id: I69ff7d02a6fd91a65887c6640bfc4f5fb785b45c
2024-08-02libstore: encapsulate worker build hook stateeldritch horrors
once goals run on multiple threads these fields must by synchronized as one, or we try to run build hooks to often (or worse, not often enough) Change-Id: I47860e46fe5c6db41755b2a3a1d9dbb5701c4ca4
2024-07-30libstore: move Goal::getBuildResult to BuildResulteldritch horrors
there are no other uses for this yet, but asking for just a subset of outputs does seem at least somewhat useful to have as a generic thing Change-Id: I30ff5055a666c351b1b086b8d05b9d7c9fb1c77a
2024-07-30libstore: count all substitutions toward the same limiteldritch horrors
limiting CA substitutions was a rather recent addition, and it used a dedicated counter to not interfere with regular substitutions. though this works fine it somewhat contradicts the documentation; job limits should apply to all kinds of substitutions, or be one limit for each. Change-Id: I1505105b14260ecc1784039b2cc4b7afcf9115c8
2024-07-30libstore: always wake up goals on EOFeldritch horrors
all goals do this. it makes no sense to not notify a goal of EOF conditions because this is the universal signal for "child done" Change-Id: Ic3980de312547e616739c57c6248a8e81308b5ee
2024-07-30libstore: simplify substitution handleEOFeldritch horrors
both substitution goals add only this single fd to their wait set. Change-Id: Ibf921f5bb3919106208a0871523b32c8f67fb3d3
2024-07-29libstore: remove Worker::updateProgresseldritch horrors
just update progress every time a goal has returned from work(). there seem to be no performance penalties, and the code is much simpler now. Change-Id: I288ee568b764ee61f40a498d986afda49987cb50
2024-07-27libutil: Add bindPath function from libstoreArtemis Tosini
bindPath/doBind is a useful function in build that is used in several parts of LocalDerivationGoal. Moving this function makes it easier to split LocalDerivationGoal implementation between several files. Change-Id: Ic5a0768479c153c1aa3ed425f12604b20bbf0f42
2024-07-26Merge changes I45d3895f,I541be3ea,Ibe51416d into mainalois31
* changes: libstore/build: block io_uring libstore/build: use an allowlist approach to syscall filtering libstore/build: always treat seccomp setup failures as fatal
2024-07-25Merge changes Ic0dfcfe2,Ibe73851f,Ia7a8df1c,I400b2031 into mainjade
* changes: package.nix: remove dead code diff-closures: remove gratuitous copy tree-wide: NULL -> nullptr libutil: rip out GNU Hurd support code
2024-07-25libstore/build: block io_uringAlois Wohlschlager
Unfortunately, io_uring is totally opaque to seccomp, and while currently there are no dangerous operations implemented, there is no guarantee that it remains this way. This means that io_uring should be blocked entirely to ensure that the sandbox is future-proof. This has not been observed to cause issues in practice. Change-Id: I45d3895f95abe1bc103a63969f444c334dbbf50d
2024-07-25libstore/build: use an allowlist approach to syscall filteringAlois Wohlschlager
Previously, system call filtering (to prevent builders from storing files with setuid/setgid permission bits or extended attributes) was performed using a blocklist. While this looks simple at first, it actually carries significant security and maintainability risks: after all, the kernel may add new syscalls to achieve the same functionality one is trying to block, and it can even be hard to actually add the syscall to the blocklist when building against a C library that doesn't know about it yet. For a recent demonstration of this happening in practice to Nix, see the introduction of fchmodat2 [0] [1]. The allowlist approach does not share the same drawback. While it does require a rather large list of harmless syscalls to be maintained in the codebase, failing to update this list (and roll out the update to all users) in time has rather benign effects; at worst, very recent programs that already rely on new syscalls will fail with an error the same way they would on a slightly older kernel that doesn't support them yet. Most importantly, no unintended new ways of performing dangerous operations will be silently allowed. Another possible drawback is reduced system call performance due to the larger filter created by the allowlist requiring more computation [2]. However, this issue has not convincingly been demonstrated yet in practice, for example in systemd or various browsers. To the contrary, it has been measured that the the actual filter constructed here has approximately the same overhead as a very simple filter blocking only one system call. This commit tries to keep the behavior as close to unchanged as possible. The system call list is in line with libseccomp 2.5.5 and glibc 2.39, which are the latest versions at the point of writing. Since libseccomp 2.5.5 is already a requirement and the distributions shipping this together with older versions of glibc are mostly not a thing any more, this should not lead to more build failures any more. [0] https://github.com/NixOS/nixpkgs/issues/300635 [1] https://github.com/NixOS/nix/issues/10424 [2] https://github.com/flatpak/flatpak/pull/4462#issuecomment-1061690607 Change-Id: I541be3ea9b249bcceddfed6a5a13ac10b11e16ad
2024-07-25libstore/build: always treat seccomp setup failures as fatalAlois Wohlschlager
In f047e4357b4f7ad66c2e476506bf35cab82e441e, I missed the behavior that if building without a dedicated build user (i.e. in single-user setups), seccomp setup failures are silently ignored. This was introduced without explanation 7 years ago (ff6becafa8efc2f7e6f2b9b889ba4adf20b8d524). Hopefully the only use-case nowadays is causing spurious test suite successes when messing up the seccomp filter during development. Let's try removing it. Change-Id: Ibe51416d9c7a6dd635c2282990224861adf1ceab
2024-07-24fix building with Musl, fixing static buildsQyriad
Musl stdout macro expands¹ to something that isn't a valid identifier, so we get syntax errors when compiling usage of a method called stdout with Musl's stdio.h. [1]: https://git.musl-libc.org/cgit/musl/tree/include/stdio.h?id=ab31e9d6a0fa7c5c408856c89df2dfb12c344039#n67 Change-Id: I10e6f6a49504399bf8edd59c5d9e4e62449469e8
2024-07-23tree-wide: NULL -> nullptrJade Lovelace
This is slightly more type safe and is more in line with modern C++. Change-Id: Ia7a8df1c7788085020d1bdc941d6f9cee356144e
2024-07-23libstore: Add FreeBSD findPlatformRootsArtemis Tosini
Use libprocstat to find garbage collector roots on FreeBSD. Tested working on a FreeBSD machine, although there is no CI yet Change-Id: Id36bac8c3de6cc4de94e2d76e9663dd4b76068a9
2024-07-22libstore: keep Goal errors as unique_ptrseldritch horrors
Error is pretty large, and most goals do not fail. this alone more than halves the size of Goal on x86_64-linux, from 720 bytes down to 344. in derived classes the difference is not as dramatic, but even the largest derived class (`LocalDerivationGoal`) loses almost 20% of its footprint Change-Id: Ifda8f94c81b6566eeb3e52d55d9796ec40c7bce8
2024-07-22libstore: remove an always-defaulted argumenteldritch horrors
Change-Id: I3c7f17d5492a16bb54480fa1aa384b96fba72d61
2024-07-22libstore: use std::async instead of Goal threadseldritch horrors
the goals are either already using std::async and merely forgot to remove std::thread vestiges or they emulate async with threads and promises. we can simply use async directly everywhere for clarity. Change-Id: I3f05098310a25984f10fff1e68c573329002b500
2024-07-22libstore: remove addToWeakGoalseldritch horrors
under owner_less it's equivalent to insert(), only sometimes a little bit faster because it does not construct a weak_ptr if the goal is in the set already. this small difference in performance does not matter here and c++23 will make insert transparent anyway, so we can drop it Change-Id: I7cbd7d6e0daa95d67145ec58183162f6c4743b15
2024-07-22libstore: remove Goal::ecBusyeldritch horrors
this should be an optional. "busy" is not an *exit* code! Change-Id: Ic231cb27b022312b1a7a7b9602f32845b7a9c934
2024-07-22libstore: remove unused Worker::waitForAnyGoaleldritch horrors
Change-Id: Ia3ebd434b17052b6760ce74d8e20025a72148613
2024-07-22enable -Werror=suggest-overrideeldritch horrors
*accidentally* overriding a function is almost guaranteed to be an error. overriding a function without labeling it as such is merely bad style, but bad style that makes the code harder to understand. Change-Id: Ic0594f3d1604ab6b3c1a75cb5facc246effe45f0
2024-07-21Merge "libstore/binary-cache-store: use correct buffer size for NAR ↵alois31
decompression" into main
2024-07-21Merge "gc: refactor the gc server thread out into a class without changing ↵jade
it" into main
2024-07-21libstore/binary-cache-store: use correct buffer size for NAR decompressionAlois Wohlschlager
Due to a leftover from a previous version where the buffer was allocated on the stack, the change introduced in commit 4ec87742a196d8ed8f41b41ef039706ce791448d accidentally passes the size of a pointer as the size of the buffer to the decompressor. Since the former is much smaller (usually 8 bytes instead of 64 kilobytes), this is safe, but leads to considerable overhead; most notably, due to excessive progress reports, which happen for each chunk. Pass the proper buffer size instead. Change-Id: If4bf472d33e21587acb5235a2d99e3cb10914633
2024-07-20Merge "Fix namespace warning being emitted if sandbox is disabled" into mainWinter Cute
2024-07-20libutil: make basic loggers thread-safeeldritch horrors
SimpleLogger is not fully thread-safe, and all loggers that wrap it are also not safe accordingly. this does not affect much, but in rare cases it can cause interleaving of messages on stderr when used with the json or raw log formats. the fix applied here is a bit of a hack, but fixing this properly requires rearchitecting the logger infrastructure. nested loggers are not the most natural abstraction here, and it is biting us. Change-Id: Ifbf34fe1e85c60e73b59faee50e7411c7b5e7c12
2024-07-19Fix namespace warning being emitted if sandbox is disabledWinter
If useChroot = false, and user namespaces aren't available for some reason (e.g. within a Docker container), this fixes a pointless warning being emitted, as we would never attempt to use them even if they were available. Change-Id: Ibcee91c088edd2cd19e70218d5a5802bff8f537b
2024-07-19gc: refactor the gc server thread out into a class without changing itJade Lovelace
This removes a *whole load* of variables from scope and enforces thread boundaries with the type system. There is not much change of significance in here, so the things to watch out for while reviewing it are primarily that the destructor ordering may have changed inadvertently, I think. Change-Id: I3cd87e6d5a08dfcf368637407251db22a8906316
2024-07-18Fixup a bunch of references to nixos.org manualsJade Lovelace
(plus one reference to CppNix github) Change-Id: Id8b3d2897f3b54e286861805cfd421adc4d5de47
2024-07-16libstore: remove remaining sinkToSource useseldritch horrors
Change-Id: Id1ee0d2ad4a3774f4bbb960d76f0f76ac4f3eff9
2024-07-16libstore: remove WriteConn::sink fieldseldritch horrors
we no longer need these since we're no longer using sinks to serialize things. Change-Id: Iffb1a3eab33c83f611c88fa4e8beaa8d5ffa079b
2024-07-16libstore: generatorize protocol serializerseldritch horrors
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
2024-07-16libstore: remove a sinkToSouce from old daemon protocoleldritch horrors
this doesn't have a test because this code path is only reached by clients that predate 2.4, and we really should not be caring about those any more right now. even the test suite doesn't, and the few tests that might care are disabled because they will not even work Change-Id: Id9eb190065138fedb2c7d90c328ff9eb9d97385b
2024-07-15libstore: rewrite narFromPath as generatoreldritch horrors
Change-Id: Ifa783c2c65c06ddd1d0212016d5bfd07666ea91c
2024-07-13language: cleanly ban integer overflowsJade Lovelace
This also bans various sneaking of negative numbers from the language into unsuspecting builtins as was exposed while auditing the consequences of changing the Nix language integer type to a newtype. It's unlikely that this change comprehensively ensures correctness when passing integers out of the Nix language and we should probably add a checked-narrowing function or something similar, but that's out of scope for the immediate change. During the development of this I found a few fun facts about the language: - You could overflow integers by converting from unsigned JSON values. - You could overflow unsigned integers by converting negative numbers into them when going into Nix config, into fetchTree, and into flake inputs. The flake inputs and Nix config cannot actually be tested properly since they both ban thunks, however, we put in checks anyway because it's possible these could somehow be used to do such shenanigans some other way. Note that Lix has banned Nix language integer overflows since the very first public beta, but threw a SIGILL about them because we run with -fsanitize=signed-overflow -fsanitize-undefined-trap-on-error in production builds. Since the Nix language uses signed integers, overflow was simply undefined behaviour, and since we defined that to trap, it did. Trapping on it was a bad UX, but we didn't even entirely notice that we had done this at all until it was reported as a bug a couple of months later (which is, to be fair, that flag working as intended), and it's got enough production time that, aside from code that is IMHO buggy (and which is, in any case, not in nixpkgs) such as https://git.lix.systems/lix-project/lix/issues/445, we don't think anyone doing anything reasonable actually depends on wrapping overflow. Even for weird use cases such as doing funny bit crimes, it doesn't make sense IMO to have wrapping behaviour, since two's complement arithmetic overflow behaviour is so *aggressively* not what you want for *any* kind of mathematics/algorithms. The Nix language exists for package management, a domain where bit crimes are already only dubiously in scope to begin with, and it makes a lot more sense for that domain for the integers to never lose precision, either by throwing errors if they would, or by being arbitrary-precision. This change will be ported to CppNix as well, to maintain language consistency. Fixes: https://git.lix.systems/lix-project/lix/issues/423 Change-Id: I51f253840c4af2ea5422b8a420aa5fafbf8fae75
2024-07-12Use std::strong_ordering for version comparisonJade Lovelace
The actual motive here is the avoidance of integer overflow if we were to make these use checked NixInts and retain the subtraction. However, the actual *intent* of this code is a three-way comparison, which can be done with operator<=>, so we should just do *that* instead. Change-Id: I7f9a7da1f3176424b528af6d1b4f1591e4ab26bf
2024-07-11libstore: remove upcast_goaleldritch horrors
upcast_goal was only ever needed to break circular includes, but the same solution that gave us upcast_goal also lets us fully remove it: just upcast goals without a wrapper function, but only in .cc files. Change-Id: I9c71654b2535121459ba7dcfd6c5da5606904032
2024-07-11libstore: turn copyNAR into a generatoreldritch horrors
Change-Id: Id452f6a03faa1037ff13af0f63e32883966ff40d
2024-07-11libutil: remove makeDecompressionSinkeldritch horrors
the sole remaining user of this function can use makeDecompressionSource instead, while making the sinkToSource in the caller unnecessary as well Change-Id: I4258227b5dbbb735a75b477d8a57007bfca305e9
2024-07-11libstore: make BinaryCacheStore::getFile return a sourceeldritch horrors
this lets us remove the last true remaining uses of makeDecompressionSink. Change-Id: I146ca2bbe1a9ae9a367117a7b8a304b23a63e5e2
2024-07-11libutil: rewrite RewritingSink as sourceeldritch horrors
the rewriting sink was just broken. when given a rewrite set that contained a key that is also a proper infix of another key it was possible to produce an incorrectly rewritten result if the writer used the wrong block size. fixing this duplicates rewriteStrings, to avoid this we'll rewrite rewriteStrings to use RewritingSource in a new mode that'll allow rewrites we had previously forbidden. Change-Id: I57fa0a9a994e654e11d07172b8e31d15f0b7e8c0
2024-07-07libstore: make LocalDerivationGoal::needsHashRewrite virtualArtemis Tosini
This rather simple function existed just to check some flags, but the response varies by platform. This is a perfect case for our subclasses. Change-Id: Ieb1732a8d024019236e0d0028ad843a24ec3dc59
2024-07-06libutil: turn HashModuloSink into a free functioneldritch horrors
Change-Id: I5878007502fa68c2816a0f4c61f7d0e60bdde702