-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Improve git commit hash lookup #5225
Conversation
* Also get the branch name. * Use rev-parse instead of describe to get a clean hash. * Return the git hash and branch name in server_info for admin connections. * Include git hash and branch name on separate lines in --version.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #5225 +/- ##
=======================================
Coverage 78.1% 78.1%
=======================================
Files 790 790
Lines 67556 67562 +6
Branches 8158 8159 +1
=======================================
+ Hits 52785 52790 +5
- Misses 14771 14772 +1
|
* upstream/develop: chore: add macos dependency installation (5233) prefix Uint384 and Uint512 with Hash in server_definitions (5231) refactor: add `rpcName` to `LEDGER_ENTRY` macro (5202)
* upstream/develop: chore: update deprecated Github Actions (5241) XLS-46: DynamicNFT (5048)
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 love it
* upstream/develop: chore: Update Visual Studio CI to VS 2022, and add VS Debug builds (5240) Add [validator_list_threshold] to validators.txt to improve UNL security (5112) Switch from assert to XRPL_ASSERT (5245) Add missing space character to a log message (5251) Cleanup API-CHANGELOG.md (5207) test: Unit tests to recreate invalid index logic error (5242) Update branch management and merge / release processes (5215) fix: Error consistency in LedgerEntry::parsePermissionedDomains() (5252) Set version to 2.4.0-b2 fix: Use consistent CMake settings for all modules (5228) Fix levelization script to ignore commented includes (5194) Fix the flag processing of NFTokenModify (5246) Fix failing assert in `connect` RPC (5235) Permissioned Domains (XLS-80d) (5161)
* upstream/develop: Fix CI unit tests (5196) Update secp256k1 library to 0.6.0 (5254)
* upstream/develop: Set version to 2.4.0-b3 Set version to 2.3.1 Update conan in the "nix" CI jobs Add RPC "simulate" to execute a dry run of a transaction (5069) Set version to 2.3.1-rc1 Reduce the peer charges for well-behaved peers:
* upstream/develop: Add deep freeze feature (XLS-77d) (5187)
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.
Clio could add these too in the future. No objection from our side.
The commit message from the first commit (plus PR #) is good to use for the merge. |
High Level Overview of Change
Changes the lookup of the git hash included in rippled's version number (for debug builds) to use
rev-parse
instead ofdescribe
, plus a few more improvements.Context of Change
Commit bf013c0 back in 2021 used
git describe
to find the hash of the commit being built when buildingrippled
.From the description of
git describe
:This sometimes results in rippled version numbers that look like:
We want version numbers that look like:
In other words, without
ppe-build-175-g
, indicating that the commit is 175 commits after theppe-build
tag, particularly since that tag is on version 2.1.0-rc1 from February.This changes the lookup to use
git rev-parse
instead, which always just returns a hash.While I was in there, I also added code to find the branch name, and included the hash and branch name in the admin-only result of
server_info
, and the output of--version
, so this info can be found on release builds as well.Type of Change
API Impact
Before / After
rippled --version
Before:
rippled --version
on a debug build may return something likeAfter:
rippled --version
on a debug build will return something likeAnd on a release build will return something like
server_info
Before:
server_info
does not include any git information.After:
server_info
on an admin connection will include git info: