-
-
Notifications
You must be signed in to change notification settings - Fork 8.4k
py/makeversionhdr.py: Always abbreviate Git hashes to same length. #17832
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
base: master
Are you sure you want to change the base?
Conversation
Signed-off-by: Daniël van de Giessen <daniel@dvdgiessen.nl>
523d041
to
8f70ffb
Compare
Code size report:
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #17832 +/- ##
=========================================
Coverage ? 98.38%
=========================================
Files ? 171
Lines ? 22283
Branches ? 0
=========================================
Hits ? 21924
Misses ? 359
Partials ? 0 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Do you need this as part of the production firmware binary , or in CI / testing ? if its just about CI/testing then adding a build_info file with git hash and build / compiler options might be a better solution as that would not affect firmware size. |
I personally don't need this to be in upstream, I already have it in my tree. :) But created the PR since I figured it might be worthwhile to have upstream. Also OK with me if we don't merge this. Clarified the tradeoff section. Indeed, it will increase code size at this moment (namely, due to the longer version string). But, it will then be more consistent: We are vastly less likely to see random +X sizes on PR's because the commit hash happened to have a collision that prevented it from being shorter, or because the build node had a larger number of references in it's repository (tbh, I don't know how GitHub does that internally, so can't really comment on how likely that is), or because the heuristic Git used changed between versions, et cetera. |
Ah, that is a quite good point |
Summary
The Git hash is embedded in the version number. The hash is abbreviated by Git. This commit changes the length of the Git hash abbreviation to a fixed number, so that the length of the version string no longer varies based on external factors.
Made this change because builds of the same MicroPython commit on multiple machines were sometimes giving a version string with different lengths. This change may also help the code size report to be more consistent, because it will less often be impacted by random changes in the version string length, at the cost of always being a few bytes longer.
Testing
By default Git chooses the length "based on the approximate number of packed objects in your repository" (man page). This was causing the version string length to be unstable depending on how big your local repository is.
At the moment of writing, the
master
branch on GitHub contains 17463 commits. Based on the statistics from my own repository, I set the value to 10, which is one higher than the highest value I saw required for a unique commit abbreviation in my local repository checkout:Trade-offs and Alternatives
Alternative is to not do this. Then we don't get the consistency benefits, but will keep semi-randomly saving a few bytes for years to come (until the default gets increased by the Git heuristic).
Note that Git will still use more characters if it required to make the hash unique. So all this setting does is increase the minimum slightly; Git will still use a longer hash if it already knows about another commit with the same abbreviation.
We can always safely increase this number later should the need arise.
Note: Presumably the users with the largest repositories are the core maintainers, so perhaps they could check the statistics for their repository and report if they think a length of 10 is the right choice.