Skip to content

clients/reth: improve version parsing in dockerfiles #1272

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

danceratopz
Copy link
Member

Hiveview reports reth's version without a commit hash: reth Version: 1.3.8.

This PR adds the build's commit hash: 1.3.8+52c3e3cc.

Try it out with the full output of reth --version:

echo "reth Version: 1.3.8
  Commit SHA: df6d5dd1dd23da180ecd2e15585b3bebc3b5c656
  Build Timestamp: 2025-04-14T08:43:20.271808115Z
  Build Features: jemalloc
  Build Profile: release" | head -2 | awk 'NR==1 {gsub("reth Version: ", ""); version=$0} NR==2 {gsub("Commit SHA: ", ""); sha=substr($0, 1, 8)} END {print version"+"sha}'

It's more brittle, but I don't think unexpected output would prevent the image getting built.

These checks should be outsourced to a helper library that we can independently unit test.

@fjl
Copy link
Collaborator

fjl commented Apr 14, 2025

These checks should be outsourced to a helper library that we can independently unit test.

I kind of disagree on the helper library. This kind of stuff is why we have hive in the first place, to paper over differences in the clients. Where else would we use this library then?

@danceratopz
Copy link
Member Author

These checks should be outsourced to a helper library that we can independently unit test.

I kind of disagree on the helper library. This kind of stuff is why we have hive in the first place, to paper over differences in the clients. Where else would we use this library then?

Sorry for the lack of explanation. I didn't want to take the logic out of hive, just out of the dockerfiles. I would suggest adding a clients/scripts/ directory with this logic. Then we could:

ADD scripts/ /scripts/

This would allow functions added to this library to be "unit" tested (and reduce code duplication across 3 dockerfiles).

@fjl
Copy link
Collaborator

fjl commented Apr 14, 2025

ahh, right, that makes sense!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants