Add bitcoind to eget_packages and update installation command - #46
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
Reviewer's guide (collapsed on small PRs)Reviewer's GuideThe PR integrates Bitcoin Core's bitcoind into the eget package installation flow, using architecture-specific upstream archives and a configurable bitcoin_version variable, while enabling it in the example package configuration. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
⛔ Workflow Failures DetectedThe following job errors and annotations were found:
🛠️ Run Links |
There was a problem hiding this comment.
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path=".ansible/playbooks/tasks/eget.yml" line_range="89-92" />
<code_context>
+ bitcoind: >-
+ --file '*/bin/bitcoind' --to bitcoind
+ https://bitcoincore.org/bin/bitcoin-core-{{ bitcoin_version | default('31.1') }}/bitcoin-{{ bitcoin_version | default('31.1') }}-{{
+ 'x86_64' if eget_arch == 'x86_64'
+ else 'aarch64' if eget_arch == 'arm64'
+ else 'arm'
+ }}-linux-gnu.tar.gz
btop: "-a btop-{{ eget_plain_arch }}-unknown-linux-musl.tar.gz aristocratos/btop"
</code_context>
<issue_to_address>
**issue (bug_risk):** On 32-bit ARM hosts, the URL is built with the `arm-linux-gnu` suffix, but Bitcoin Core's ARM archive uses `arm-linux-gnueabihf.tar.gz`; `eget` therefore receives a nonexistent URL and bitcoind installation fails.
**Triggers:** When `ansible_facts['architecture']` contains `arm` but is not `x86_64` or `aarch64`.
**Suggested fix:** Map the 32-bit ARM case to `arm-linux-gnueabihf` rather than appending the generic `-linux-gnu` suffix.
```suggestion
'x86_64-linux-gnu' if eget_arch == 'x86_64'
else 'aarch64-linux-gnu' if eget_arch == 'arm64'
else 'arm-linux-gnueabihf'
}}.tar.gz
```
</issue_to_address>Sourcery assessment
Needs a human reviewer. 1 finding to address first, and a merge can install and potentially run a new bitcoind binary from an external URL, and a bad version, archive, or architecture choice could leave behind software and downloaded node data after the configuration is reverted. The impact is bounded and repairable by removing the package/data or rerunning installation with the correct version.
Blocking findings: .ansible/playbooks/tasks/eget.yml:92
| 'x86_64' if eget_arch == 'x86_64' | ||
| else 'aarch64' if eget_arch == 'arm64' | ||
| else 'arm' | ||
| }}-linux-gnu.tar.gz |
There was a problem hiding this comment.
issue (bug_risk): On 32-bit ARM hosts, the URL is built with the arm-linux-gnu suffix, but Bitcoin Core's ARM archive uses arm-linux-gnueabihf.tar.gz; eget therefore receives a nonexistent URL and bitcoind installation fails.
Triggers: When ansible_facts['architecture'] contains arm but is not x86_64 or aarch64.
Suggested fix: Map the 32-bit ARM case to arm-linux-gnueabihf rather than appending the generic -linux-gnu suffix.
| 'x86_64' if eget_arch == 'x86_64' | |
| else 'aarch64' if eget_arch == 'arm64' | |
| else 'arm' | |
| }}-linux-gnu.tar.gz | |
| 'x86_64-linux-gnu' if eget_arch == 'x86_64' | |
| else 'aarch64-linux-gnu' if eget_arch == 'arm64' | |
| else 'arm-linux-gnueabihf' | |
| }}.tar.gz |
Summary by Sourcery
New Features: