From 6f8e9c4aeb4e3a96c7693c805c5c931ff3740fcb Mon Sep 17 00:00:00 2001 From: Amadey Vorontsov Date: Sun, 10 May 2026 12:34:48 +0000 Subject: [PATCH] [M5+] resolve_version + cmd_add lockfile pin --- CHANGELOG.md | 12 ++++++ CMakeLists.txt | 1 + docs/version-resolution.md | 4 +- src/cli/cmd_add.cpp | 73 +++++++++++++++++++++++++++++++- src/resolver/resolver.cppm | 11 +++++ src/resolver/version_resolve.cpp | 42 ++++++++++++++++++ tests/cmd_add.cpp | 14 +++--- tests/cmd_remove.cpp | 7 +++ 8 files changed, 155 insertions(+), 9 deletions(-) create mode 100644 src/resolver/version_resolve.cpp diff --git a/CHANGELOG.md b/CHANGELOG.md index e6914dd..fad0d91 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -91,6 +91,18 @@ All notable changes to cargoxx will be documented in this file. 6 cases against fixtures derived from a real fmt 10.2.1 response; `tests/devbox_resolve_live.cpp` (gated by `CARGOXX_NETWORK_TESTS=1`) hits the live API. +- `cargoxx.resolver::resolve_version(name, version)` orchestrator + chains `devbox_resolve` (HTTP, primary) → `nixpkgs_git_resolve` + (offline, fallback). Returns a 40-char nixpkgs SHA. Wildcards + (`*`, empty) are rejected with `ResolutionVersionNotFound` since + they aren't concrete pins. +- `cargoxx add @` now resolves the pin to a nixpkgs + rev *before* writing the manifest. Failure to resolve aborts the + add cleanly (no manifest mutation, no lockfile mutation). On + success the rev is persisted into `Cargoxx.lock`'s `nixpkgs_rev` + for that dep. Wildcards (`cargoxx add fmt`) skip resolution and + track `nixos-unstable` as before. Tests opt out of the network + resolver via `CARGOXX_NO_AUTORESOLVE=1`. - `cargoxx.resolver::nixpkgs_git_resolve(name, version, repo_path?)` — fallback for when search.devbox.sh is unreachable. Lazily clones `https://github.com/NixOS/nixpkgs.git` into diff --git a/CMakeLists.txt b/CMakeLists.txt index 03eb422..d0fd2a7 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -60,6 +60,7 @@ target_sources(cargoxx src/resolver/discover.cpp src/resolver/search_devbox.cpp src/resolver/nixpkgs_git.cpp + src/resolver/version_resolve.cpp src/cli/cmd_new.cpp src/cli/cmd_build.cpp src/cli/cmd_run.cpp diff --git a/docs/version-resolution.md b/docs/version-resolution.md index 5b6bcf5..ecce519 100644 --- a/docs/version-resolution.md +++ b/docs/version-resolution.md @@ -307,8 +307,8 @@ mitigation is to append `-` to the input attr. | Phase | Status | Commit | | --- | --- | --- | | 1. devbox_resolve + parser | ✅ | `df2c25b` | -| 2. nixpkgs_git_resolve fallback | ✅ | (this commit) | -| 3. resolve_version + cmd_add wire-up | pending | — | +| 2. nixpkgs_git_resolve fallback | ✅ | `cb82e91` | +| 3. resolve_version + cmd_add wire-up | ✅ | (this commit) | | 4. cmd_build lockfile merge | pending | — | | 5. flake codegen for per-dep inputs | pending | — | | 6. SPEC §7/§10 amendment + smoke | pending | — | diff --git a/src/cli/cmd_add.cpp b/src/cli/cmd_add.cpp index ef8158d..ba6b461 100644 --- a/src/cli/cmd_add.cpp +++ b/src/cli/cmd_add.cpp @@ -4,6 +4,7 @@ import std; import cargoxx.util; import cargoxx.manifest; import cargoxx.linkdb; +import cargoxx.lockfile; import cargoxx.resolver; namespace cargoxx::cli { @@ -33,6 +34,47 @@ auto recipe_already_known(const std::string& name, const std::string& version, return false; } +// Persists `nixpkgs_rev` for the (name, version) pair into Cargoxx.lock. +// Overwrites any existing entry for the same (name, version). Other +// lockfile entries (root package, sibling deps) are preserved verbatim. +auto record_lockfile_rev(const fs::path& project_root, const std::string& name, + const std::string& version, const std::string& rev) + -> util::Result { + const auto lock_path = project_root / "Cargoxx.lock"; + lockfile::Lockfile lock; + std::error_code ec; + if (std::filesystem::exists(lock_path, ec)) { + auto parsed = lockfile::parse(lock_path); + if (!parsed) { + return std::unexpected(parsed.error()); + } + lock = std::move(*parsed); + } else { + lock.version = 1; + } + + bool replaced = false; + for (auto& p : lock.packages) { + if (p.name == name && p.version == version) { + p.nixpkgs_rev = rev; + replaced = true; + break; + } + } + if (!replaced) { + lock.packages.push_back(lockfile::LockfilePackage{ + .name = name, + .version = version, + .dependencies = {}, + .nixpkgs_attr = std::nullopt, + .nixpkgs_rev = rev, + .linkdb_source = std::nullopt, + }); + } + + return lockfile::write(lock, lock_path); +} + // Drives the resolver chain (Conan → vcpkg → nix-cmake-scan), running a // real `cmd_build` against each candidate via verify_link. On success the // overlay carries a confirmed row for the package. @@ -119,13 +161,42 @@ auto cmd_add(const fs::path& project_root, const std::string& name, } } + // Concrete @ is a hard pin — must yield a specific nixpkgs + // rev for the generated flake.nix. Resolve it BEFORE writing the + // manifest so a failure leaves nothing on disk. Wildcards (`*`) and + // CARGOXX_NO_AUTORESOLVE skip this step entirely; the dep tracks + // nixos-unstable. + const bool wildcard = effective_version == "*"; + auto* env = std::getenv("CARGOXX_NO_AUTORESOLVE"); + const bool autoresolve_disabled = env != nullptr && *env != 0; + std::optional resolved_rev; + if (!wildcard && !autoresolve_disabled) { + auto rev = resolver::resolve_version(name, effective_version); + if (!rev) { + return std::unexpected(rev.error()); + } + resolved_rev = std::move(*rev); + } + m->dependencies.push_back(manifest::Dependency{ .name = name, .version_spec = effective_version, .components = std::move(components), }); - return manifest::write(*m, manifest_path); + if (auto r = manifest::write(*m, manifest_path); !r) { + return std::unexpected(r.error()); + } + + if (resolved_rev) { + if (auto r = record_lockfile_rev(project_root, name, effective_version, + *resolved_rev); + !r) { + return std::unexpected(r.error()); + } + } + + return {}; } } // namespace cargoxx::cli diff --git a/src/resolver/resolver.cppm b/src/resolver/resolver.cppm index bafb8f9..955d524 100644 --- a/src/resolver/resolver.cppm +++ b/src/resolver/resolver.cppm @@ -174,4 +174,15 @@ auto nixpkgs_git_resolve(const std::string& name, const std::string& version, std::optional repo_path = std::nullopt) -> util::Result; +// Top-level orchestrator: try `devbox_resolve` first, then +// `nixpkgs_git_resolve` as fallback. Returns a 40-char nixpkgs SHA, or +// `ResolutionVersionNotFound` when both probes come back empty. +// +// Use this from `cargoxx add @` to capture the rev +// that lockfile/codegen will pin. Wildcards (`*`, empty) should NOT be +// passed — they are not concrete versions and the resolver returns +// `ResolutionVersionNotFound` for them by design. +auto resolve_version(const std::string& name, const std::string& version) + -> util::Result; + } // namespace cargoxx::resolver diff --git a/src/resolver/version_resolve.cpp b/src/resolver/version_resolve.cpp new file mode 100644 index 0000000..a24e6d5 --- /dev/null +++ b/src/resolver/version_resolve.cpp @@ -0,0 +1,42 @@ +module cargoxx.resolver; + +import std; +import cargoxx.util; + +namespace cargoxx::resolver { + +namespace { + +auto error(util::ErrorCode code, std::string msg) -> util::Error { + return util::Error{code, std::move(msg), "", std::nullopt, std::nullopt}; +} + +} // namespace + +auto resolve_version(const std::string& name, const std::string& version) + -> util::Result { + if (name.empty()) { + return std::unexpected(error(util::ErrorCode::ResolutionUnknownPackage, + "resolve_version: name is empty")); + } + if (version.empty() || version == "*") { + return std::unexpected(error( + util::ErrorCode::ResolutionVersionNotFound, + std::format("resolve_version requires a concrete version (got '{}')", + version))); + } + + if (auto r = devbox_resolve(name, version); r) { + if (!r->commit_hash.empty()) { + return r->commit_hash; + } + } + if (auto r = nixpkgs_git_resolve(name, version, std::nullopt); r) { + return *r; + } + return std::unexpected(error( + util::ErrorCode::ResolutionVersionNotFound, + std::format("no nixpkgs revision found for {} {}", name, version))); +} + +} // namespace cargoxx::resolver diff --git a/tests/cmd_add.cpp b/tests/cmd_add.cpp index 6d1d0d0..6955b6f 100644 --- a/tests/cmd_add.cpp +++ b/tests/cmd_add.cpp @@ -12,6 +12,14 @@ namespace manifest = cargoxx::manifest; namespace { +// Disable resolve_version for the whole TU — these unit tests don't +// want to hit search.devbox.sh or clone nixpkgs. Individual cases that +// actually want to test the network path still set the env locally. +struct DisableAutoResolveScope { + DisableAutoResolveScope() { setenv("CARGOXX_NO_AUTORESOLVE", "1", 1); } +}; +const DisableAutoResolveScope autoresolve_disabled_; + auto fresh_dir() -> std::filesystem::path { auto d = std::filesystem::temp_directory_path() / std::format("cargoxx-add-test-{}", std::random_device{}()); @@ -80,9 +88,7 @@ TEST_CASE("cmd_add with wildcard version still rejects unknown packages", auto parent = fresh_dir(); auto root = scaffold(parent); - setenv("CARGOXX_NO_AUTORESOLVE", "1", /*overwrite=*/1); auto r = cmd_add(root, "obscurelib", "", {}, overlay_path(parent)); - unsetenv("CARGOXX_NO_AUTORESOLVE"); REQUIRE_FALSE(r.has_value()); REQUIRE(r.error().code == ErrorCode::LinkdbUnknownPackage); } @@ -91,11 +97,7 @@ TEST_CASE("cmd_add rejects an unknown package", "[cli][add]") { auto parent = fresh_dir(); auto root = scaffold(parent); - // Disable the auto-resolution chain — keeps the unit test fast and - // independent of nixpkgs / Conan / vcpkg availability. - setenv("CARGOXX_NO_AUTORESOLVE", "1", /*overwrite=*/1); auto r = cmd_add(root, "obscurelib", "0.0.1", {}, overlay_path(parent)); - unsetenv("CARGOXX_NO_AUTORESOLVE"); REQUIRE_FALSE(r.has_value()); REQUIRE(r.error().code == ErrorCode::LinkdbUnknownPackage); } diff --git a/tests/cmd_remove.cpp b/tests/cmd_remove.cpp index f828536..60ae5ab 100644 --- a/tests/cmd_remove.cpp +++ b/tests/cmd_remove.cpp @@ -13,6 +13,13 @@ namespace manifest = cargoxx::manifest; namespace { +// Disable the network resolver chains in cmd_add (which we use as a +// fixture-builder). Per-version-resolution tests live elsewhere. +struct DisableAutoResolveScope { + DisableAutoResolveScope() { setenv("CARGOXX_NO_AUTORESOLVE", "1", 1); } +}; +const DisableAutoResolveScope autoresolve_disabled_; + auto fresh_dir() -> std::filesystem::path { auto d = std::filesystem::temp_directory_path() / std::format("cargoxx-remove-test-{}", std::random_device{}());