Fix git system hash for worktrees

[chuseman]

Running GitVersion from a git worktree nearly works for me -- I just get an exception when GitVersionCacheKeyFactory.GetGitSystemHash() is called.

To come up with the gitSystemHash portion of the composite hash, .git/refs/* are enumerated and all file names are included. This falls down for worktrees because the .git directory returned is actually .git/worktrees/<worktree name> and there isn't a refs folder here.

So, this PR just does some string matching to see if the path looks like a worktree and if so, return the parent .git dir path instead. I didn't see an easy way using LibGit2Sharp to jump from a worktree to its parent repo path and I figured string matching on the path is probably safe enough for this case.

Also, I added a more meaningful message when throwing an ArgumentException() when the refs folder couldn't be found.

[tests-checker]

Could you please add tests to make sure this change works as expected?

[chuseman]

I give up trying to make LibGit2Sharp create a cross-platform worktree to test this 🤦‍♂

[asbjornu]

@chuseman, is that why AppVeyor is failing the build?

[chuseman]

No, I was just trying to add a test for completeness. The AppVeyor thing looks like it was a transient network error:

Preparing to run build script...
C:\projects\gitversion\build.ps1 : Exception calling "DownloadFile" with "2" argument(s): "A connection attempt failed because the connected party did not properly respond after a period of time, or established connection failed because connected host has failed to respond A connection attempt failed because the connected party did not properly respond after a period of time, or established connection failed because connected host has failed to respond"

Do you have a way to kick off another AppVeyor build to see if it works now?

[asbjornu]

No, I was just trying to add a test for completeness.

I see. Yes, I expected to find a test in this PR somewhere. If you have a proposal for a test, perhaps we can discuss in this PR how to make it work cross-platform? Please add it to the PR.

Do you have a way to kick off another AppVeyor build to see if it works now?

Yes, done.

[asbjornu]

@chuseman, when LibGit2Sharp throws LibGit2Sharp.LibGit2SharpException : reference is not a branch, do you know which reference is failing? If not, can we try to catch LibGit2SharpException and write the failing reference to the console?

[chuseman]

I don't see a way to get the reference that it is complaining about. The exception doesn't have any interesting properties that aren't logged already.

I think this is actually some kind of LibGit2Sharp on Mono failure -- it only fails when running the net472 test on Mono. LibGit2Sharp's test to add a worktree does almost exactly what mine does so I don't think I'm calling the API incorrectly. I don't think they run tests on Mono though - just .NET Core for Linux/Mac.

This probably isn't worth fighting for this case though because the part that is failing is just the arranging of the environment (creating a dummy worktree) to test my new logic. When actually exercising the new logic, it works fine on Mono. So, for now I've just added a skip for this worktree test on mono.

[asbjornu]

@chuseman, have you inspected the Exception.Data property for information as well?

[chuseman]

@chuseman, have you inspected the Exception.Data property for information as well?

Yes - the only data added are libgit2.code and libgit2.category, both of which were -1 (they were already in the build output).

[asbjornu]

I think there might be tests that already ignore Mono through the use of [Category] or something similar. Can you please investigate that option?

[chuseman]

I did see the NoMono category, but it looks like the logic is to skip the whole test when running on Unix. This test can run with .NET Core, so I didn't want to skip it altogether.

[asbjornu]

Aha, that logic is wrong, then. There are surely a lot of tests decorated with NoMono that break on Unix, but then they should be marked with a NoUnix category. To not continue digging in this erroneous hole and make it any bigger, let's fix the NoMono vs. NoUnix issue first and then decorate this test with NoUnix. Ok? If you want to tackle it in this PR that would be very much appreciated, otherwise I can take a look in another PR if I can find a few minutes of spare time.

[chuseman]

I started adding the changes for a NoUnix category, but there were some required category changes so I think its better in a separate PR. I'll open another one once I work through the failing tests.

For now, I've just switched my new test over to NoMono so it should pass.

[arturcic]

@chuseman can you rebase on top of master? the change from af4e947 was merged

[codecov]

Codecov Report

Merging #1759 into master will not change coverage.
The diff coverage is n/a.

@@          Coverage Diff           @@
##           master   #1759   +/-   ##
======================================
  Coverage    66.9%   66.9%           
======================================
  Files         130     130           
  Lines        4829    4829           
======================================
  Hits         3231    3231           
  Misses       1598    1598

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update af4e947...f4de9eb. Read the comment docs.

[asbjornu]

@chuseman, can you please have a look at what @ermshiperete has done in #1793 and consider whether it solves your problem or not?

[chuseman]

@asbjornu yes, I think so. I'll close this PR in favor of #1793