Skip to content

fetchFromRadicle: init#434360

Merged
SuperSandro2000 merged 5 commits intoNixOS:masterfrom
Defelo:push-uzswprnmqzky
Aug 21, 2025
Merged

fetchFromRadicle: init#434360
SuperSandro2000 merged 5 commits intoNixOS:masterfrom
Defelo:push-uzswprnmqzky

Conversation

@Defelo
Copy link
Copy Markdown
Member

@Defelo Defelo commented Aug 17, 2025

Add a fetcher for projects hosted on Radicle.

Things done

  • Built on platform:
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • Tested, as applicable:
  • Ran nixpkgs-review on this PR. See nixpkgs-review usage.
  • Tested basic functionality of all binary files, usually in ./result/bin/.
  • Nixpkgs Release Notes
    • Package update: when the change is major or breaking.
  • NixOS Release Notes
    • Module addition: when adding a new NixOS module.
    • Module update: when the change is significant.
  • Fits CONTRIBUTING.md, pkgs/README.md, maintainers/README.md and other READMEs.

Add a 👍 reaction to pull requests you find important.

@nixpkgs-ci nixpkgs-ci bot added 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-darwin: 1 This PR causes 1 package to rebuild on Darwin. 10.rebuild-linux: 1 This PR causes 1 package to rebuild on Linux. 6.topic: fetch Fetchers (e.g. fetchgit, fetchsvn, ...) labels Aug 17, 2025
@Defelo Defelo marked this pull request as ready for review August 17, 2025 06:39
@nixpkgs-ci nixpkgs-ci bot added the 9.needs: reviewer This PR currently has no reviewers requested and needs attention. label Aug 17, 2025
@nixpkgs-ci nixpkgs-ci bot removed the 9.needs: reviewer This PR currently has no reviewers requested and needs attention. label Aug 17, 2025
@Defelo Defelo force-pushed the push-uzswprnmqzky branch from dfcd8d2 to 1e4fe75 Compare August 17, 2025 15:58
@Defelo Defelo mentioned this pull request Aug 17, 2025
13 tasks
@Defelo Defelo requested a review from matthiasbeyer August 17, 2025 16:07
Copy link
Copy Markdown
Contributor

@matthiasbeyer matthiasbeyer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only a small documentation suggestion, everything else looks fine to me.

Do we need docs somewhere for this? Probably?

@lorenzleutgeb
Copy link
Copy Markdown
Member

lorenzleutgeb commented Aug 17, 2025

I'd appreciate if you could wait for my review here. The reason is that Radicle 1.3 now supports Canonical References, which means that one will not have to access tags via a particular namespace anymore in the future. In the next weeks, probably all of the Radicle-hosted packages will switch to Canonical References.

However, such a fetcher should still allow fetching from a particular namespace, in my opinion.

I would like to read through the suggested changes here with a fresh brain, and the implications of Canonical References in mind.

@lorenzleutgeb
Copy link
Copy Markdown
Member

I have implemented something similar before, just for reference: https://github.com/radicle-nix/radicle-nix/blob/main/pkg/fetchFromRadicleBridge/package.nix

Fetching via radicle-httpd as is done here isn't a true fetchFromRadicle in my opinion. That's why I called it "bridge".

What would deserve the name fetchFromRadicle IMO is something like this: https://github.com/radicle-nix/radicle-nix/blob/main/pkg/fetchFromRadicle/package.nix

Copy link
Copy Markdown
Member

@lorenzleutgeb lorenzleutgeb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, from what I understand, just omitting the node argument would work nicely with Canonical References. I just made a minor suggestion.

Please think about the name once more, maybe this should be called something like:

  • fetchFromRadicleHttpd (for now, radicle-httpd is really the only server compatible with this)
  • fetchFromRadicleBridge as we're using the given seed as a "bridge" to the Radicle network. Maybe even rename the seed argument to bridge.
  • fetchFromRadicleHttp
  • fetchFromRadicleSeed as this will rely on a particular seed being available.

@Defelo Defelo force-pushed the push-uzswprnmqzky branch from 1e4fe75 to 77f26b6 Compare August 17, 2025 23:13
@Defelo
Copy link
Copy Markdown
Member Author

Defelo commented Aug 17, 2025

Hey @lorenzleutgeb, thanks for the feedback! I applied the change you suggested and removed the default value for seed.

Regarding the name, let's assume we eventually want to add another implementation which starts a node itself and thus doesn't require a seed. I think we could simply use the same fetchFromRadicle fetcher in both cases: If the seed parameter is set, the fetchgit implementation is used to fetch the repo via the radicle-httpd instance on the specified seed. And if seed is null (which could also be the default value), the implementation using radicle-node is used. This would be similar to what other fetchers like fetchFromGitHub do, which use different implementations (fetchgit vs. fetchzip) depending on their arguments. That's just an idea though, I will of course change the name of this fetcher implementation, if we decide not to do that.

@Defelo Defelo force-pushed the push-uzswprnmqzky branch from 77f26b6 to 9c9cfe8 Compare August 18, 2025 00:11
@Defelo
Copy link
Copy Markdown
Member Author

Defelo commented Aug 18, 2025

Do we need docs somewhere for this? Probably?

Added some documentation to the fetchers section

@nixpkgs-ci nixpkgs-ci bot added 8.has: documentation This PR adds or changes documentation and removed 10.rebuild-darwin: 1 This PR causes 1 package to rebuild on Darwin. 10.rebuild-linux: 1 This PR causes 1 package to rebuild on Linux. labels Aug 18, 2025
@nixpkgs-ci nixpkgs-ci bot added 12.approvals: 1 This PR was reviewed and approved by one person. 12.approved-by: package-maintainer This PR was reviewed and approved by a maintainer listed in any of the changed packages. labels Aug 18, 2025
@Defelo Defelo requested a review from matthiasbeyer August 18, 2025 06:43
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the .git suffix always true?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Copy Markdown
Contributor

@matthiasbeyer matthiasbeyer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Besides the one (non-blocking) comment, 👍

@nixpkgs-ci nixpkgs-ci bot added 12.approvals: 2 This PR was reviewed and approved by two persons. and removed 12.approvals: 1 This PR was reviewed and approved by one person. labels Aug 18, 2025
@Defelo Defelo force-pushed the push-uzswprnmqzky branch from 9c9cfe8 to 74dd8c4 Compare August 19, 2025 14:15
@Defelo
Copy link
Copy Markdown
Member Author

Defelo commented Aug 19, 2025

Rebased onto master, changed radicle-tui to use fetchFromRadicle, and simplified its update script.

I checked all FODs again using nix build -f. radicle-node.src radicle-httpd.src radicle-explorer.src radicle-tui.src --rebuild and tested radicle-tui's update script again.

Are there any blockers remaining?

@Defelo Defelo force-pushed the push-uzswprnmqzky branch from 74dd8c4 to 9fbba6d Compare August 19, 2025 14:29
@SuperSandro2000 SuperSandro2000 merged commit 39f09b0 into NixOS:master Aug 21, 2025
26 of 28 checks passed
@nixpkgs-ci
Copy link
Copy Markdown
Contributor

nixpkgs-ci bot commented Aug 21, 2025

Backport failed for release-25.05, because it was unable to cherry-pick the commit(s).

Please cherry-pick the changes locally and resolve any conflicts.

git fetch origin release-25.05
git worktree add -d .worktree/backport-434360-to-release-25.05 origin/release-25.05
cd .worktree/backport-434360-to-release-25.05
git switch --create backport-434360-to-release-25.05
git cherry-pick -x 5e2cc452717a454ca3517caf85eba3ede38d71f3 0f4642c7961dfc1348b607f47ff8c9a8cf6b3656 036be6edbae5f37143def9dba764ceb8b387b5e5 1ddbbb16715d268ff003ab7b11821cd04bb0b4e2 914c4f23461c546d580675742b0c5bd4f04b4796

@Defelo Defelo deleted the push-uzswprnmqzky branch August 22, 2025 05:35
@mdaniels5757 mdaniels5757 added 8.has: port to stable This PR already has a backport to the stable release. and removed backport release-25.05 labels Aug 30, 2025
@Defelo Defelo mentioned this pull request Mar 1, 2026
13 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

6.topic: fetch Fetchers (e.g. fetchgit, fetchsvn, ...) 8.has: documentation This PR adds or changes documentation 8.has: port to stable This PR already has a backport to the stable release. 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 12.approvals: 2 This PR was reviewed and approved by two persons. 12.approved-by: package-maintainer This PR was reviewed and approved by a maintainer listed in any of the changed packages.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants