#953 Fix dynamic repositories

[GeertvanHorrik]

No description provided.

[GeertvanHorrik]

Fixes #953

[JakeGinnivan]

Does this test need to hit the actual repo or could it just use a fixture like elsewhere?

[asbjornu]

@GeertvanHorrik: Now that master builds are green again, can you please rebase this so we can see if this turns green as well?

[GeertvanHorrik]

I just hit "Update branch", I hope that was the right button :-)

[aateeque]

Not sure if this fixes the issue with dynamic repositories. As reported by others in #953 it seems an uninitialized GitPreparer may have something to do here. Maybe its my setup and I am looking at this completely the wrong way, but the issue I have found is that the constructor parameter targetPath for GitPreparer needs to be a path that contains a .git directory - in other words the target path has to be within a git repository.
If the targetPath is not set it uses the current directory which will be the download path for GitVersion.exe (from nuget for example while using Cake). In this instance, the call to GetDotGitDirectory() from ConfigurationProvider.Verify() throws an exception as Repository.Discover(targetPath) returns null for a non-git targetPath. Following this GitVersion will exit with 1 after failing an attempt to print a graph for a non-git directory.

[GeertvanHorrik]

This requires an update of GitTools.Core, checking the status now.

[GeertvanHorrik]

Just verified this stuff locally on my machine, all seems to work, I think we should merge this.

[GeertvanHorrik]

But it should be merged into 4.0.0 beta. @JakeGinnivan which branch this I merge this into?

[asbjornu]

@GeertvanHorrik: I've restarted all failing builds since it seems like the Mono source server was down when they were run. Stuff targeting v4 should be merged into master.

[GeertvanHorrik]

Looks like the builds are not really doing anything (or at least I cannot check the details of the AppVeyor builds).

[GeertvanHorrik]

But since GitTools.Core was already merged, this PR only:

1. Fixes possible NullReferenceException
2. Adds more unit tests

[asbjornu]

@GeertvanHorrik: True. Since Windows and OS X builds are green, this can safely be merged. It's only Linux builds that fail with:

Failed to fetch http://download.mono-project.com/repo/debian/pool/main/m/mono/mono-jay_4.6.2.16-0xamarin1_amd64.deb  503  Service Temporarily Unavailable [IP: 52.85.142.49 80]

[GeertvanHorrik]

@asbjornu Thanks for the fast replies, really appreciate this. Can you merge it, not sure if I should use "rebase & merge" or "squash & merge".

[asbjornu]

@GeertvanHorrik: No problem. I've rebased 3badcd2 on top of the HEAD of master to make the merge a bit less messy. When it's done testing, I think it's good to go. .

[GeertvanHorrik]

Thanks, looks like everything succeeded (at least the AppVeyor builds). I'll merge this now.

[JakeGinnivan]

@GeertvanHorrik just running these tests, they are pretty slow because they talk to GitHub and actually close another repo (they take about 5 seconds each..). Can you submit a PR which changes these tests to use the fixture approach. We have other tests which simulate the dynamic repo builds and run super fast.

[JakeGinnivan]

I also get

Message: System.UnauthorizedAccessException : Access to the path 'pack-101bf845e769c8eee6dfff4cefaaf619d573f0b4.idx' is denied.

The second time I run the tests..