-
Notifications
You must be signed in to change notification settings - Fork 98
fix(nix): hopli aarch64 build working and add man-page generation #7310
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.
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
Conversation
📝 WalkthroughWalkthroughThe changes update the Nix build configuration and documentation to add automated generation of man pages for the Changes
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
Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Suggested reviewers
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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)
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed 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)
Other keywords and placeholders
Documentation and Community
|
There was a problem hiding this 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 |
There was a problem hiding this 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
…sh is relative and does not rely on the CI runner absolute path
There was a problem hiding this 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
There was a problem hiding this 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 usingexpect()
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 usingexpect()
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
⛔ 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 fromfutures::executor::block_on
totokio::runtime::Runtime::new()?.block_on
in the build script.
There was a problem hiding this 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
Close #7287
Close #7256