Skip to content
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

v.utils: update githash to be able to get the githash of every passed project #21178

Merged
merged 8 commits into from
Apr 4, 2024

Conversation

ttytm
Copy link
Member

@ttytm ttytm commented Apr 3, 2024

Reworks githash to extend its use cases by allowing it to return the githash of any passed path. Currently, it is used for @VCURRENTHASH and only returns the path of the V hash. Chances that githash was called directly outside the V codebase itself are low, but it is a public function, so this change can be considered breaking.

The changes:

  • Simplifies: the function no longer uses a bool param to return a static value if it is set to false; instead, we assign static values directly.
  • Uses early returns instead of a less safe workaround with a for loop.
  • Utilizes Result values to be able to trace down misbehavior instead of assigning a fallback value directly in the function.

The new functionality will enable an easy implementation of @VMODHASH in #21091.

@spytheman spytheman merged commit a458ade into vlang:master Apr 4, 2024
54 checks passed
@spytheman
Copy link
Member

Excellent work.

assert test_rev == version.githash(git_proj_path)!
os.write_file('README.md', '')!
os.execute_opt('git add .')!
os.execute_opt('git commit -m "test2"')!
test_rev2 := os.execute_opt('git rev-parse HEAD')!.output[..7]
test_rev2 := os.execute_opt('git rev-parse --short HEAD')!.output.trim_space()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is a bit brittle, since if you do not specify the length explicitly with --short=7 for example, it will depend on https://git-scm.com/docs/git-config#Documentation/git-config.txt-coreabbrev which says:

core.abbrev
Set the length object names are abbreviated to. If unspecified or set to "auto",
an appropriate value is computed based on the approximate number of packed objects in
your repository, which hopefully is enough for abbreviated object names to stay unique
for some time. If set to "no", no abbreviation is made and the object names are shown
in their full length. The minimum length is 4.

i.e. in the future, when the v repo grows, it may decide to use 8 characters.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, we should add that specification to that

@ttytm ttytm deleted the util/update-githash branch April 4, 2024 02:42
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.

None yet

2 participants