From 7a3d5b2ff0e39ae0d7b393f454671a08da56776b Mon Sep 17 00:00:00 2001 From: Daniel Fullmer Date: Thu, 5 May 2022 10:46:48 -0700 Subject: [PATCH 01/17] Use correct URL for GitHub Enterprise For GitHub Enterprise, the API is accessed through a slightly different URL. See [1], where it says: > Use http(s)://[hostname]/api/v3 to access the API for GitHub > Enterprise Server. Also tested working on a GHE instance. [1] https://docs.github.com/en/enterprise-server@3.3/rest/guides/getting-started-with-the-rest-api --- src/libfetchers/github.cc | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/libfetchers/github.cc b/src/libfetchers/github.cc index 1bdf2759f..50b3150ee 100644 --- a/src/libfetchers/github.cc +++ b/src/libfetchers/github.cc @@ -243,7 +243,10 @@ struct GitHubInputScheme : GitArchiveInputScheme Hash getRevFromRef(nix::ref store, const Input & input) const override { auto host = maybeGetStrAttr(input.attrs, "host").value_or("github.com"); - auto url = fmt("https://api.%s/repos/%s/%s/commits/%s", // FIXME: check + auto url = fmt( + host == "github.com" + ? "https://api.%s/repos/%s/%s/commits/%s" + : "https://%s/api/v3/repos/%s/%s/commits/%s", host, getStrAttr(input.attrs, "owner"), getStrAttr(input.attrs, "repo"), *input.getRef()); Headers headers = makeHeadersWithAuthTokens(host); @@ -262,7 +265,10 @@ struct GitHubInputScheme : GitArchiveInputScheme // FIXME: use regular /archive URLs instead? api.github.com // might have stricter rate limits. auto host = maybeGetStrAttr(input.attrs, "host").value_or("github.com"); - auto url = fmt("https://api.%s/repos/%s/%s/tarball/%s", // FIXME: check if this is correct for self hosted instances + auto url = fmt( + host == "github.com" + ? "https://api.%s/repos/%s/%s/commits/%s" + : "https://%s/api/v3/repos/%s/%s/commits/%s", host, getStrAttr(input.attrs, "owner"), getStrAttr(input.attrs, "repo"), input.getRev()->to_string(Base16, false)); From 59d9551c25aa50cdfe65f6ce5cf9c07f9ff42736 Mon Sep 17 00:00:00 2001 From: Jan Tojnar Date: Sun, 8 May 2022 18:59:00 +0200 Subject: [PATCH 02/17] libexpr: Fix manual link in error message It was changed to the old manual in https://github.com/NixOS/nix/commit/8895fa70a4b05ddebbb5a23ea96464d5e01345fb --- src/libexpr/eval.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index d29ff5543..8d67691f0 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -1590,7 +1590,7 @@ void EvalState::autoCallFunction(Bindings & args, Value & fun, Value & res) Nix attempted to evaluate a function as a top level expression; in this case it must have its arguments supplied either by default values, or passed explicitly with '--arg' or '--argstr'. See -https://nixos.org/manual/nix/stable/#ss-functions.)", symbols[i.name]); +https://nixos.org/manual/nix/stable/expressions/language-constructs.html#functions.)", symbols[i.name]); } } From c060e93b3c48c5d6cfde88ee9080a10f73f00fe4 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 9 May 2022 22:01:15 +0000 Subject: [PATCH 03/17] Bump docker/login-action from 1 to 2 Bumps [docker/login-action](https://github.com/docker/login-action) from 1 to 2. - [Release notes](https://github.com/docker/login-action/releases) - [Commits](https://github.com/docker/login-action/compare/v1...v2) --- updated-dependencies: - dependency-name: docker/login-action dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index d01ef4768..aae5b93e0 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -100,7 +100,7 @@ jobs: - run: docker tag nix:$NIX_VERSION nixos/nix:$NIX_VERSION - run: docker tag nix:$NIX_VERSION nixos/nix:master - name: Login to Docker Hub - uses: docker/login-action@v1 + uses: docker/login-action@v2 with: username: ${{ secrets.DOCKERHUB_USERNAME }} password: ${{ secrets.DOCKERHUB_TOKEN }} From 7c75f1d52b3078608be29cbe0b009875829cc03f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Th=C3=A9ophane=20Hufschmitt?= Date: Tue, 10 May 2022 11:56:47 +0200 Subject: [PATCH 04/17] Expand the testing section of the hacking docs - Make it clear what the different kind of tests are, where they live and how they can be ran - Ask people to primarily write unit tests --- doc/manual/src/contributing/hacking.md | 41 ++++++++++++++++++-------- 1 file changed, 29 insertions(+), 12 deletions(-) diff --git a/doc/manual/src/contributing/hacking.md b/doc/manual/src/contributing/hacking.md index 90a8f1f94..7ce8d8de6 100644 --- a/doc/manual/src/contributing/hacking.md +++ b/doc/manual/src/contributing/hacking.md @@ -71,18 +71,6 @@ To install it in `$(pwd)/outputs` and test it: nix (Nix) 3.0 ``` -To run a functional test: - -```console -make tests/test-name-should-auto-complete.sh.test -``` - -To run the unit-tests for C++ code: - -``` -make check -``` - If you have a flakes-enabled Nix you can replace: ```console @@ -94,3 +82,32 @@ by: ```console $ nix develop ``` + +## Testing + +Nix comes with three different flavors of tests: unit, functional and integration. + +Most tests are currently written as functional tests. +**However**, it is preferable (as much as it makes sense) to primarily test new code with unit tests. + +### Unit-tests + +The unit-tests for each Nix library (`libexpr`, `libstore`, etc..) are defined +under `src/{library_name}/tests` using the +[googletest](https://google.github.io/googletest/) framework. + +You can run the whole testsuite with `make check`, or the tests for a specific component with `make libfoo-tests_RUN`. Finer-grained filtering is also possible using the [--gtest_filter](https://google.github.io/googletest/advanced.html#running-a-subset-of-the-tests) command-line option. + +### Functional tests + +The functional tests reside under the `tests` directory and are listed in `tests/local.mk`. +The whole testsuite can be run with `make install && make installcheck`. +Individual tests can be run with `make tests/{testName}.sh.test`. + +### Integration tests + +The integration tests are defined in the Nix flake under the `hydraJobs.tests` attribute. +These tests include everything that needs to interact with external services or run Nix in a non-trivial distributed setup. +Because these tests are expensive and require more than what the standard github-actions setup provides, they only run on the master branch (on ). + +You can run them manually with `nix build .#hydraJobs.tests.{testName}` or `nix-build -A hydraJobs.tests.{testName}` From a9cbc2857f4dd3af738b76edea00d692fbcee63c Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Tue, 10 May 2022 16:42:35 +0200 Subject: [PATCH 05/17] nix develop: Find bin/bash in the bashInteractive outputs --- src/nix/develop.cc | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/src/nix/develop.cc b/src/nix/develop.cc index 1190b8348..3a99fff6f 100644 --- a/src/nix/develop.cc +++ b/src/nix/develop.cc @@ -512,9 +512,20 @@ struct CmdDevelop : Common, MixEnvironment Strings{"legacyPackages." + settings.thisSystem.get() + "."}, nixpkgsLockFlags); - shell = store->printStorePath( - Installable::toStorePath(getEvalStore(), store, Realise::Outputs, OperateOn::Output, bashInstallable)) - + "/bin/bash"; + bool found = false; + + for (auto & path : Installable::toStorePaths(getEvalStore(), store, Realise::Outputs, OperateOn::Output, {bashInstallable})) { + auto s = store->printStorePath(path) + "/bin/bash"; + if (pathExists(s)) { + shell = s; + found = true; + break; + } + } + + if (!found) + throw Error("package 'nixpkgs#bashInteractive' does not provide a 'bin/bash'"); + } catch (Error &) { ignoreException(); } From 2998527b185a41157be6ead42fd03a66601c4f56 Mon Sep 17 00:00:00 2001 From: Jimmy Reichley Date: Tue, 10 May 2022 16:53:22 -0400 Subject: [PATCH 06/17] Allow setting bash-prompt-prefix nix develop configuration --- src/libexpr/flake/config.cc | 2 +- src/nix/develop.cc | 6 ++++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/src/libexpr/flake/config.cc b/src/libexpr/flake/config.cc index 92ec27046..3e9d264b4 100644 --- a/src/libexpr/flake/config.cc +++ b/src/libexpr/flake/config.cc @@ -31,7 +31,7 @@ static void writeTrustedList(const TrustedList & trustedList) void ConfigFile::apply() { - std::set whitelist{"bash-prompt", "bash-prompt-suffix", "flake-registry"}; + std::set whitelist{"bash-prompt", "bash-prompt-prefix", "bash-prompt-suffix", "flake-registry"}; for (auto & [name, value] : settings) { diff --git a/src/nix/develop.cc b/src/nix/develop.cc index 3a99fff6f..2a3fc0213 100644 --- a/src/nix/develop.cc +++ b/src/nix/develop.cc @@ -18,6 +18,9 @@ struct DevelopSettings : Config Setting bashPrompt{this, "", "bash-prompt", "The bash prompt (`PS1`) in `nix develop` shells."}; + Setting bashPromptPrefix{this, "", "bash-prompt-prefix", + "Prefix prepended to the `PS1` environment variable in `nix develop` shells."}; + Setting bashPromptSuffix{this, "", "bash-prompt-suffix", "Suffix appended to the `PS1` environment variable in `nix develop` shells."}; }; @@ -482,6 +485,9 @@ struct CmdDevelop : Common, MixEnvironment if (developSettings.bashPrompt != "") script += fmt("[ -n \"$PS1\" ] && PS1=%s;\n", shellEscape(developSettings.bashPrompt.get())); + if (developSettings.bashPromptPrefix != "") + script += fmt("[ -n \"$PS1\" ] && PS1=%s\"$PS1\";\n", + shellEscape(developSettings.bashPromptPrefix.get())); if (developSettings.bashPromptSuffix != "") script += fmt("[ -n \"$PS1\" ] && PS1+=%s;\n", shellEscape(developSettings.bashPromptSuffix.get())); From 584475acf9f4b8eda2a451901f6f9af35ae976e0 Mon Sep 17 00:00:00 2001 From: Jimmy Reichley Date: Tue, 10 May 2022 16:55:25 -0400 Subject: [PATCH 07/17] Add documentation for bash-prompt-prefix --- src/nix/develop.md | 4 ++-- src/nix/flake.md | 7 ++++--- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/src/nix/develop.md b/src/nix/develop.md index 8bcff66c9..e036ec6b9 100644 --- a/src/nix/develop.md +++ b/src/nix/develop.md @@ -80,8 +80,8 @@ initialised by `stdenv` and exits. This build environment can be recorded into a profile using `--profile`. The prompt used by the `bash` shell can be customised by setting the -`bash-prompt` and `bash-prompt-suffix` settings in `nix.conf` or in -the flake's `nixConfig` attribute. +`bash-prompt`, `bash-prompt-prefix`, and `bash-prompt-suffix` settings in +`nix.conf` or in the flake's `nixConfig` attribute. # Flake output attributes diff --git a/src/nix/flake.md b/src/nix/flake.md index c8251eb74..aa3f9f303 100644 --- a/src/nix/flake.md +++ b/src/nix/flake.md @@ -331,9 +331,10 @@ The following attributes are supported in `flake.nix`: * `nixConfig`: a set of `nix.conf` options to be set when evaluating any part of a flake. In the interests of security, only a small set of - whitelisted options (currently `bash-prompt`, `bash-prompt-suffix`, - and `flake-registry`) are allowed to be set without confirmation so long as - `accept-flake-config` is not set in the global configuration. + whitelisted options (currently `bash-prompt`, `bash-prompt-prefix`, + `bash-prompt-suffix`, and `flake-registry`) are allowed to be set without + confirmation so long as `accept-flake-config` is not set in the global + configuration. ## Flake inputs From 54457382f948bff30e2879a7d9047616e134ac5b Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Wed, 11 May 2022 11:36:56 +0200 Subject: [PATCH 08/17] Fix static build https://hydra.nixos.org/build/176211267 --- src/libexpr/tests/local.mk | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libexpr/tests/local.mk b/src/libexpr/tests/local.mk index cb1c4a977..b95980cab 100644 --- a/src/libexpr/tests/local.mk +++ b/src/libexpr/tests/local.mk @@ -10,6 +10,6 @@ libexpr-tests_SOURCES := $(wildcard $(d)/*.cc) libexpr-tests_CXXFLAGS += -I src/libexpr -I src/libutil -I src/libstore -I src/libexpr/tests -libexpr-tests_LIBS = libexpr libutil libstore +libexpr-tests_LIBS = libexpr libutil libstore libfetchers libexpr-tests_LDFLAGS := $(GTEST_LIBS) -lgmock From aefc6c4f41bfac0c76807c234fd0a786dd40f140 Mon Sep 17 00:00:00 2001 From: Eli Kogan-Wang Date: Wed, 11 May 2022 12:15:08 +0200 Subject: [PATCH 09/17] Add priority for nix profile install --- src/libstore/builtins/buildenv.cc | 3 ++- src/nix/profile.cc | 29 +++++++++++++++++++++++++++-- tests/nix-profile.sh | 16 ++++++++++++++++ 3 files changed, 45 insertions(+), 3 deletions(-) diff --git a/src/libstore/builtins/buildenv.cc b/src/libstore/builtins/buildenv.cc index 6f6ad57cb..c17c76e71 100644 --- a/src/libstore/builtins/buildenv.cc +++ b/src/libstore/builtins/buildenv.cc @@ -93,8 +93,9 @@ static void createLinks(State & state, const Path & srcDir, const Path & dstDir, auto prevPriority = state.priorities[dstFile]; if (prevPriority == priority) throw Error( - "packages '%1%' and '%2%' have the same priority %3%; " + "files '%1%' and '%2%' have the same priority %3%; " "use 'nix-env --set-flag priority NUMBER INSTALLED_PKGNAME' " + "or 'nix profile --priority NUMBER INSTALLED_PKGNAME' " "to change the priority of one of the conflicting packages" " (0 being the highest priority)", srcFile, readLink(dstFile), priority); diff --git a/src/nix/profile.cc b/src/nix/profile.cc index 685776bec..fb8bef670 100644 --- a/src/nix/profile.cc +++ b/src/nix/profile.cc @@ -37,7 +37,7 @@ struct ProfileElement StorePathSet storePaths; std::optional source; bool active = true; - // FIXME: priority + int priority = 5; std::string describe() const { @@ -116,6 +116,9 @@ struct ProfileManifest for (auto & p : e["storePaths"]) element.storePaths.insert(state.store->parseStorePath((std::string) p)); element.active = e["active"]; + if(e.contains("priority")) { + element.priority = e["priority"]; + } if (e.value(sUrl, "") != "") { element.source = ProfileElementSource { parseFlakeRef(e[sOriginalUrl]), @@ -153,6 +156,7 @@ struct ProfileManifest nlohmann::json obj; obj["storePaths"] = paths; obj["active"] = element.active; + obj["priority"] = element.priority; if (element.source) { obj["originalUrl"] = element.source->originalRef.to_string(); obj["url"] = element.source->resolvedRef.to_string(); @@ -177,7 +181,7 @@ struct ProfileManifest for (auto & element : elements) { for (auto & path : element.storePaths) { if (element.active) - pkgs.emplace_back(store->printStorePath(path), true, 5); + pkgs.emplace_back(store->printStorePath(path), true, element.priority); references.insert(path); } } @@ -259,6 +263,23 @@ builtPathsPerInstallable( struct CmdProfileInstall : InstallablesCommand, MixDefaultProfile { + std::optional priority; + CmdProfileInstall() { + addFlag({ + .longName = "priority", + .description = "The priority of the package to install.", + .labels = {"priority"}, + .handler = {[&](std::string s) { + try{ + priority = std::stoi(s); + } catch (std::invalid_argument & e) { + throw ParseError("invalid priority '%s'", s); + } + }}, + // .completer = // no completer since number + }); + }; + std::string description() override { return "install a package into a profile"; @@ -282,6 +303,10 @@ struct CmdProfileInstall : InstallablesCommand, MixDefaultProfile for (auto & installable : installables) { ProfileElement element; + if(priority) { + element.priority = *priority; + }; + if (auto installable2 = std::dynamic_pointer_cast(installable)) { // FIXME: make build() return this? auto [attrPath, resolvedRef, drv] = installable2->toDerivation(); diff --git a/tests/nix-profile.sh b/tests/nix-profile.sh index f8da3d929..1cc724483 100644 --- a/tests/nix-profile.sh +++ b/tests/nix-profile.sh @@ -120,3 +120,19 @@ nix profile install "$flake1Dir^man" (! [ -e $TEST_HOME/.nix-profile/bin/hello ]) [ -e $TEST_HOME/.nix-profile/share/man ] (! [ -e $TEST_HOME/.nix-profile/include ]) + +# test priority +nix profile remove 0 + +# Make another flake. +flake2Dir=$TEST_ROOT/flake2 +printf World > $flake1Dir/who +cp -r $flake1Dir $flake2Dir +printf World2 > $flake2Dir/who + +nix profile install $flake1Dir +[[ $($TEST_HOME/.nix-profile/bin/hello) = "Hello World" ]] +nix profile install $flake2Dir --priority 100 +[[ $($TEST_HOME/.nix-profile/bin/hello) = "Hello World" ]] +nix profile install $flake2Dir --priority 0 +[[ $($TEST_HOME/.nix-profile/bin/hello) = "Hello World2" ]] From 1461e6cdda06f7f461114cce5b415f6d50381311 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Na=C3=AFm=20Favier?= Date: Wed, 11 May 2022 12:55:31 +0200 Subject: [PATCH 10/17] Stop the logger properly in legacy commands Ensures the logger is stopped on exit in legacy commands. Without this, when using `nix-build --log-format bar` and stopping nix with CTRL+C, the bar is not cleared from the screen. --- src/nix-build/nix-build.cc | 4 ---- src/nix-env/nix-env.cc | 2 -- src/nix-store/nix-store.cc | 2 -- src/nix/main.cc | 4 ++-- 4 files changed, 2 insertions(+), 10 deletions(-) diff --git a/src/nix-build/nix-build.cc b/src/nix-build/nix-build.cc index 519855ea3..426f23905 100644 --- a/src/nix-build/nix-build.cc +++ b/src/nix-build/nix-build.cc @@ -543,8 +543,6 @@ static void main_nix_build(int argc, char * * argv) restoreProcessContext(); - logger->stop(); - execvp(shell->c_str(), argPtrs.data()); throw SysError("executing shell '%s'", *shell); @@ -603,8 +601,6 @@ static void main_nix_build(int argc, char * * argv) outPaths.push_back(outputPath); } - logger->stop(); - for (auto & path : outPaths) std::cout << store->printStorePath(path) << '\n'; } diff --git a/src/nix-env/nix-env.cc b/src/nix-env/nix-env.cc index 96f3c3b26..c412bb814 100644 --- a/src/nix-env/nix-env.cc +++ b/src/nix-env/nix-env.cc @@ -1489,8 +1489,6 @@ static int main_nix_env(int argc, char * * argv) globals.state->printStats(); - logger->stop(); - return 0; } } diff --git a/src/nix-store/nix-store.cc b/src/nix-store/nix-store.cc index 153b84137..9163eefd0 100644 --- a/src/nix-store/nix-store.cc +++ b/src/nix-store/nix-store.cc @@ -1095,8 +1095,6 @@ static int main_nix_store(int argc, char * * argv) op(opFlags, opArgs); - logger->stop(); - return 0; } } diff --git a/src/nix/main.cc b/src/nix/main.cc index 6d0f6ce6e..dadb54306 100644 --- a/src/nix/main.cc +++ b/src/nix/main.cc @@ -261,6 +261,8 @@ void mainWrapped(int argc, char * * argv) } #endif + Finally f([] { logger->stop(); }); + programPath = argv[0]; auto programName = std::string(baseNameOf(programPath)); @@ -279,8 +281,6 @@ void mainWrapped(int argc, char * * argv) verbosity = lvlInfo; } - Finally f([] { logger->stop(); }); - NixArgs args; if (argc == 2 && std::string(argv[1]) == "__dump-args") { From 831e2743ea0bac57ebccb73f532667a194b938d7 Mon Sep 17 00:00:00 2001 From: Norbert Melzer Date: Thu, 12 May 2022 00:56:39 +0200 Subject: [PATCH 11/17] fix GitHub URL template --- src/libfetchers/github.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libfetchers/github.cc b/src/libfetchers/github.cc index 0721a13f2..0631fb6e8 100644 --- a/src/libfetchers/github.cc +++ b/src/libfetchers/github.cc @@ -267,8 +267,8 @@ struct GitHubInputScheme : GitArchiveInputScheme auto host = maybeGetStrAttr(input.attrs, "host").value_or("github.com"); auto url = fmt( host == "github.com" - ? "https://api.%s/repos/%s/%s/commits/%s" - : "https://%s/api/v3/repos/%s/%s/commits/%s", + ? "https://api.%s/repos/%s/%s/tarball/%s" + : "https://%s/api/v3/repos/%s/%s/tarball/%s", host, getStrAttr(input.attrs, "owner"), getStrAttr(input.attrs, "repo"), input.getRev()->to_string(Base16, false)); From 65a913d29be7305b2c743fb92c93f0e6bb12d610 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Th=C3=A9ophane=20Hufschmitt?= Date: Thu, 12 May 2022 12:02:31 +0200 Subject: [PATCH 12/17] =?UTF-8?q?Don=E2=80=99t=20recommend=20writing=20uni?= =?UTF-8?q?t=20tests?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit As asked in --- doc/manual/src/contributing/hacking.md | 3 --- 1 file changed, 3 deletions(-) diff --git a/doc/manual/src/contributing/hacking.md b/doc/manual/src/contributing/hacking.md index 7ce8d8de6..59ce5cac7 100644 --- a/doc/manual/src/contributing/hacking.md +++ b/doc/manual/src/contributing/hacking.md @@ -87,9 +87,6 @@ $ nix develop Nix comes with three different flavors of tests: unit, functional and integration. -Most tests are currently written as functional tests. -**However**, it is preferable (as much as it makes sense) to primarily test new code with unit tests. - ### Unit-tests The unit-tests for each Nix library (`libexpr`, `libstore`, etc..) are defined From be2b19041eeec53fba24f7c2494f3f700a4ec595 Mon Sep 17 00:00:00 2001 From: Eli Kogan-Wang Date: Fri, 13 May 2022 22:02:28 +0200 Subject: [PATCH 13/17] Integrate review changes --- src/libcmd/installables.cc | 11 ++++++++--- src/libcmd/installables.hh | 3 ++- src/libexpr/eval-cache.cc | 22 ++++++++++++++++++++++ src/libexpr/eval-cache.hh | 3 +++ src/libstore/builtins/buildenv.cc | 2 +- src/nix/profile.cc | 25 ++++++++++++------------- tests/nix-profile.sh | 2 ++ 7 files changed, 50 insertions(+), 18 deletions(-) diff --git a/src/libcmd/installables.cc b/src/libcmd/installables.cc index a94e60aca..3f6dfd592 100644 --- a/src/libcmd/installables.cc +++ b/src/libcmd/installables.cc @@ -609,7 +609,7 @@ InstallableFlake::InstallableFlake( throw UsageError("'--arg' and '--argstr' are incompatible with flakes"); } -std::tuple InstallableFlake::toDerivation() +std::tuple> InstallableFlake::toDerivation() { auto attr = getCursor(*state); @@ -621,11 +621,15 @@ std::tuple InstallableF auto drvPath = attr->forceDerivation(); std::set outputsToInstall; + std::optional priority; - if (auto aMeta = attr->maybeGetAttr(state->sMeta)) + if (auto aMeta = attr->maybeGetAttr(state->sMeta)) { if (auto aOutputsToInstall = aMeta->maybeGetAttr("outputsToInstall")) for (auto & s : aOutputsToInstall->getListOfStrings()) outputsToInstall.insert(s); + if (auto aPriority = aMeta->maybeGetAttr("priority")) + priority = aPriority->getInt(); + } if (outputsToInstall.empty() || std::get_if(&outputsSpec)) { outputsToInstall.clear(); @@ -643,9 +647,10 @@ std::tuple InstallableF auto drvInfo = DerivationInfo { .drvPath = std::move(drvPath), .outputsToInstall = std::move(outputsToInstall), + .priority = priority, }; - return {attrPath, getLockedFlake()->flake.lockedRef, std::move(drvInfo)}; + return {attrPath, getLockedFlake()->flake.lockedRef, std::move(drvInfo), priority}; } std::vector InstallableFlake::toDerivations() diff --git a/src/libcmd/installables.hh b/src/libcmd/installables.hh index 1a5a96153..d7b61f1b8 100644 --- a/src/libcmd/installables.hh +++ b/src/libcmd/installables.hh @@ -142,6 +142,7 @@ struct InstallableValue : Installable { StorePath drvPath; std::set outputsToInstall; + std::optional priority; }; virtual std::vector toDerivations() = 0; @@ -176,7 +177,7 @@ struct InstallableFlake : InstallableValue Value * getFlakeOutputs(EvalState & state, const flake::LockedFlake & lockedFlake); - std::tuple toDerivation(); + std::tuple> toDerivation(); std::vector toDerivations() override; diff --git a/src/libexpr/eval-cache.cc b/src/libexpr/eval-cache.cc index 0eb4bc79e..b4bce512b 100644 --- a/src/libexpr/eval-cache.cc +++ b/src/libexpr/eval-cache.cc @@ -621,6 +621,28 @@ bool AttrCursor::getBool() return v.boolean; } +NixInt AttrCursor::getInt() +{ + if (root->db) { + if (!cachedValue) + cachedValue = root->db->getAttr(getKey()); + if (cachedValue && !std::get_if(&cachedValue->second)) { + if (auto i = std::get_if(&cachedValue->second)) { + debug("using cached Integer attribute '%s'", getAttrPathStr()); + return *i; + } else + throw TypeError("'%s' is not an Integer", getAttrPathStr()); + } + } + + auto & v = forceValue(); + + if (v.type() != nInt) + throw TypeError("'%s' is not an Integer", getAttrPathStr()); + + return v.integer; +} + std::vector AttrCursor::getListOfStrings() { if (root->db) { diff --git a/src/libexpr/eval-cache.hh b/src/libexpr/eval-cache.hh index 636e293ad..105e9217b 100644 --- a/src/libexpr/eval-cache.hh +++ b/src/libexpr/eval-cache.hh @@ -63,6 +63,7 @@ typedef std::variant< misc_t, failed_t, bool, + NixInt, std::vector > AttrValue; @@ -116,6 +117,8 @@ public: bool getBool(); + NixInt getInt(); + std::vector getListOfStrings(); std::vector getAttrs(); diff --git a/src/libstore/builtins/buildenv.cc b/src/libstore/builtins/buildenv.cc index c17c76e71..58ef89a1c 100644 --- a/src/libstore/builtins/buildenv.cc +++ b/src/libstore/builtins/buildenv.cc @@ -95,7 +95,7 @@ static void createLinks(State & state, const Path & srcDir, const Path & dstDir, throw Error( "files '%1%' and '%2%' have the same priority %3%; " "use 'nix-env --set-flag priority NUMBER INSTALLED_PKGNAME' " - "or 'nix profile --priority NUMBER INSTALLED_PKGNAME' " + "or 'nix profile install --priority NUMBER INSTALLED_PKGNAME' " "to change the priority of one of the conflicting packages" " (0 being the highest priority)", srcFile, readLink(dstFile), priority); diff --git a/src/nix/profile.cc b/src/nix/profile.cc index fb8bef670..ca5041873 100644 --- a/src/nix/profile.cc +++ b/src/nix/profile.cc @@ -269,14 +269,7 @@ struct CmdProfileInstall : InstallablesCommand, MixDefaultProfile .longName = "priority", .description = "The priority of the package to install.", .labels = {"priority"}, - .handler = {[&](std::string s) { - try{ - priority = std::stoi(s); - } catch (std::invalid_argument & e) { - throw ParseError("invalid priority '%s'", s); - } - }}, - // .completer = // no completer since number + .handler = {&priority}, }); }; @@ -303,21 +296,27 @@ struct CmdProfileInstall : InstallablesCommand, MixDefaultProfile for (auto & installable : installables) { ProfileElement element; - if(priority) { - element.priority = *priority; - }; + if (auto installable2 = std::dynamic_pointer_cast(installable)) { // FIXME: make build() return this? - auto [attrPath, resolvedRef, drv] = installable2->toDerivation(); + auto [attrPath, resolvedRef, drv, priority] = installable2->toDerivation(); element.source = ProfileElementSource { installable2->flakeRef, resolvedRef, attrPath, installable2->outputsSpec }; + + if(drv.priority) { + element.priority = *drv.priority; + } } + if(priority) { // if --priority was specified we want to override the priority of the installable + element.priority = *priority; + }; + element.updateStorePaths(getEvalStore(), store, builtPaths[installable.get()]); manifest.elements.push_back(std::move(element)); @@ -476,7 +475,7 @@ struct CmdProfileUpgrade : virtual SourceExprCommand, MixDefaultProfile, MixProf Strings{}, lockFlags); - auto [attrPath, resolvedRef, drv] = installable->toDerivation(); + auto [attrPath, resolvedRef, drv, priority] = installable->toDerivation(); if (element.source->resolvedRef == resolvedRef) continue; diff --git a/tests/nix-profile.sh b/tests/nix-profile.sh index 1cc724483..7ba3235fa 100644 --- a/tests/nix-profile.sh +++ b/tests/nix-profile.sh @@ -136,3 +136,5 @@ nix profile install $flake2Dir --priority 100 [[ $($TEST_HOME/.nix-profile/bin/hello) = "Hello World" ]] nix profile install $flake2Dir --priority 0 [[ $($TEST_HOME/.nix-profile/bin/hello) = "Hello World2" ]] +# nix profile install $flake1Dir --priority 100 +# [[ $($TEST_HOME/.nix-profile/bin/hello) = "Hello World" ]] From c81d24f1c70cc454c9a88cea70048d8563f60784 Mon Sep 17 00:00:00 2001 From: Eli Kogan-Wang Date: Mon, 16 May 2022 02:28:21 +0200 Subject: [PATCH 14/17] Add int to eval-cache, bump eval cache schema version --- src/libexpr/eval-cache.cc | 24 +++++++++++++++++++++++- src/libexpr/eval-cache.hh | 1 + 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/src/libexpr/eval-cache.cc b/src/libexpr/eval-cache.cc index b4bce512b..bf811c8ed 100644 --- a/src/libexpr/eval-cache.cc +++ b/src/libexpr/eval-cache.cc @@ -47,7 +47,7 @@ struct AttrDb { auto state(_state->lock()); - Path cacheDir = getCacheDir() + "/nix/eval-cache-v3"; + Path cacheDir = getCacheDir() + "/nix/eval-cache-v4"; createDirs(cacheDir); Path dbPath = cacheDir + "/" + fingerprint.to_string(Base16, false) + ".sqlite"; @@ -175,6 +175,24 @@ struct AttrDb }); } + AttrId setInt( + AttrKey key, + int n) + { + return doSQLite([&]() + { + auto state(_state->lock()); + + state->insertAttribute.use() + (key.first) + (symbols[key.second]) + (AttrType::Int) + (n).exec(); + + return state->db.getLastInsertedRowId(); + }); + } + AttrId setListOfStrings( AttrKey key, const std::vector & l) @@ -287,6 +305,8 @@ struct AttrDb } case AttrType::Bool: return {{rowId, queryAttribute.getInt(2) != 0}}; + case AttrType::Int: + return {{rowId, queryAttribute.getInt(2)}}; case AttrType::ListOfStrings: return {{rowId, tokenizeString>(queryAttribute.getStr(2), "\t")}}; case AttrType::Missing: @@ -426,6 +446,8 @@ Value & AttrCursor::forceValue() cachedValue = {root->db->setString(getKey(), v.path), string_t{v.path, {}}}; else if (v.type() == nBool) cachedValue = {root->db->setBool(getKey(), v.boolean), v.boolean}; + else if (v.type() == nInt) + cachedValue = {root->db->setInt(getKey(), v.integer), v.integer}; else if (v.type() == nAttrs) ; // FIXME: do something? else diff --git a/src/libexpr/eval-cache.hh b/src/libexpr/eval-cache.hh index 105e9217b..ec255c60d 100644 --- a/src/libexpr/eval-cache.hh +++ b/src/libexpr/eval-cache.hh @@ -45,6 +45,7 @@ enum AttrType { Failed = 5, Bool = 6, ListOfStrings = 7, + Int = 8, }; struct placeholder_t {}; From 27d0f6747d7e70be4b9ade28ce77444e6135cadb Mon Sep 17 00:00:00 2001 From: Eli Kogan-Wang Date: Mon, 16 May 2022 15:17:35 +0200 Subject: [PATCH 15/17] resolve redundant priority passing, wrap NixInt in eval-cache variant --- src/libcmd/installables.cc | 4 ++-- src/libcmd/installables.hh | 2 +- src/libexpr/eval-cache.cc | 6 +++--- src/libexpr/eval-cache.hh | 3 ++- src/nix/profile.cc | 4 ++-- 5 files changed, 10 insertions(+), 9 deletions(-) diff --git a/src/libcmd/installables.cc b/src/libcmd/installables.cc index 3f6dfd592..635ce19b6 100644 --- a/src/libcmd/installables.cc +++ b/src/libcmd/installables.cc @@ -609,7 +609,7 @@ InstallableFlake::InstallableFlake( throw UsageError("'--arg' and '--argstr' are incompatible with flakes"); } -std::tuple> InstallableFlake::toDerivation() +std::tuple InstallableFlake::toDerivation() { auto attr = getCursor(*state); @@ -650,7 +650,7 @@ std::tupleflake.lockedRef, std::move(drvInfo), priority}; + return {attrPath, getLockedFlake()->flake.lockedRef, std::move(drvInfo)}; } std::vector InstallableFlake::toDerivations() diff --git a/src/libcmd/installables.hh b/src/libcmd/installables.hh index d7b61f1b8..5d715210e 100644 --- a/src/libcmd/installables.hh +++ b/src/libcmd/installables.hh @@ -177,7 +177,7 @@ struct InstallableFlake : InstallableValue Value * getFlakeOutputs(EvalState & state, const flake::LockedFlake & lockedFlake); - std::tuple> toDerivation(); + std::tuple toDerivation(); std::vector toDerivations() override; diff --git a/src/libexpr/eval-cache.cc b/src/libexpr/eval-cache.cc index bf811c8ed..6b3c27fd5 100644 --- a/src/libexpr/eval-cache.cc +++ b/src/libexpr/eval-cache.cc @@ -306,7 +306,7 @@ struct AttrDb case AttrType::Bool: return {{rowId, queryAttribute.getInt(2) != 0}}; case AttrType::Int: - return {{rowId, queryAttribute.getInt(2)}}; + return {{rowId, (int_t) queryAttribute.getInt(2)}}; case AttrType::ListOfStrings: return {{rowId, tokenizeString>(queryAttribute.getStr(2), "\t")}}; case AttrType::Missing: @@ -649,9 +649,9 @@ NixInt AttrCursor::getInt() if (!cachedValue) cachedValue = root->db->getAttr(getKey()); if (cachedValue && !std::get_if(&cachedValue->second)) { - if (auto i = std::get_if(&cachedValue->second)) { + if (auto i = std::get_if(&cachedValue->second)) { debug("using cached Integer attribute '%s'", getAttrPathStr()); - return *i; + return (*i).x; } else throw TypeError("'%s' is not an Integer", getAttrPathStr()); } diff --git a/src/libexpr/eval-cache.hh b/src/libexpr/eval-cache.hh index ec255c60d..68b5952eb 100644 --- a/src/libexpr/eval-cache.hh +++ b/src/libexpr/eval-cache.hh @@ -52,6 +52,7 @@ struct placeholder_t {}; struct missing_t {}; struct misc_t {}; struct failed_t {}; +struct int_t { NixInt x; int_t(NixInt x) : x(x) {}; }; typedef uint64_t AttrId; typedef std::pair AttrKey; typedef std::pair string_t; @@ -64,7 +65,7 @@ typedef std::variant< misc_t, failed_t, bool, - NixInt, + int_t, std::vector > AttrValue; diff --git a/src/nix/profile.cc b/src/nix/profile.cc index ca5041873..1aae347df 100644 --- a/src/nix/profile.cc +++ b/src/nix/profile.cc @@ -300,7 +300,7 @@ struct CmdProfileInstall : InstallablesCommand, MixDefaultProfile if (auto installable2 = std::dynamic_pointer_cast(installable)) { // FIXME: make build() return this? - auto [attrPath, resolvedRef, drv, priority] = installable2->toDerivation(); + auto [attrPath, resolvedRef, drv] = installable2->toDerivation(); element.source = ProfileElementSource { installable2->flakeRef, resolvedRef, @@ -475,7 +475,7 @@ struct CmdProfileUpgrade : virtual SourceExprCommand, MixDefaultProfile, MixProf Strings{}, lockFlags); - auto [attrPath, resolvedRef, drv, priority] = installable->toDerivation(); + auto [attrPath, resolvedRef, drv] = installable->toDerivation(); if (element.source->resolvedRef == resolvedRef) continue; From e53349dd6e4a710eb7abff78722853cad418e9d2 Mon Sep 17 00:00:00 2001 From: Eli Kogan-Wang Date: Mon, 16 May 2022 16:16:06 +0200 Subject: [PATCH 16/17] change priority conflict message --- src/libstore/builtins/buildenv.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libstore/builtins/buildenv.cc b/src/libstore/builtins/buildenv.cc index 58ef89a1c..47458a388 100644 --- a/src/libstore/builtins/buildenv.cc +++ b/src/libstore/builtins/buildenv.cc @@ -95,7 +95,7 @@ static void createLinks(State & state, const Path & srcDir, const Path & dstDir, throw Error( "files '%1%' and '%2%' have the same priority %3%; " "use 'nix-env --set-flag priority NUMBER INSTALLED_PKGNAME' " - "or 'nix profile install --priority NUMBER INSTALLED_PKGNAME' " + "or type 'nix profile install --help' if using 'nix profile' to find out how" "to change the priority of one of the conflicting packages" " (0 being the highest priority)", srcFile, readLink(dstFile), priority); From 43a2c1367292733d3e0aa2e57137c897fb66d8f6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Th=C3=A9ophane=20Hufschmitt?= Date: Mon, 16 May 2022 16:36:21 +0200 Subject: [PATCH 17/17] Make nix::eval_cache::int_t more idiomatic MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Don’t explicitely give it a constructor, but use aggregate initialization instead (also prevents having an implicit coertion, which is probably good here) --- src/libexpr/eval-cache.cc | 6 +++--- src/libexpr/eval-cache.hh | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/libexpr/eval-cache.cc b/src/libexpr/eval-cache.cc index 6b3c27fd5..6a2e775d0 100644 --- a/src/libexpr/eval-cache.cc +++ b/src/libexpr/eval-cache.cc @@ -306,7 +306,7 @@ struct AttrDb case AttrType::Bool: return {{rowId, queryAttribute.getInt(2) != 0}}; case AttrType::Int: - return {{rowId, (int_t) queryAttribute.getInt(2)}}; + return {{rowId, int_t{queryAttribute.getInt(2)}}}; case AttrType::ListOfStrings: return {{rowId, tokenizeString>(queryAttribute.getStr(2), "\t")}}; case AttrType::Missing: @@ -447,7 +447,7 @@ Value & AttrCursor::forceValue() else if (v.type() == nBool) cachedValue = {root->db->setBool(getKey(), v.boolean), v.boolean}; else if (v.type() == nInt) - cachedValue = {root->db->setInt(getKey(), v.integer), v.integer}; + cachedValue = {root->db->setInt(getKey(), v.integer), int_t{v.integer}}; else if (v.type() == nAttrs) ; // FIXME: do something? else @@ -651,7 +651,7 @@ NixInt AttrCursor::getInt() if (cachedValue && !std::get_if(&cachedValue->second)) { if (auto i = std::get_if(&cachedValue->second)) { debug("using cached Integer attribute '%s'", getAttrPathStr()); - return (*i).x; + return i->x; } else throw TypeError("'%s' is not an Integer", getAttrPathStr()); } diff --git a/src/libexpr/eval-cache.hh b/src/libexpr/eval-cache.hh index 68b5952eb..c93e55b93 100644 --- a/src/libexpr/eval-cache.hh +++ b/src/libexpr/eval-cache.hh @@ -52,7 +52,7 @@ struct placeholder_t {}; struct missing_t {}; struct misc_t {}; struct failed_t {}; -struct int_t { NixInt x; int_t(NixInt x) : x(x) {}; }; +struct int_t { NixInt x; }; typedef uint64_t AttrId; typedef std::pair AttrKey; typedef std::pair string_t;