From f5b9f105e256dba7c58d24639c8d579a336046b5 Mon Sep 17 00:00:00 2001 From: eldritch horrors Date: Thu, 16 Jul 2026 21:40:43 +0200 Subject: [PATCH] libstore: drop Store::connect there's no reason a store should not open a connection during init if it needs to open any connection to function. delaying connection setup like this makes openStore less deterministic and graceful fallback impossible this caused test failures where the old behaviour was required to ensure test output stability. because of course something like that must happen Change-Id: I946adddee026f1f4c74b4699d730b4d1ac9a5072 --- lix/legacy/build-remote.cc | 1 - lix/libstore/legacy-ssh-store.cc | 2 +- lix/libstore/remote-store.cc | 3 +-- lix/libstore/remote-store.hh | 2 +- lix/libstore/store-api.hh | 9 ++------- lix/nix/ping-store.cc | 2 -- tests/functional2/cli/test_daemon.py | 2 +- tests/functional2/eval/test_attr_paths.py | 2 ++ tests/functional2/eval/test_symlink_resoultion.py | 4 ++++ tests/functional2/repl_characterization/test_repl.py | 1 + tests/functional2/store/test_fetch_credentials.py | 1 + 11 files changed, 14 insertions(+), 15 deletions(-) diff --git a/lix/legacy/build-remote.cc b/lix/legacy/build-remote.cc index 2311e6d11..7c030953a 100644 --- a/lix/legacy/build-remote.cc +++ b/lix/legacy/build-remote.cc @@ -272,7 +272,6 @@ try { logger->startActivity(lvlTalkative, actUnknown, fmt("connecting to '%s'", bestMachine->name)); sshStore = TRY_AWAIT(bestMachine->openStore()); - TRY_AWAIT(sshStore->connect()); co_return BuilderConnection{std::move(bestSlotLock), sshStore, bestMachine->storeUri}; } catch (std::exception & e) { // NOLINT(lix-foreign-exceptions) printError("cannot build on '%s': %s", bestMachine->name, e.what()); diff --git a/lix/libstore/legacy-ssh-store.cc b/lix/libstore/legacy-ssh-store.cc index ea4240332..530e115cd 100644 --- a/lix/libstore/legacy-ssh-store.cc +++ b/lix/libstore/legacy-ssh-store.cc @@ -594,7 +594,7 @@ public: co_return result::current_exception(); } - kj::Promise> connect() override + kj::Promise> init() override try { auto conn(TRY_AWAIT(connections->get())); co_return result::success(); diff --git a/lix/libstore/remote-store.cc b/lix/libstore/remote-store.cc index c9d73511e..101a895a0 100644 --- a/lix/libstore/remote-store.cc +++ b/lix/libstore/remote-store.cc @@ -697,8 +697,7 @@ try { co_return result::current_exception(); } - -kj::Promise> RemoteStore::connect() +kj::Promise> RemoteStore::init() try { auto conn(TRY_AWAIT(getConnection())); co_return result::success(); diff --git a/lix/libstore/remote-store.hh b/lix/libstore/remote-store.hh index 1a81b876f..b66cf3138 100644 --- a/lix/libstore/remote-store.hh +++ b/lix/libstore/remote-store.hh @@ -170,7 +170,7 @@ public: kj::Promise>> getVersion() override; - kj::Promise> connect() override; + kj::Promise> init() override; kj::Promise> getProtocol() override; diff --git a/lix/libstore/store-api.hh b/lix/libstore/store-api.hh index c2b0bbaaa..859a1b602 100644 --- a/lix/libstore/store-api.hh +++ b/lix/libstore/store-api.hh @@ -253,7 +253,8 @@ public: /** * Perform any necessary effectful operation to make the store up and - * running + * running. For stores that open connections to remote hosts `init()` + * must also open such a connection and report errors as appropriate. */ virtual kj::Promise> init() { @@ -874,12 +875,6 @@ public: (co_await state.lock())->pathInfoCache.clear(); } - /** - * Establish a connection to the store, for store types that have - * a notion of connection. Otherwise this is a no-op. - */ - virtual kj::Promise> connect() { return {result::success()}; } - /** * Get the protocol version of this store or it's connection. */ diff --git a/lix/nix/ping-store.cc b/lix/nix/ping-store.cc index 1bc1899b6..5391409fc 100644 --- a/lix/nix/ping-store.cc +++ b/lix/nix/ping-store.cc @@ -25,7 +25,6 @@ struct CmdPingStore : StoreCommand, MixJSON { if (!json) { notice("Store URL: %s", store->getUri()); - aio().blockOn(store->connect()); if (auto version = aio().blockOn(store->getVersion())) notice("Version: %s", *version); if (auto trusted = aio().blockOn(store->isTrustedClient())) @@ -37,7 +36,6 @@ struct CmdPingStore : StoreCommand, MixJSON }); res["url"] = store->getUri(); - aio().blockOn(store->connect()); if (auto version = aio().blockOn(store->getVersion())) res["version"] = *version; if (auto trusted = aio().blockOn(store->isTrustedClient())) diff --git a/tests/functional2/cli/test_daemon.py b/tests/functional2/cli/test_daemon.py index 173da57bd..ad7a04665 100644 --- a/tests/functional2/cli/test_daemon.py +++ b/tests/functional2/cli/test_daemon.py @@ -25,5 +25,5 @@ def test_stdio_forward_failure(nix: Nix, files: Path): .run() .expect(1) ) - assert "stream ended unexpectedly" in result.stderr_plain + assert "cannot open connection to remote store" in result.stderr_plain assert "Lix crashed" not in result.stderr_plain diff --git a/tests/functional2/eval/test_attr_paths.py b/tests/functional2/eval/test_attr_paths.py index 5b5923fa7..957f85d1d 100644 --- a/tests/functional2/eval/test_attr_paths.py +++ b/tests/functional2/eval/test_attr_paths.py @@ -3,6 +3,8 @@ import pytest from typing import NamedTuple from textwrap import dedent +pytestmark = pytest.mark.no_daemon + class ShouldError(NamedTuple): expr: str diff --git a/tests/functional2/eval/test_symlink_resoultion.py b/tests/functional2/eval/test_symlink_resoultion.py index 844cfe1a8..c317de430 100644 --- a/tests/functional2/eval/test_symlink_resoultion.py +++ b/tests/functional2/eval/test_symlink_resoultion.py @@ -1,10 +1,14 @@ from collections.abc import Callable from pathlib import Path +import pytest + from testlib.fixtures.file_helper import with_files, File, AssetSymlink, Symlink from testlib.fixtures.nix import Nix from testlib.fixtures.snapshot import Snapshot +pytestmark = pytest.mark.no_daemon + @with_files( { diff --git a/tests/functional2/repl_characterization/test_repl.py b/tests/functional2/repl_characterization/test_repl.py index ff5e5b4d3..eba063703 100644 --- a/tests/functional2/repl_characterization/test_repl.py +++ b/tests/functional2/repl_characterization/test_repl.py @@ -21,6 +21,7 @@ def _clean_output(output: str, origin: Path) -> str: return re.sub(lix_version_regex, "Lix VERSION", output.replace(str(origin), "/pwd")) +@pytest.mark.nix_settings(trusted_users="*") # silence trusted settings warnings def test_repl_char(nix: Nix, do_snapshot_update: bool, metadata: ReplTestMetadata, files: Path): nix.settings.add_xp_feature("nix-command", "flakes", "repl-automation") with MarkdownRenderer() as renderer: diff --git a/tests/functional2/store/test_fetch_credentials.py b/tests/functional2/store/test_fetch_credentials.py index 84a4c41f9..b12724bb9 100644 --- a/tests/functional2/store/test_fetch_credentials.py +++ b/tests/functional2/store/test_fetch_credentials.py @@ -9,6 +9,7 @@ from testlib.fixtures.file_helper import File, FileDeclaration, with_files from testlib.fixtures.http_server import http_server from testlib.fixtures.nix import Nix +pytestmark = pytest.mark.nix_settings(trusted_users="*") ca_key: str = """ -----BEGIN EC PARAMETERS----- BggqhkjOPQMBBw==