Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Radicle-cli: 0.6.1 -> 1.0.0-rc8 #301511

Closed
wants to merge 3 commits into from
Closed

Radicle-cli: 0.6.1 -> 1.0.0-rc8 #301511

wants to merge 3 commits into from

Conversation

roblabla
Copy link
Contributor

@roblabla roblabla commented Apr 4, 2024

Description of changes

Updates radicle-cli to the latest version.

I unfortunately had to disable tests, as nearly half of them failed in hard to debug ways.

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 24.05 Release Notes (or backporting 23.05 and 23.11 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@ofborg ofborg bot requested a review from amesgen April 4, 2024 11:33
@amesgen
Copy link
Member

amesgen commented Apr 4, 2024

FTR: #244417 (comment)

@roblabla
Copy link
Contributor Author

roblabla commented Apr 4, 2024

FTR: #244417 (comment)

I'm not sure why this old MR got closed. Version 0.9 is compatible with the current network from my minimal testing. Although radicle's installation script installs 1.0.0-rc2 that version requires Rust 1.77, which hasn't been packaged in nixpkgs yet, so I held off from it.

If we want the latest rc version, we can hold off until #298340 lands, and I can update this MR to package 1.0.0-rc2.

@koalalorenzo
Copy link

Do we have any updates on this? Will it make the cut for 24.05?

@amesgen
Copy link
Member

amesgen commented May 4, 2024

I think updating this to the latest rc version as mentioned by @roblabla seems fine.

@roblabla roblabla changed the title Radicle-cli: 0.6.1 -> 0.9.0 Radicle-cli: 0.6.1 -> 1.0.0-rc8 May 4, 2024
@roblabla
Copy link
Contributor Author

roblabla commented May 4, 2024 •
edited
Loading

I updated the MR to use 1.0.0-rc8. Upstream also deprecated their github-based mirror (https://github.com/radicle-dev/heartwood is archived), so I changed it to fetchgit from radicle's official repository (rad:z3gqcJUoA1n9HaHKufZs5FCSGazv5) on radicle's official seed (seed.radicle.xyz)

@amesgen amesgen mentioned this pull request May 5, 2024
Merged
13 tasks
lorenzleutgeb
lorenzleutgeb suggested changes May 5, 2024
View reviewed changes
Copy link
Member

@lorenzleutgeb lorenzleutgeb left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Just was pointed at this by @amesgen, and indeed I think it's a pity that I duplicated your work. Sorry about that. However, please check out #309050 (comment) and let's agree on how to proceed. I am happy to first PR against roblabla:radicle-cli-0.9.0.

pkgs/applications/version-management/radicle-cli/default.nix
@@ -18,28 +18,15 @@

rustPlatform.buildRustPackage rec {
pname = "radicle-cli";
version = "0.6.1";
version = "1.0.0-rc8";
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
version = "1.0.0-rc8";
version = "1.0.0-rc.8";

For consistency with upstream.

@lorenzleutgeb
Copy link
Member

lorenzleutgeb commented May 5, 2024 •
edited
Loading

Binaries produced by this derivation do not report the version like upstream expects (e.g. when reporting a bug).

$ ./result/bin/rad version
rad pre-release (unknown)
$ ./result/bin/rad debug | jq -r '.radVersion'
pre-release

My conflicting PR has a fix for this, setting RADICLE_VERSION.

See also ofborg failure: https://github.com/NixOS/nixpkgs/pull/301511/checks?check_run_id=24594513497

lorenzleutgeb
lorenzleutgeb reviewed May 5, 2024
View reviewed changes
pkgs/applications/version-management/radicle-cli/default.nix

# Otherwise, there are errors due to the `abigen` macro from `ethers`.
auditable = false;
cargoSha256 = "sha256-Ej9cxuHxiQDwCXOwcIyfCau9Thhz+hf3IevN6F4rDjM=";

nativeBuildInputs = [
Copy link
Member

Choose a reason for hiding this comment

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

Did you verify that all nativeBuildInputs are still required? I believe that this will build fine without CMake, see #309050.

@roblabla
Copy link
Contributor Author

roblabla commented May 5, 2024

Your MR looks better, let's close this one and converge on #309050

@roblabla roblabla closed this May 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lorenzleutgeb lorenzleutgeb lorenzleutgeb requested changes

@amesgen amesgen Awaiting requested review from amesgen

Assignees
No one assigned
Labels
10.rebuild-darwin: 1-10 10.rebuild-darwin: 1 10.rebuild-linux: 1-10 10.rebuild-linux: 1
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@roblabla @amesgen @koalalorenzo @lorenzleutgeb