Implement VCSDownloadStrategy#last_commit

[vladshablinsky]

• Have you followed the guidelines in our Contributing document?
• Have you checked to ensure there aren't other open Pull Requests for the same change?
• Have you added an explanation of what your changes do and why you'd like us to include them?
• Have you written new tests for your changes? Here's an example.
• Have you successfully run brew tests with your changes locally?

---

Description

• Add VCSDownloadStrategy#last_commit
• Add GitDownloadStrategyTests

This PR is a part of #274, so merged commits will be removed from #274.

cc @MikeMcQuaid @xu-cheng

[MikeMcQuaid]

👍

[UniqMartin]

👍

[xu-cheng]

Thanks everyone.

[UniqMartin]

FYI, I just fixed a minor problem with this in 32f7e73. It felt innocent enough that I committed it directly. Noticed this because I have a non-standard core.abbrev setting and the test suite was failing:

  1) Failure:
GitDownloadStrategyTests#test_last_commit [/opt/homebrew/Library/Homebrew/test/test_download_strategies.rb:122]:
--- expected
+++ actual
@@ -1 +1 @@
-"c50c79b"
+"c50c79b9"

[vladshablinsky]

@UniqMartin I have read git documentation and now understand that I was wrong. Thanks for the fix!
I thought git --git-dir rev-parse --short HEAD would print k first symbols of revision string where k is the minimum length more or equal to 7 so that no other revision starts with that string.

So I would probably fix it this way if I wouldn't realise git works pretty different from my expectations:

assert "c50c79b9573c9482b9178a1882695fbbcd36fe7d".start_with?(@strategy.last_commit)

Thanks again!

[UniqMartin]

No worries! I also overlooked this detail when reviewing this PR. Your snippet, that would have changed the related test instead of the implementation, is also a perfectly valid fix for the test failure.

The reason I decided to make the change in the implementation is because I was of the opinion that these shortened commit hashes will eventually be used for the improved HEAD installation support, i.e. they will be used to create a “version number” like HEAD-<last_commit>. (Please correct me if I'm wrong about this.) If that's the case, it would be odd that this generated “version number” will be different across different systems with different Git configurations. (Of course there's also the small chance that the 7-character hash becomes non-unique as more commits are added to the repository, which would also change the output of git rev-parse --short even if the =7 is added.)

[vladshablinsky]

Please correct me if I'm wrong about this.

Everything is right. I'm OK with your fix, it makes sense for me.