Skip to content

Conversation

@edolstra
Copy link
Collaborator

@edolstra edolstra commented Oct 10, 2025

Motivation

This makes nix flake clone work on all input types, including tarballs, e.g.

# nix flake clone "https://flakehub.com/f/NixOS/nixpkgs/*" --dest nixpkgs

Context

Summary by CodeRabbit

  • New Features
    • Support cloning tarball-based flakes into a directory via flake clone.
  • Bug Fixes
    • More robust path handling and clearer errors when the destination already exists during flake cloning.
  • Refactor
    • Standardized destination path handling and propagated store context for cloning operations to improve reliability.
  • Tests
    • Added functional tests covering cloning of tarball flakes, presence of flake.nix in the unpacked directory, and error behavior on re-clone into an existing path.

For input types that have no concept of cloning, we now default to
copying the entire source tree.
@coderabbitai
Copy link

coderabbitai bot commented Oct 10, 2025

Walkthrough

Introduces a store-aware cloning pathway. Updates clone method signatures in Input, InputScheme, and scheme implementations (Git, GitHub, GitLab, SourceHut) to accept ref and std::filesystem::path. CmdFlakeClone updated to pass store. Implements store-backed cloning in fetchers.cc. Adds functional tests for cloning tarball flakes and existing-destination errors.

Changes

Cohort / File(s) Summary
Core fetchers logic
src/libfetchers/fetchers.cc
Refactors Input::clone and InputScheme::clone to accept ref and std::filesystem::path; implements store-backed accessor cloning with existence check, activity logging, streaming to sink, and path restore.
Git-based schemes
src/libfetchers/git.cc, src/libfetchers/github.cc
Updates clone signatures to include ref and std::filesystem::path; propagates store to downstream clone calls for Git, GitHub, GitLab, SourceHut input schemes.
Public headers / API
src/libfetchers/include/nix/fetchers/fetchers.hh
Changes public API: Input::clone and InputScheme::clone now require ref and std::filesystem::path.
CLI flake command
src/nix/flake.cc
CmdFlakeClone destDir type changed to std::filesystem::path; passes store to input.clone(store, destDir).
Functional tests
tests/functional/flakes/flakes.sh
Adds tests for cloning tarball flakes, verifies unpacked flake.nix presence, and asserts error on re-clone into existing path.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor User
  participant CLI as nix flake clone
  participant Store
  participant Input as Input
  participant Scheme as InputScheme
  participant FS as Filesystem

  User->>CLI: nix flake clone URL destDir
  CLI->>Store: resolve(URL)
  Store-->>CLI: Input
  CLI->>Input: clone(Store, destDir)
  Input->>Scheme: clone(Store, Input, destDir)

  rect rgba(220,235,255,0.5)
    note over Scheme: Store-backed cloning
    Scheme->>FS: check destDir exists
    alt dest exists
      Scheme-->>CLI: error (path exists)
    else
      Scheme->>Store: makeAccessor(Input)
      Scheme->>Store: openSink(destDir)
      Store-->>Scheme: sink
      Scheme->>Store: stream accessor -> sink
      Scheme->>Store: restorePath(sink)
      Scheme-->>CLI: success
    end
  end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Poem

In burrows of code where fetchers abide,
I nibbled the paths, with Store as my guide.
We packed up the flakes, to sinks we bestowed—
If a den was taken, we paused the load.
Hop, clone, restore—now neatly aligned.
Carrots committed, dependencies signed. 🥕

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title clearly and concisely conveys the primary change by specifying that the nix flake clone command now supports all input types, directly reflecting the pull request’s objective and implementation.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch eelcodolstra/fh-1073-nix-flake-clone-doesnt-understand-urls

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 80d3406 and 9bf4eba.

📒 Files selected for processing (6)
  • src/libfetchers/fetchers.cc (3 hunks)
  • src/libfetchers/git.cc (1 hunks)
  • src/libfetchers/github.cc (3 hunks)
  • src/libfetchers/include/nix/fetchers/fetchers.hh (2 hunks)
  • src/nix/flake.cc (2 hunks)
  • tests/functional/flakes/flakes.sh (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (6)
src/libfetchers/git.cc (1)
src/libfetchers/github.cc (32)
  • store (235-235)
  • store (245-315)
  • store (245-245)
  • store (317-337)
  • store (317-317)
  • store (349-355)
  • store (349-349)
  • store (391-409)
  • store (391-391)
  • store (429-435)
  • store (429-429)
  • store (464-488)
  • store (464-464)
  • store (509-518)
  • store (509-509)
  • store (538-585)
  • input (136-158)
  • input (136-136)
  • input (162-162)
  • input (237-237)
  • input (339-347)
  • input (339-339)
  • input (376-379)
  • input (376-376)
  • input (381-384)
  • input (381-381)
  • input (386-389)
  • input (386-386)
  • input (411-427)
  • input (411-411)
  • input (490-507)
  • input (490-490)
tests/functional/flakes/flakes.sh (1)
src/libfetchers/fetchers.cc (4)
  • clone (393-397)
  • clone (393-393)
  • clone (509-521)
  • clone (509-509)
src/libfetchers/include/nix/fetchers/fetchers.hh (2)
src/libfetchers/git.cc (18)
  • store (286-305)
  • store (286-286)
  • input (246-272)
  • input (246-246)
  • input (274-284)
  • input (274-274)
  • input (307-310)
  • input (307-307)
  • input (312-377)
  • input (312-316)
  • input (420-423)
  • input (420-420)
  • input (425-428)
  • input (425-425)
  • input (430-433)
  • input (430-430)
  • input (435-438)
  • input (435-435)
src/libfetchers/github.cc (14)
  • store (235-235)
  • store (245-315)
  • store (245-245)
  • store (317-337)
  • store (317-317)
  • store (349-355)
  • store (349-349)
  • store (391-409)
  • store (391-391)
  • store (429-435)
  • store (429-429)
  • store (464-488)
  • store (464-464)
  • store (509-518)
src/nix/flake.cc (1)
src/libfetchers/github.cc (16)
  • store (235-235)
  • store (245-315)
  • store (245-245)
  • store (317-337)
  • store (317-317)
  • store (349-355)
  • store (349-349)
  • store (391-409)
  • store (391-391)
  • store (429-435)
  • store (429-429)
  • store (464-488)
  • store (464-464)
  • store (509-518)
  • store (509-509)
  • store (538-585)
src/libfetchers/fetchers.cc (4)
src/libfetchers/git.cc (20)
  • store (286-305)
  • store (286-286)
  • input (246-272)
  • input (246-246)
  • input (274-284)
  • input (274-274)
  • input (307-310)
  • input (307-307)
  • input (312-377)
  • input (312-316)
  • input (420-423)
  • input (420-420)
  • input (425-428)
  • input (425-425)
  • input (430-433)
  • input (430-430)
  • input (435-438)
  • input (435-435)
  • getAccessor (863-883)
  • getAccessor (863-863)
src/libfetchers/github.cc (14)
  • store (235-235)
  • store (245-315)
  • store (245-245)
  • store (317-337)
  • store (317-317)
  • store (349-355)
  • store (349-349)
  • store (391-409)
  • store (391-391)
  • store (429-435)
  • store (429-429)
  • store (464-488)
  • store (464-464)
  • store (509-518)
src/libfetchers/git-utils.cc (4)
  • getAccessor (1286-1296)
  • getAccessor (1287-1287)
  • getAccessor (1298-1312)
  • getAccessor (1299-1299)
src/libutil/archive.cc (2)
  • restorePath (275-280)
  • restorePath (275-275)
src/libfetchers/github.cc (2)
src/libfetchers/git.cc (18)
  • store (286-305)
  • store (286-286)
  • input (246-272)
  • input (246-246)
  • input (274-284)
  • input (274-274)
  • input (307-310)
  • input (307-307)
  • input (312-377)
  • input (312-316)
  • input (420-423)
  • input (420-420)
  • input (425-428)
  • input (425-425)
  • input (430-433)
  • input (430-430)
  • input (435-438)
  • input (435-435)
src/nix/flake.cc (14)
  • store (128-139)
  • store (128-128)
  • store (162-172)
  • store (162-162)
  • store (216-317)
  • store (216-216)
  • store (322-326)
  • store (322-322)
  • store (360-852)
  • store (360-360)
  • store (885-982)
  • store (885-885)
  • store (1052-1058)
  • store (1052-1052)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: build_aarch64-darwin / build
  • GitHub Check: build_x86_64-linux / build
🔇 Additional comments (1)
src/libfetchers/github.cc (1)

429-435: LGTM! Clone method signatures updated correctly.

All three implementations (GitHub, GitLab, SourceHut) properly updated to the new API with ref<Store> store and std::filesystem::path destDir parameters, and correctly propagate the store through the internal clone chain.

Also applies to: 509-518, 601-609


Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions
Copy link

@github-actions github-actions bot temporarily deployed to pull request October 10, 2025 14:19 Inactive
@edolstra edolstra added this pull request to the merge queue Oct 13, 2025
Merged via the queue into main with commit 51bf3ea Oct 13, 2025
35 checks passed
@edolstra edolstra deleted the eelcodolstra/fh-1073-nix-flake-clone-doesnt-understand-urls branch October 13, 2025 11:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants