Skip to content

Conversation

tolbrino
Copy link
Contributor

@tolbrino tolbrino commented Jul 15, 2025

Close #7287
Close #7256

  • remove armv7l cross-compilation target
  • update nixpkgs to most recent release 25.05
  • update darwin sdk to most recent sdk, removing use of specific frameworks

@tolbrino tolbrino added this to the 3.0.0 milestone Jul 15, 2025
@tolbrino tolbrino self-assigned this Jul 15, 2025
Copy link
Contributor

coderabbitai bot commented Jul 15, 2025

📝 Walkthrough

Walkthrough

The changes update the Nix build configuration and documentation to add automated generation of man pages for the hoprd and hopli binaries. Support for the armv7l-linux target is removed from the build system. Documentation is updated with instructions for building and installing the new man pages.

Changes

File(s) Change Summary
README.md Added instructions for building, viewing, and installing hoprd and hopli man pages via Nix.
flake.nix Updated nixpkgs input, removed armv7l-linux support, added man page derivations for hoprd and hopli.
nix/man-pages.nix New file: defines logic to generate compressed man pages for hoprd and hopli using help2man.
nix/rust-builder.nix Refactored environment variable configuration for cross-compilation; removed armv7l special case.
nix/rust-package.nix Simplified Darwin build inputs to use apple-sdk_15 package.
sdk/python/localcluster/anvil.py Adjusted string quoting style in command construction (single to double quotes).
sdk/python/localcluster/cluster.py Added spaces in f-string expressions for directory paths.
sdk/python/localcluster/snapshot.py Added spaces in f-string expressions for directory paths.
tests/test_stress.py Changed string literals from single to double quotes for consistency.
tests/utils.py Added spaces in f-string expressions for log messages.
Cargo.toml Modified dependencies to disable default features and enable specific features for sea-orm and sqlx.
db/entity/Cargo.toml Changed tokio from optional to mandatory build dependency; removed explicit "tokio" feature.
db/entity/build.rs Replaced futures::executor::block_on with Tokio runtime's block_on for async command execution.

Sequence Diagram(s)

sequenceDiagram
    participant Developer
    participant Nix
    participant help2man
    participant ManPage

    Developer->>Nix: nix build .#hoprd-man
    Nix->>help2man: Run help2man on hoprd binary
    help2man->>ManPage: Generate hoprd.1.gz
    Nix->>Developer: Output man page derivation

    Developer->>Nix: nix build .#hopli-man
    Nix->>help2man: Run help2man on hopli binary
    help2man->>ManPage: Generate hopli.1.gz
    Nix->>Developer: Output man page derivation
Loading

Assessment against linked issues

Objective Addressed Explanation
Generate hoprd and hopli man pages (#7287)
Provide Nix build recipe for man pages (#7287)
Remove/disable armv7l-linux build for hopli and hoprd (#7256)
Fix OpenSSL linking for aarch64/armv7l cross-compilation (#7256) Armv7l support is removed, not fixed; aarch64-linux build is not addressed in this PR.

Assessment against linked issues: Out-of-scope changes

Code Change Explanation
String formatting and quoting style changes in Python files (sdk/python/localcluster/, tests/) These are unrelated to the objectives of man page generation or armv7l/aarch64 build configuration.
Dependency feature modifications in Cargo.toml and db/entity/Cargo.toml These changes relate to Rust dependency features and async runtime usage, not linked to the stated issues.
Async runtime change in db/entity/build.rs Changing from futures executor to Tokio runtime is unrelated to man page generation or cross-compilation fixes.

Suggested reviewers

  • tolbrino
  • Teebor-Choka

📜 Recent review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 602591e and 3e9e5d0.

📒 Files selected for processing (1)
  • Cargo.toml (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • Cargo.toml
⏰ 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). (10)
  • GitHub Check: tests-unit
  • GitHub Check: Build Candidate Binaries (hopli)
  • GitHub Check: Build Candidate Binaries (hoprd)
  • GitHub Check: tests-unit-nightly
  • GitHub Check: hoprd / docker
  • GitHub Check: hopli / docker
  • GitHub Check: Lint / Lint
  • GitHub Check: Audit / Audit
  • GitHub Check: Zizmor / Zizmor
  • GitHub Check: Pre-commit / Pre-commit

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@github-actions github-actions bot added toolchain Developer and product happiness testing Related to tests labels Jul 15, 2025
@ausias-armesto ausias-armesto modified the milestones: 3.0.0, 3.0.0-temporary Jul 15, 2025
@tolbrino tolbrino requested a review from a team July 15, 2025 17:38
@tolbrino tolbrino marked this pull request as ready for review July 15, 2025 17:38
@Copilot Copilot AI review requested due to automatic review settings July 15, 2025 17:38
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes the aarch64 build for hopli and adds man-page generation capabilities. The changes address build issues with ARM64 architecture support and introduce documentation generation for the HOPR tools.

  • Removes armv7l-linux build support and simplifies cross-compilation configuration
  • Updates nixpkgs to release-25.05 and modernizes Apple SDK usage for Darwin builds
  • Adds automated man page generation for hoprd and hopli binaries using help2man
  • Standardizes Python string formatting to use double quotes consistently

Reviewed Changes

Copilot reviewed 7 out of 11 changed files in this pull request and generated no comments.

Show a summary per file
File Description
tests/test_stress.py Standardizes string quotes from single to double quotes
sdk/python/localcluster/anvil.py Converts f-string quotes to double quotes for consistency
nix/rust-package.nix Modernizes Darwin SDK usage by switching to apple-sdk_15
nix/rust-builder.nix Improves cross-compilation support and removes armv7l target handling
nix/man-pages.nix Adds new module for generating man pages using help2man
flake.nix Updates nixpkgs version, removes armv7l builds, and integrates man page generation
README.md Documents how to build and use the new man pages

@ausias-armesto ausias-armesto added the binary:aarch64-linux Build aarch64-linux binary label Jul 15, 2025
Copy link
Contributor

@ausias-armesto ausias-armesto left a comment

Choose a reason for hiding this comment

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

I added the label binary:aarch64-linux, and you need to check that the build binary works on the CI

Copy link
Contributor

@ausias-armesto ausias-armesto left a comment

Choose a reason for hiding this comment

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

I will keep the PR open, because I want to integrate in this PR the generation of the manual in the OS package.
I will be doing commits

@tolbrino tolbrino changed the title Fix hopli aarch64 build and add man-page generation fix(nix): hopli aarch64 build working and add man-page generation Jul 16, 2025
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (3)
db/entity/build.rs (3)

99-120: Consider using expect() for clearer error messages.

The migration to Tokio runtime is correct and consistent with the Cargo.toml changes. However, consider using expect() with a descriptive message for better error handling.

-    tokio::runtime::Runtime::new()?.block_on(execute_sea_orm_cli_command([
+    tokio::runtime::Runtime::new()
+        .expect("The Tokio runtime must be buildable")
+        .block_on(execute_sea_orm_cli_command([

122-137: Consider using expect() for clearer error messages.

Same suggestion as the previous call - using expect() would provide a clearer error message if runtime creation fails.

-    tokio::runtime::Runtime::new()?.block_on(execute_sea_orm_cli_command([
+    tokio::runtime::Runtime::new()
+        .expect("The Tokio runtime must be buildable")
+        .block_on(execute_sea_orm_cli_command([

66-140: Consider reusing the Tokio runtime for efficiency.

While the current approach works fine for a build script, creating a new runtime for each call could be optimized by reusing a single runtime instance.

 fn main() -> anyhow::Result<()> {
+    let rt = tokio::runtime::Runtime::new()
+        .expect("The Tokio runtime must be buildable");
+
     let cargo_manifest_dir = &env::var("CARGO_MANIFEST_DIR").context("should point to a valid manifest dir")?;
     // ... existing code ...
     
-    tokio::runtime::Runtime::new()?.block_on(execute_sea_orm_cli_command([
+    rt.block_on(execute_sea_orm_cli_command([
         // ... migrate command args ...
     ]))?;

-    tokio::runtime::Runtime::new()?.block_on(execute_sea_orm_cli_command([
+    rt.block_on(execute_sea_orm_cli_command([
         // ... generate command args ...
     ]))?;
📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f0ab12c and 481c887.

⛔ Files ignored due to path filters (1)
  • Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (5)
  • Cargo.toml (2 hunks)
  • db/entity/Cargo.toml (1 hunks)
  • db/entity/build.rs (2 hunks)
  • flake.nix (5 hunks)
  • nix/rust-builder.nix (2 hunks)
✅ Files skipped from review due to trivial changes (1)
  • Cargo.toml
🚧 Files skipped from review as they are similar to previous changes (2)
  • flake.nix
  • nix/rust-builder.nix
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: NumberFour8
PR: hoprnet/hoprnet#6630
File: crypto/packet/src/por.rs:84-84
Timestamp: 2024-11-13T17:56:37.536Z
Learning: In PR #6630, the changes include the removal of ticket validation from the last hop and fixes for Index Offset handling as specified in the PR objectives.
Learnt from: ausias-armesto
PR: hoprnet/hoprnet#7312
File: docs/changelog/changelog-3.0.0-rc.1.md:39-46
Timestamp: 2025-07-15T15:28:59.124Z
Learning: The changelog files in the hoprnet/hoprnet project are auto-generated from PRs, so manual editing suggestions for deduplication are not appropriate. Any improvements to prevent duplicate entries would need to be handled at the generation level.
Learnt from: NumberFour8
PR: hoprnet/hoprnet#6244
File: transport/protocol/src/ack/processor.rs:31-32
Timestamp: 2024-07-28T07:26:06.634Z
Learning: The GitHub issue to standardize the usage of `METRIC_TICKETS_COUNT` across the codebase was successfully created and can be tracked at https://github.com/hoprnet/hoprnet/issues/6245.
Learnt from: NumberFour8
PR: hoprnet/hoprnet#6244
File: transport/protocol/src/ack/processor.rs:31-32
Timestamp: 2024-10-09T06:16:18.170Z
Learning: The GitHub issue to standardize the usage of `METRIC_TICKETS_COUNT` across the codebase was successfully created and can be tracked at https://github.com/hoprnet/hoprnet/issues/6245.
Learnt from: ausias-armesto
PR: hoprnet/hoprnet#7251
File: .github/workflows/package.yaml:66-68
Timestamp: 2025-06-19T10:10:00.368Z
Learning: In the HOPR packaging workflow (.github/workflows/package.yaml), the `just package` command creates a single distribution package that includes all binaries (hoprd and hopli) for the target architecture. The workflow downloads both hoprd and hopli artifacts, extracts them to ./dist/bin, and nfpm packages both binaries together into one .deb, .rpm, or .apk file per target.
Learnt from: Teebor-Choka
PR: hoprnet/hoprnet#6545
File: transport/p2p/src/swarm.rs:491-494
Timestamp: 2024-10-11T10:43:34.869Z
Learning: When reviewing revert pull requests in the `hoprnet/hoprnet` repository, avoid making code recommendations or suggestions.
Learnt from: ausias-armesto
PR: hoprnet/hoprnet#7251
File: .github/workflows/package.yaml:0-0
Timestamp: 2025-06-19T11:37:27.410Z
Learning: In the HOPR packaging workflow (.github/workflows/package.yaml), both hoprd and hopli binaries are downloaded and used together in a single package creation process. The `just package` command creates one distribution package (.deb, .rpm, or .apk) per target that contains both binaries, which is why the workflow only uploads one package per format rather than separate packages for each binary.
Learnt from: ausias-armesto
PR: hoprnet/hoprnet#7297
File: deploy/Formula/hoprd.rb:427-427
Timestamp: 2025-07-10T11:37:58.562Z
Learning: In the HOPR Homebrew formula (deploy/Formula/hoprd.rb), the API endpoint display intentionally uses HOPRD_HOST (which includes the P2P address and port) rather than HOPRD_API_HOST in the installation summary, as confirmed by maintainer ausias-armesto.
Learnt from: tolbrino
PR: hoprnet/hoprnet#7070
File: nix/testShell.nix:22-23
Timestamp: 2025-04-15T19:24:23.570Z
Learning: In the HOPR project, the `autoPatchelf` command from Nix can handle the case where the target directory (like ./.venv) doesn't exist, so no additional directory existence check is needed before running it.
Learnt from: Teebor-Choka
PR: hoprnet/hoprnet#6972
File: .github/workflows/audit-pr.yaml:25-32
Timestamp: 2025-04-02T11:36:56.281Z
Learning: For HOPR projects, auditing tools like cargo-audit should be managed through the Nix environment rather than installed separately to ensure consistency and reproducibility.
db/entity/Cargo.toml (14)
Learnt from: tolbrino
PR: hoprnet/hoprnet#6399
File: chain/api/Cargo.toml:50-53
Timestamp: 2024-11-01T15:26:53.058Z
Learning: In `chain/api/Cargo.toml`, it's acceptable to declare the same dependency `hopr-db-sql` in both `[dependencies]` and `[dev-dependencies]` with different features, especially when additional features are only needed for tests.
Learnt from: Teebor-Choka
PR: hoprnet/hoprnet#6509
File: db/entity/src/conversions/tickets.rs:71-71
Timestamp: 2024-10-09T06:16:18.170Z
Learning: When the schema generation code is updated via cargo dependency changes, it may handle schema changes such as field type updates automatically.
Learnt from: Teebor-Choka
PR: hoprnet/hoprnet#6509
File: db/entity/src/conversions/tickets.rs:71-71
Timestamp: 2024-09-25T19:27:26.293Z
Learning: When the schema generation code is updated via cargo dependency changes, it may handle schema changes such as field type updates automatically.
Learnt from: tolbrino
PR: hoprnet/hoprnet#6399
File: common/primitive-types/Cargo.toml:24-24
Timestamp: 2024-11-01T09:32:55.651Z
Learning: In the hoprnet project, dependency versions are defined in the workspace Cargo.toml, so it's not necessary to specify them in individual package Cargo.toml files.
Learnt from: Teebor-Choka
PR: hoprnet/hoprnet#6886
File: nix/rust-package.nix:84-84
Timestamp: 2025-02-25T00:35:33.437Z
Learning: When transitioning from vendored dependencies to a private proxy registry in Rust projects, GitHub Actions workflows need to be refactored to accommodate the new update strategy, particularly workflows handling dependency updates like renovate-cargo-update.yaml.
Learnt from: Teebor-Choka
PR: hoprnet/hoprnet#6673
File: hoprd/hoprd/src/main.rs:164-165
Timestamp: 2024-11-25T13:44:06.729Z
Learning: When initializing the Tokio runtime in the `main` function of `hoprd/hoprd/src/main.rs`, prefer using `expect("The Tokio runtime must be buildable")` to provide a clear error message instead of propagating the error with `?` or using `.unwrap()`.
Learnt from: tolbrino
PR: hoprnet/hoprnet#6399
File: db/entity/Cargo.toml:4-4
Timestamp: 2024-10-21T14:58:45.665Z
Learning: In the HOPR project, crate versions are intentionally kept separate and do not need to be aligned across related packages.
Learnt from: Teebor-Choka
PR: hoprnet/hoprnet#6508
File: rustfmt.toml:6-6
Timestamp: 2024-10-09T06:16:18.170Z
Learning: The directory `ethereum/bindings/src/codegen` is generated and may not exist in the project structure but should still be included in the `ignore` directive in `rustfmt.toml` to prevent formatting of generated code.
Learnt from: Teebor-Choka
PR: hoprnet/hoprnet#6508
File: rustfmt.toml:6-6
Timestamp: 2024-09-25T14:16:43.307Z
Learning: The directory `ethereum/bindings/src/codegen` is generated and may not exist in the project structure but should still be included in the `ignore` directive in `rustfmt.toml` to prevent formatting of generated code.
Learnt from: Teebor-Choka
PR: hoprnet/hoprnet#6345
File: hopr-socks/hopr-socks-server/Cargo.toml:20-20
Timestamp: 2024-06-21T20:51:35.871Z
Learning: All code and packages in the `hoprnet` project should use the workspace level dependency of `reqwest`.
Learnt from: tolbrino
PR: hoprnet/hoprnet#6399
File: db/migration/src/m20240930_000017_logs_create_log.rs:112-113
Timestamp: 2024-11-01T09:32:53.909Z
Learning: In our Rust SeaORM database migrations, we use binary types (e.g., `binary_len(8)`) for index columns such as `TransactionIndex`, `LogIndex`, and `BlockNumber` to maintain consistency across models.
Learnt from: NumberFour8
PR: hoprnet/hoprnet#6511
File: Cargo.toml:77-77
Timestamp: 2024-09-28T21:05:38.392Z
Learning: The `float-cmp` crate is used in the `hopr-primitive-types` crate.
Learnt from: NumberFour8
PR: hoprnet/hoprnet#6511
File: Cargo.toml:77-77
Timestamp: 2024-10-09T06:16:18.170Z
Learning: The `float-cmp` crate is used in the `hopr-primitive-types` crate.
Learnt from: Teebor-Choka
PR: hoprnet/hoprnet#6972
File: .github/workflows/audit-pr.yaml:25-32
Timestamp: 2025-04-02T11:36:56.281Z
Learning: For HOPR projects, auditing tools like cargo-audit should be managed through the Nix environment rather than installed separately to ensure consistency and reproducibility.
db/entity/build.rs (5)
Learnt from: Teebor-Choka
PR: hoprnet/hoprnet#6673
File: hoprd/hoprd/src/main.rs:164-165
Timestamp: 2024-11-25T13:44:06.729Z
Learning: When initializing the Tokio runtime in the `main` function of `hoprd/hoprd/src/main.rs`, prefer using `expect("The Tokio runtime must be buildable")` to provide a clear error message instead of propagating the error with `?` or using `.unwrap()`.
Learnt from: Teebor-Choka
PR: hoprnet/hoprnet#6509
File: db/entity/src/conversions/tickets.rs:71-71
Timestamp: 2024-10-09T06:16:18.170Z
Learning: When the schema generation code is updated via cargo dependency changes, it may handle schema changes such as field type updates automatically.
Learnt from: Teebor-Choka
PR: hoprnet/hoprnet#6509
File: db/entity/src/conversions/tickets.rs:71-71
Timestamp: 2024-09-25T19:27:26.293Z
Learning: When the schema generation code is updated via cargo dependency changes, it may handle schema changes such as field type updates automatically.
Learnt from: Teebor-Choka
PR: hoprnet/hoprnet#6230
File: chain/indexer/src/block.rs:158-161
Timestamp: 2024-10-09T06:16:18.170Z
Learning: The `ethers-rs` library used in the `block.rs` file already implements a retry mechanism for fetching block numbers from the RPC interface. This information was provided by the user Teebor-Choka.
Learnt from: Teebor-Choka
PR: hoprnet/hoprnet#6230
File: chain/indexer/src/block.rs:158-161
Timestamp: 2024-07-28T07:26:06.634Z
Learning: The `ethers-rs` library used in the `block.rs` file already implements a retry mechanism for fetching block numbers from the RPC interface. This information was provided by the user Teebor-Choka.
⏰ 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). (8)
  • GitHub Check: Build Candidate Binaries (hopli)
  • GitHub Check: tests-unit-nightly
  • GitHub Check: hoprd / docker
  • GitHub Check: hopli / docker
  • GitHub Check: Zizmor / Zizmor
  • GitHub Check: Lint / Lint
  • GitHub Check: Pre-commit / Pre-commit
  • GitHub Check: Audit / Audit
🔇 Additional comments (1)
db/entity/Cargo.toml (1)

26-26: LGTM! Dependency change aligns with build script migration.

Making tokio a mandatory build dependency is consistent with the migration from futures::executor::block_on to tokio::runtime::Runtime::new()?.block_on in the build script.

@tolbrino tolbrino requested a review from a team July 16, 2025 12:26
@github-actions github-actions bot added dependencies Pull requests that update a dependency file crate:hopr-db-entity labels Jul 16, 2025
Copy link
Contributor

@ausias-armesto ausias-armesto left a comment

Choose a reason for hiding this comment

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

I will add the manuals in a different PR

@tolbrino tolbrino enabled auto-merge (squash) July 16, 2025 12:46
@ausias-armesto ausias-armesto removed the binary:aarch64-linux Build aarch64-linux binary label Jul 16, 2025
@tolbrino tolbrino merged commit 6dcbb00 into master Jul 16, 2025
31 checks passed
@tolbrino tolbrino deleted the tb/202507-hopli branch July 16, 2025 13:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
crate:hopr-db-entity dependencies Pull requests that update a dependency file testing Related to tests toolchain Developer and product happiness
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Generate hoprd and hopli man pages Unable to compile hopli-aarch64-linux and hopli-armv7l-linux
4 participants