-
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
Open
ximinez
wants to merge
4
commits into
XRPLF:develop
Choose a base branch
from
ximinez:pr/gch
base: develop
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
+55
−6
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* 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.
ximinez
added
API Change
Build System
Bug
Tech Debt
Non-urgent improvements
Documentation
README changes, code comments, etc.
Perf impact not expected
Change is not expected to improve nor harm performance.
labels
Dec 17, 2024
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #5225 +/- ##
=========================================
- Coverage 78.0% 78.0% -0.0%
=========================================
Files 789 789
Lines 67007 67012 +5
Branches 8110 8112 +2
=========================================
+ Hits 52274 52275 +1
- Misses 14733 14737 +4
|
* 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)
legleux
approved these changes
Jan 14, 2025
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)
vinaychandradixit
approved these changes
Jan 26, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
API Change
Bug
Build System
Documentation
README changes, code comments, etc.
Perf impact not expected
Change is not expected to improve nor harm performance.
Tech Debt
Non-urgent improvements
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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: