diff options
| -rw-r--r-- | CONTRIBUTING.md | 89 | ||||
| -rw-r--r-- | README.md | 2 | ||||
| -rw-r--r-- | package.nix | 2 | ||||
| -rw-r--r-- | src/libstore/build/drv-output-substitution-goal.cc | 14 | ||||
| -rw-r--r-- | src/libstore/build/drv-output-substitution-goal.hh | 7 | ||||
| -rw-r--r-- | src/libstore/build/worker.hh | 1 | ||||
| -rw-r--r-- | src/libstore/filetransfer.cc | 28 | ||||
| -rw-r--r-- | src/libstore/misc.cc | 32 | ||||
| -rw-r--r-- | src/libstore/realisation.cc | 15 | ||||
| -rw-r--r-- | src/libstore/store-api.cc | 7 | ||||
| -rw-r--r-- | src/libutil/closure.hh | 68 | ||||
| -rw-r--r-- | src/libutil/serialise.cc | 2 | ||||
| -rw-r--r-- | tests/unit/libutil/closure.cc | 35 |
13 files changed, 102 insertions, 200 deletions
diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 8908b9887..4a4c81cb2 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -1,76 +1,53 @@ -# Contributing to Nix +# Contributing to Lix -Welcome and thank you for your interest in contributing to Nix! -We appreciate your support. +Welcome and thank you for considering contributing to Lix! We're currently in a soft release phase, and your support means a lot to us. -Reading and following these guidelines will help us make the contribution process easy and effective for everyone involved. +To ensure a smooth and effective contribution process, here is a summary of our guidelines: -## Report a bug - -1. Check on the [GitHub issue tracker](https://github.com/NixOS/nix/issues) if your bug was already reported. - -2. If you were not able to find the bug or feature [open a new issue](https://github.com/NixOS/nix/issues/new/choose) +## Getting help? -3. The issue templates will guide you in specifying your issue. - The more complete the information you provide, the more likely it can be found by others and the more useful it is in the future. - Make sure reported bugs can be reproduced easily. +If you have any question regarding getting started or reporting bugs, feel free +to reach out to us. -4. Once submitted, do not expect issues to be picked up or solved right away. - The only way to ensure this, is to [work on the issue yourself](#making-changes-to-nix). - -## Report a security vulnerability +On Matrix, we have a space at `#space:lix.systems`, composed of: -Check out the [security policy](https://github.com/NixOS/nix/security/policy). +- [`#discuss:lix.systems`](https://matrix.to/#/#discuss:lix.systems) for discussions on Lix. +- [`#dev:lix.systems`](https://matrix.to/#/#dev:lix.systems) for the development channel on Lix. -## Making changes to Nix - -1. Check for [pull requests](https://github.com/NixOS/nix/pulls) that might already cover the contribution you are about to make. - There are many open pull requests that might already do what you intent to work on. - You can use [labels](https://github.com/NixOS/nix/labels) to filter for relevant topics. - -2. Search for related issues that cover what you're going to work on. It could help to mention there that you will work on the issue. +## Report a bug - Issues labeled [good first issue](https://github.com/NixOS/nix/labels/good-first-issue) should be relatively easy to fix and are likely to get merged quickly. - Pull requests addressing issues labeled [idea approved](https://github.com/NixOS/nix/labels/idea%20approved) are especially welcomed by maintainers and will receive prioritised review. +- Check if your bug has already been reported in the [issue tracker](https://git.lix.systems/lix-project/lix/issues). +- If you can't find the bug or feature, please open a new issue. -3. Check the [Nix reference manual](https://nixos.org/manual/nix/unstable/contributing/hacking.html) for information on building Nix and running its tests. +We maintain a copy of the upstream Nix bugs. Their organisation can be read about [here](https://wiki.lix.systems/books/lix-contributors/page/bug-tracker-organisation). - For contributions to the command line interface, please check the [CLI guidelines](https://nixos.org/manual/nix/unstable/contributing/cli-guideline.html). +## Report a security vulnerability -4. Make your changes! +For security vulnerabilities, reach out by email at `security at lix dot systems`. -5. [Create a pull request](https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/creating-a-pull-request) for your changes. - * Link related issues in your pull request to inform interested parties and future contributors about your change. - * Make sure to have [a clean history of commits on your branch by using rebase](https://www.digitalocean.com/community/tutorials/how-to-rebase-and-update-a-pull-request). - If your pull request closes one or multiple issues, note that in the description using `Closes: #<number>`, as it will then happen automatically when your change is merged. - * [Mark the pull request as draft](https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/changing-the-stage-of-a-pull-request) if you're not done with the changes. +## Making changes to Lix -6. Do not expect your pull request to be reviewed immediately. - Nix maintainers follow a [structured process for reviews and design decisions](https://github.com/NixOS/nix/tree/master/maintainers#project-board-protocol), which may or may not prioritise your work. +Before diving into making changes, we want to engage with you and your ideas. - Following this checklist will make the process smoother for everyone: +We have a few policies in effect; please take the time to familiarize yourself: - - [ ] Fixes an [idea approved](https://github.com/NixOS/nix/labels/idea%20approved) issue - - [ ] Tests, as appropriate: - - Functional tests – [`tests/functional/**.sh`](./tests/functional) - - Unit tests – [`src/*/tests`](./src/) - - Integration tests – [`tests/nixos/*`](./tests/nixos) - - [ ] User documentation in the [manual](..doc/manual/src) - - [ ] API documentation in header files - - [ ] Code and comments are self-explanatory - - [ ] Commit message explains **why** the change was made - - [ ] New feature or incompatible change: updated [release notes](./doc/manual/src/release-notes/rl-next.md) +- [Style guide on code](https://wiki.lix.systems/books/lix-contributors/page/code) +- [Freeze policy and recommended contributions](https://wiki.lix.systems/books/lix-contributors/page/freezes-and-recommended-contributions) -7. If you need additional feedback or help to getting pull request into shape, ask other contributors using [@mentions](https://docs.github.com/en/get-started/writing-on-github/getting-started-with-writing-and-formatting-on-github/basic-writing-and-formatting-syntax#mentioning-people-and-teams). +To avoid duplication of effort, it may be a good idea to check out the list of +[pending pull requests](https://gerrit.lix.systems/q/status:open+-is:wip) (or "change lists", as Gerrit calls them). Once you have +an idea of what you might want to do, we recommend dropping a message on our +Matrix to ensure your contribution fits with our current schedule and plans -## Making changes to the Nix manual +When you're ready and your changes are ready to go: -The Nix reference manual is hosted on https://nixos.org/manual/nix. -The underlying source files are located in [`doc/manual/src`](./doc/manual/src). -For small changes you can [use GitHub to edit these files](https://docs.github.com/en/repositories/working-with-files/managing-files/editing-files) -For larger changes see the [Nix reference manual](https://nixos.org/manual/nix/unstable/contributing/hacking.html). +- Submit your code. + - Submitting a GitHub PR [on our mirror](https://github.com/lix-project/lix) is totally ok if that's easier for you and your change is relatively small (300 lines or so). -## Getting help + We may ask you to resubmit it as a Gerrit CL if it is necessary for the change you're making. + - Our primary code review system is [our Gerrit instance](https://gerrit.lix.systems), where you can open a change list (CL). + If you're new to Gerrit, check out [our wiki page about Gerrit](https://wiki.lix.systems/books/lix-contributors/page/gerrit). +- Make sure to link any related issues. +- If needed, indicate that the change is 'work in progress'. -Whenever you're stuck or do not know how to proceed, you can always ask for help. -The appropriate channels to do so can be found on the [NixOS Community](https://nixos.org/community/) page. +You can obtain an account on our platforms by clicking "Sign In with GitHub" on the sign-in page. @@ -22,7 +22,7 @@ See our [Hacking guide](https://git.lix.systems/lix-project/lix/src/branch/main/ ## Additional Resources - [Our wiki](https://wiki.lix.systems) -- [Matrix - #nix:nixos.org](https://matrix.to/#/#space:lix.systems) +- [Matrix - #space:lix.systems](https://matrix.to/#/#space:lix.systems) ## License diff --git a/package.nix b/package.nix index af0a44fee..4abce3aad 100644 --- a/package.nix +++ b/package.nix @@ -155,8 +155,6 @@ let ./.version ./configure.ac ./m4 - # TODO: do we really need README.md? It doesn't seem used in the build. - ./README.md ]; topLevelBuildFiles = fileset.unions ( diff --git a/src/libstore/build/drv-output-substitution-goal.cc b/src/libstore/build/drv-output-substitution-goal.cc index b30957c84..0e85650a7 100644 --- a/src/libstore/build/drv-output-substitution-goal.cc +++ b/src/libstore/build/drv-output-substitution-goal.cc @@ -3,6 +3,7 @@ #include "worker.hh" #include "substitution-goal.hh" #include "callback.hh" +#include "signals.hh" namespace nix { @@ -38,6 +39,18 @@ void DrvOutputSubstitutionGoal::tryNext() { trace("trying next substituter"); + /* Make sure that we are allowed to start a substitution. Note that even + if maxSubstitutionJobs == 0, we still allow a substituter to run. This + prevents infinite waiting. */ + if (worker.runningCASubstitutions >= std::max(1U, settings.maxSubstitutionJobs.get())) { + worker.waitForBuildSlot(shared_from_this()); + return; + } + + maintainRunningSubstitutions = + std::make_unique<MaintainCount<uint64_t>>(worker.runningCASubstitutions); + worker.updateProgress(); + if (subs.size() == 0) { /* None left. Terminate this goal and let someone else deal with it. */ @@ -87,6 +100,7 @@ void DrvOutputSubstitutionGoal::tryNext() void DrvOutputSubstitutionGoal::realisationFetched() { worker.childTerminated(this); + maintainRunningSubstitutions.reset(); try { outputInfo = downloadState->promise.get_future().get(); diff --git a/src/libstore/build/drv-output-substitution-goal.hh b/src/libstore/build/drv-output-substitution-goal.hh index da2426e5e..ab6fb796e 100644 --- a/src/libstore/build/drv-output-substitution-goal.hh +++ b/src/libstore/build/drv-output-substitution-goal.hh @@ -41,6 +41,13 @@ class DrvOutputSubstitutionGoal : public Goal { */ std::shared_ptr<Store> sub; + /** + * The substituter thread. + */ + std::thread thr; + + std::unique_ptr<MaintainCount<uint64_t>> maintainRunningSubstitutions; + struct DownloadState { Pipe outPipe; diff --git a/src/libstore/build/worker.hh b/src/libstore/build/worker.hh index 23ad87914..ba4cd88d7 100644 --- a/src/libstore/build/worker.hh +++ b/src/libstore/build/worker.hh @@ -166,6 +166,7 @@ public: uint64_t doneSubstitutions = 0; uint64_t failedSubstitutions = 0; uint64_t runningSubstitutions = 0; + uint64_t runningCASubstitutions = 0; uint64_t expectedDownloadSize = 0; uint64_t doneDownloadSize = 0; uint64_t expectedNarSize = 0; diff --git a/src/libstore/filetransfer.cc b/src/libstore/filetransfer.cc index aa8f4be1d..67b9fef81 100644 --- a/src/libstore/filetransfer.cc +++ b/src/libstore/filetransfer.cc @@ -149,6 +149,8 @@ struct curlFileTransfer : public FileTransfer size_t writeCallback(void * contents, size_t size, size_t nmemb) { + const size_t realSize = size * nmemb; + try { if (!headersProcessed) { if (auto h = getHeader("content-encoding")) { @@ -161,7 +163,6 @@ struct curlFileTransfer : public FileTransfer headersProcessed = true; } - size_t realSize = size * nmemb; result.bodySize += realSize; if (successfulStatuses.count(getHTTPStatus()) && this->dataCallback) { @@ -174,7 +175,7 @@ struct curlFileTransfer : public FileTransfer return realSize; } catch (...) { writeException = std::current_exception(); - return 0; + return CURL_WRITEFUNC_ERROR; } } @@ -710,9 +711,8 @@ struct curlFileTransfer : public FileTransfer struct State { bool done = false, failed = false; std::exception_ptr exc; - std::string data; + std::string data, encoding; std::condition_variable avail, request; - std::unique_ptr<FinishSink> decompressor; }; auto _state = std::make_shared<Sync<State>>(); @@ -736,7 +736,7 @@ struct curlFileTransfer : public FileTransfer state->avail.notify_one(); state->request.notify_one(); }}, - [_state, &sink](TransferItem & transfer, std::string_view data) { + [_state](TransferItem & transfer, std::string_view data) { auto state(_state->lock()); if (state->failed) { @@ -744,10 +744,6 @@ struct curlFileTransfer : public FileTransfer throw std::exception{}; } - if (!state->decompressor) { - state->decompressor = makeDecompressionSink(transfer.encoding, sink); - } - /* If the buffer is full, then go to sleep until the calling thread wakes us up (i.e. when it has removed data from the buffer). We don't wait forever to prevent stalling the @@ -764,11 +760,12 @@ struct curlFileTransfer : public FileTransfer state->avail.notify_one(); }); + std::unique_ptr<FinishSink> decompressor; + while (true) { checkInterrupt(); std::string chunk; - FinishSink * sink = nullptr; /* Grab data if available, otherwise wait for the download thread to wake us up. */ @@ -779,8 +776,8 @@ struct curlFileTransfer : public FileTransfer if (state->done) { if (state->exc) std::rethrow_exception(state->exc); - if (state->decompressor) { - state->decompressor->finish(); + if (decompressor) { + decompressor->finish(); } return; } @@ -791,10 +788,13 @@ struct curlFileTransfer : public FileTransfer } chunk = std::move(state->data); - sink = state->decompressor.get(); /* Reset state->data after the move, since we check data.empty() */ state->data = ""; + if (!decompressor) { + decompressor = makeDecompressionSink(state->encoding, sink); + } + state->request.notify_one(); } @@ -802,7 +802,7 @@ struct curlFileTransfer : public FileTransfer if it's blocked on a full buffer. We don't hold the state lock while doing this to prevent blocking the download thread if sink() takes a long time. */ - (*sink)(chunk); + (*decompressor)(chunk); } } }; diff --git a/src/libstore/misc.cc b/src/libstore/misc.cc index c8646083b..22f6b67ee 100644 --- a/src/libstore/misc.cc +++ b/src/libstore/misc.cc @@ -14,10 +14,9 @@ namespace nix { void Store::computeFSClosure(const StorePathSet & startPaths, StorePathSet & paths_, bool flipDirection, bool includeOutputs, bool includeDerivers) { - std::function<std::set<StorePath>(const StorePath & path, std::future<ref<const ValidPathInfo>> &)> queryDeps; + std::function<std::set<StorePath>(const StorePath & path, ref<const ValidPathInfo>)> queryDeps; if (flipDirection) - queryDeps = [&](const StorePath& path, - std::future<ref<const ValidPathInfo>> & fut) { + queryDeps = [&](const StorePath& path, ref<const ValidPathInfo>) { StorePathSet res; StorePathSet referrers; queryReferrers(path, referrers); @@ -36,10 +35,8 @@ void Store::computeFSClosure(const StorePathSet & startPaths, return res; }; else - queryDeps = [&](const StorePath& path, - std::future<ref<const ValidPathInfo>> & fut) { + queryDeps = [&](const StorePath& path, ref<const ValidPathInfo> info) { StorePathSet res; - auto info = fut.get(); for (auto& ref : info->references) if (ref != path) res.insert(ref); @@ -54,24 +51,11 @@ void Store::computeFSClosure(const StorePathSet & startPaths, return res; }; - computeClosure<StorePath>( - startPaths, paths_, - [&](const StorePath& path, - std::function<void(std::promise<std::set<StorePath>>&)> - processEdges) { - std::promise<std::set<StorePath>> promise; - std::function<void(std::future<ref<const ValidPathInfo>>)> - getDependencies = - [&](std::future<ref<const ValidPathInfo>> fut) { - try { - promise.set_value(queryDeps(path, fut)); - } catch (...) { - promise.set_exception(std::current_exception()); - } - }; - queryPathInfo(path, getDependencies); - processEdges(promise); - }); + paths_.merge(computeClosure<StorePath>( + startPaths, + [&](const StorePath& path) -> std::set<StorePath> { + return queryDeps(path, queryPathInfo(path)); + })); } void Store::computeFSClosure(const StorePath & startPath, diff --git a/src/libstore/realisation.cc b/src/libstore/realisation.cc index 93ddb5b20..50c911da0 100644 --- a/src/libstore/realisation.cc +++ b/src/libstore/realisation.cc @@ -43,20 +43,7 @@ void Realisation::closure(Store & store, const std::set<Realisation> & startOutp return res; }; - computeClosure<Realisation>( - startOutputs, res, - [&](const Realisation& current, - std::function<void(std::promise<std::set<Realisation>>&)> - processEdges) { - std::promise<std::set<Realisation>> promise; - try { - auto res = getDeps(current); - promise.set_value(res); - } catch (...) { - promise.set_exception(std::current_exception()); - } - return processEdges(promise); - }); + res.merge(computeClosure<Realisation>(startOutputs, getDeps)); } nlohmann::json Realisation::toJSON() const { diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index 8c9940c86..f696e4c1f 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -1444,9 +1444,10 @@ std::shared_ptr<Store> openFromNonUri(const std::string & uri, const Store::Para warn("'%s' does not exist, so Nix will use '%s' as a chroot store", stateDir, chrootStore); } else debug("'%s' does not exist, so Nix will use '%s' as a chroot store", stateDir, chrootStore); - Store::Params params2; - params2["root"] = chrootStore; - return LocalStore::makeLocalStore(params); + Store::Params chrootStoreParams; + chrootStoreParams["root"] = chrootStore; + // FIXME? this ignores *all* store parameters passed to this function? + return LocalStore::makeLocalStore(chrootStoreParams); } #endif else diff --git a/src/libutil/closure.hh b/src/libutil/closure.hh index 16e3b93e4..146931fc8 100644 --- a/src/libutil/closure.hh +++ b/src/libutil/closure.hh @@ -1,72 +1,32 @@ #pragma once ///@file +#include <functional> #include <set> -#include <future> -#include "sync.hh" - -using std::set; namespace nix { template<typename T> -using GetEdgesAsync = std::function<void(const T &, std::function<void(std::promise<set<T>> &)>)>; - -template<typename T> -void computeClosure( - const set<T> startElts, - set<T> & res, - GetEdgesAsync<T> getEdgesAsync +std::set<T> computeClosure( + std::set<T> startElts, + std::function<std::set<T>(const T &)> getEdges ) { - struct State - { - size_t pending; - set<T> & res; - std::exception_ptr exc; - }; - - Sync<State> state_(State{0, res, 0}); + std::set<T> res, queue = std::move(startElts); - std::function<void(const T &)> enqueue; + while (!queue.empty()) { + std::set<T> next; - std::condition_variable done; - - enqueue = [&](const T & current) -> void { - { - auto state(state_.lock()); - if (state->exc) return; - if (!state->res.insert(current).second) return; - state->pending++; + for (auto & e : queue) { + if (res.insert(e).second) { + next.merge(getEdges(e)); + } } - getEdgesAsync(current, [&](std::promise<set<T>> & prom) { - try { - auto children = prom.get_future().get(); - for (auto & child : children) - enqueue(child); - { - auto state(state_.lock()); - assert(state->pending); - if (!--state->pending) done.notify_one(); - } - } catch (...) { - auto state(state_.lock()); - if (!state->exc) state->exc = std::current_exception(); - assert(state->pending); - if (!--state->pending) done.notify_one(); - }; - }); - }; - - for (auto & startElt : startElts) - enqueue(startElt); - - { - auto state(state_.lock()); - while (state->pending) state.wait(done); - if (state->exc) std::rethrow_exception(state->exc); + queue = std::move(next); } + + return res; } } diff --git a/src/libutil/serialise.cc b/src/libutil/serialise.cc index 692144b75..a6cc919d2 100644 --- a/src/libutil/serialise.cc +++ b/src/libutil/serialise.cc @@ -230,7 +230,7 @@ std::unique_ptr<FinishSink> sourceToSink(std::function<void(Source &)> fun) if (cur.empty()) { yield(); if (yield.get()) { - return (size_t)0; + throw EndOfFile("coroutine exhausted"); } } diff --git a/tests/unit/libutil/closure.cc b/tests/unit/libutil/closure.cc index 7597e7807..b4eaad6f9 100644 --- a/tests/unit/libutil/closure.cc +++ b/tests/unit/libutil/closure.cc @@ -16,15 +16,11 @@ map<string, set<string>> testGraph = { }; TEST(closure, correctClosure) { - set<string> aClosure; set<string> expectedClosure = {"A", "B", "C", "F", "G"}; - computeClosure<string>( + set<string> aClosure = computeClosure<string>( {"A"}, - aClosure, - [&](const string currentNode, function<void(promise<set<string>> &)> processEdges) { - promise<set<string>> promisedNodes; - promisedNodes.set_value(testGraph[currentNode]); - processEdges(promisedNodes); + [&](const string currentNode) { + return testGraph[currentNode]; } ); @@ -33,12 +29,10 @@ TEST(closure, correctClosure) { TEST(closure, properlyHandlesDirectExceptions) { struct TestExn {}; - set<string> aClosure; EXPECT_THROW( computeClosure<string>( {"A"}, - aClosure, - [&](const string currentNode, function<void(promise<set<string>> &)> processEdges) { + [&](const string currentNode) -> set<string> { throw TestExn(); } ), @@ -46,25 +40,4 @@ TEST(closure, properlyHandlesDirectExceptions) { ); } -TEST(closure, properlyHandlesExceptionsInPromise) { - struct TestExn {}; - set<string> aClosure; - EXPECT_THROW( - computeClosure<string>( - {"A"}, - aClosure, - [&](const string currentNode, function<void(promise<set<string>> &)> processEdges) { - promise<set<string>> promise; - try { - throw TestExn(); - } catch (...) { - promise.set_exception(std::current_exception()); - } - processEdges(promise); - } - ), - TestExn - ); -} - } |