Fixed issue with Dynamic Repostitories not working

[rubenmamo]

Fixed issue which was causing dynamic repositories to fail as per issue #782

Added missing call to gitPreparer.Initialise

Tested on my CI environment and it is working

[asbjornu]

@rubenmamo Thanks for this! But can you please rebase on master so it gets the fixes in #876 that ensures the Travis build is run?

[rubenmamo]

@asbjornu I'm a bit new to git. I think I rebased it correctly. Let me know if it is not.

[asbjornu]

@rubenmamo Thanks for trying, but this didn't end up quite right. 😃 Please try the following:

git checkout ResolveDynamicRepositoryIssue
git branch ResolveDynamicRepositoryIssue-Backup
git reset --hard upstream/master
git cherry-pick e8fd5697e68436ac3a4b8a1e58dfbfa26deb687c

This will first backup your current ResolveDynamicRepositoryIssue branch to ResolveDynamicRepositoryIssue-Backup just in case you do something wrong, then reset ResolveDynamicRepositoryIssue to the current HEAD of master in upstream, which is this repository. If you don't have upstream defined, do this before any of the above:

git remote add upstream https://github.com/GitTools/GitVersion.git

When all of this is done, git status will let you know that you're ahead of the remote branch by 1 commit and behind by something like 12. This means local and remote have diverged, which is correct. To be able to push to your remote branch, you will need to use --force:

git push --force origin ResolveDynamicRepositoryIssue

Please let me know if there's any problems or questions. Good luck! 👍

[rubenmamo]

@asbjornu Followed your instructions and it seems to have worked

Thanks a lot for your help

[asbjornu]

@rubenmamo Yes, that looks perfect! Now if only @gep13 or @JakeGinnivan could weigh in on the implications of always doing gitPreparer.Initialise(false, targetBranch);, we could possibly have this one merged! I would appreciate a test for this, though.

[gep13]

@rubenmamo @asbjornu in all honesty, I am not going to have any chance to look at this in the immediate future :-(

[rubenmamo]

I looked at trying to implement a test for this, however I couldn't see a way of testing for this without re-engineering the whole method (which could probably cause more problems)

Below is an excerpt of the method I modified.

        public VersionVariables ExecuteGitVersion(string targetUrl, string dynamicRepositoryLocation, Authentication authentication, string targetBranch, bool noFetch, string workingDirectory, string commitId, Config overrideConfig = null)
        {
            // More code here
            var gitPreparer = new GitPreparer(targetUrl, dynamicRepositoryLocation, authentication, fetch, workingDirectory);
            gitPreparer.Initialise(false, targetBranch);
            var dotGitDirectory = gitPreparer.GetDotGitDirectory();
            var projectRoot = gitPreparer.GetProjectRootDirectory();
            // More code here
        }

Since the instance of GitPreparer is created inside the method I would not be able to mock it's execution (unless I'm missing something I'm unaware of)

Unfortunately this specific bug makes GitVersion completely unusable with TeamCity when using Dynamic Repositories. I had to revert to GitVersion v3.3.0 in order to get my builds working, however v3.3.0 has another bug which causes GitVersion to not load the GitVersion.yml file from the repository (it tries to load it from the GitVersion folder) and hence I could still not work with v3.3.0. In the end I made this fix and copied a locally built GitVersion to my CI environment so that I could work. I suspect there might be other people who could run into this issue

[rubenmamo]

I suspect the bug was created in commit 9b336cd

This change introduced the code in question in the ExecuteGitVersion method. In the previous commit ( 6d01254 ) of the file, GitPreparer was initialised in ExecuteInternal and the following code was present:

            var gitPreparer = new GitPreparer(targetUrl, dynamicRepositoryLocation, authentication, noFetch, workingDirectory);
            var applicableBuildServers = BuildServerList.GetApplicableBuildServers();
            var buildServer = applicableBuildServers.FirstOrDefault();

            gitPreparer.Initialise(buildServer != null, ResolveCurrentBranch(buildServer, targetBranch));

            var dotGitDirectory = gitPreparer.GetDotGitDirectory();
            var projectRoot = gitPreparer.GetProjectRootDirectory();

Notice the Initialise before the gitPrepare.GetDotGitDirectory() call

I noticed that the current version of ExecuteInternal calls

gitPreparer.Initialise(buildServer != null, ResolveCurrentBranch(buildServer, targetBranch, gitPreparer.IsDynamicGitRepository));

Unfortunately this is not valid as gitPreparer.IsDynamicGitRepository is only setup after the call to gitPreparer.Initialise.

I suggest that I change my Initialise code and the code in ExecuteInternal to be as follows:

gitPreparer.Initialise(buildServer != null, ResolveCurrentBranch(buildServer, targetBranch, !string.IsNullOrWhiteSpace(dynamicRepositoryLocation)));

any thoughts?

[asbjornu]

@rubenmamo Nice find! Yes, I think that Initialise() method looks more in line with how it was before, which should remedy the problem without introducing new ones. We should really try to write a test for this, though. Even if it means restructuring large parts of the code to make it testable.

[JakeGinnivan]

Thanks for investigating this @rubenmamo!

ExecuteCoreTests could possible have a test added. All of GitVersions tests are integration style so you don't need to mock anything out. Simply build a git repo on the fly with the repositories then run GitVersion.

Happy to merge but did you want to give writing a test a go? If there are specific road blocks also let me know, we might be able to nut them out.

[rubenmamo]

I'll try writing a test for this. I think it's a bit more complex than just building a git repo on the fly though.

Since this is related to Dynamic Repositories I need a fake project on disk without a .git folder, then ExecuteCore needs to somehow clone a repository and use it with the dynamic folders functionality. ExecuteCore will need the URL of a repository, I guess I could use the gitversion repository URL.

[rubenmamo]

I submitted an additional commit with the following changes:

1. Removed the gitPreparer.Initialise call from the ExecuteInternal method (since the same call is now done earlier)
2. Changed my previous fix so it uses the parameters as previously passed in ExecuteInternal
3. Added a test which runs ExecuteCore as if it were using dynamic repositories. If you comment line 30 of ExecuteCore.cs the test will fail with a DirectoryNotFound Exception

I think this should cover everything we discussed. The tests pass on my machine. I just noticed the Travis CI build failed. I'm not sure why it's failed on Travis CI :(

[rubenmamo]

The log from the run of my machine is:

INFO [05/26/16 16:38:08:41] Project root is: C:\Users\rubenmamo\AppData\Local\Temp\TestRepositories\cd62ed72-66c8-4da9-843d-4a3b422a0416
INFO [05/26/16 16:38:08:42] Begin: Loading version variables from disk cache
  INFO [05/26/16 16:38:08:43] Cache file C:\Users\rubenmamo\AppData\Local\Temp\TestRepositories\cd62ed72-66c8-4da9-843d-4a3b422a0416\.git\gitversion_cache\040E3C243ACC5802A17BD3E9C3078982B6BA6E30.yml not found.
INFO [05/26/16 16:38:08:43] End: Loading version variables from disk cache (Took: 11.00ms)
INFO [05/26/16 16:38:08:43] Using latest commit on specified branch
INFO [05/26/16 16:38:08:44] Running against branch: master (c0eaa0c6d8bebce2a462d8820bb0fd58d6b7614a)
INFO [05/26/16 16:38:08:44] Begin: Calculating base versions
  INFO [05/26/16 16:38:08:44] Fallback base version: 0.1.0 with commit count source c0eaa0c6d8bebce2a462d8820bb0fd58d6b7614a
  INFO [05/26/16 16:38:08:47] Base version used: Fallback base version: 0.1.0 with commit count source c0eaa0c6d8bebce2a462d8820bb0fd58d6b7614a
INFO [05/26/16 16:38:08:47] End: Calculating base versions (Took: 34.00ms)
INFO [05/26/16 16:38:08:48] Skipping version increment
INFO [05/26/16 16:38:08:48] 0 commits found between c0eaa0c6d8bebce2a462d8820bb0fd58d6b7614a and c0eaa0c6d8bebce2a462d8820bb0fd58d6b7614a
INFO [05/26/16 16:38:08:49] Begin: Creating dictionary
INFO [05/26/16 16:38:08:50] End: Creating dictionary (Took: 4.00ms)
INFO [05/26/16 16:38:08:50] Begin: Storing version variables to cache file C:\Users\rubenmamo\AppData\Local\Temp\TestRepositories\cd62ed72-66c8-4da9-843d-4a3b422a0416\.git\gitversion_cache\040E3C243ACC5802A17BD3E9C3078982B6BA6E30.yml
INFO [05/26/16 16:38:08:50] End: Storing version variables to cache file C:\Users\rubenmamo\AppData\Local\Temp\TestRepositories\cd62ed72-66c8-4da9-843d-4a3b422a0416\.git\gitversion_cache\040E3C243ACC5802A17BD3E9C3078982B6BA6E30.yml (Took: 1.00ms)
INFO [05/26/16 16:38:08:60] Creating dynamic repository at 'C:\Users\rubenmamo\AppData\Local\Temp\Project1'
INFO [05/26/16 16:38:08:60] Setting up credentials using name '****'
INFO [05/26/16 16:38:08:60] Retrieving git info from url '****'
INFO [05/26/16 16:38:09:30] Project root is: C:\windows\system32
INFO [05/26/16 16:38:09:32] Begin: Loading version variables from disk cache
  INFO [05/26/16 16:38:09:34] Cache file C:\Users\rubenmamo\AppData\Local\Temp\Project1\.git\gitversion_cache\1D687A3B5A8ED183B28B2409835A0546C6D9D591.yml not found.
INFO [05/26/16 16:38:09:34] End: Loading version variables from disk cache (Took: 22.00ms)
INFO [05/26/16 16:38:09:35] Using latest commit on specified branch
INFO [05/26/16 16:38:09:39] Running against branch: head/master (ba896e7a88dba50a2a0dd76dd32e31bc23cdb24d)
INFO [05/26/16 16:38:09:39] 0 commits found between ba896e7a88dba50a2a0dd76dd32e31bc23cdb24d and ba896e7a88dba50a2a0dd76dd32e31bc23cdb24d
INFO [05/26/16 16:38:09:40] Begin: Calculating base versions
  INFO [05/26/16 16:38:09:44] Fallback base version: 0.1.0 with commit count source f143f59ed9f54f19527a8272eca93cdc13edd0c7
  INFO [05/26/16 16:38:09:64] Git tag '1.1.3': 1.1.3 with commit count source ba896e7a88dba50a2a0dd76dd32e31bc23cdb24d
  INFO [05/26/16 16:38:09:67] Base version used: Git tag '1.1.3': 1.1.3 with commit count source ba896e7a88dba50a2a0dd76dd32e31bc23cdb24d
INFO [05/26/16 16:38:09:67] End: Calculating base versions (Took: 276.02ms)
INFO [05/26/16 16:38:09:69] Skipping version increment
INFO [05/26/16 16:38:09:69] Using branch name to calculate version tag
INFO [05/26/16 16:38:09:93] 0 commits found between ba896e7a88dba50a2a0dd76dd32e31bc23cdb24d and ba896e7a88dba50a2a0dd76dd32e31bc23cdb24d
INFO [05/26/16 16:38:09:96] Begin: Creating dictionary
INFO [05/26/16 16:38:09:96] End: Creating dictionary (Took: 0.00ms)
INFO [05/26/16 16:38:09:96] Begin: Storing version variables to cache file C:\Users\rubenmamo\AppData\Local\Temp\Project1\.git\gitversion_cache\1D687A3B5A8ED183B28B2409835A0546C6D9D591.yml
INFO [05/26/16 16:38:09:96] End: Storing version variables to cache file C:\Users\rubenmamo\AppData\Local\Temp\Project1\.git\gitversion_cache\1D687A3B5A8ED183B28B2409835A0546C6D9D591.yml (Took: 0.99ms)

whilst the one from TravisCI is

=> ExecuteCoreTests.DynamicRepositoriesShouldNotErrorWithFailedToFindGitDirectory
Created git repository at '/tmp/TestRepositories/bead1664-a325-47bc-b642-25bc7b2f9792'
WARN [05/26/16 14:27:08:24] Failed to check build server 'ContinuaCi': No access to the given key
INFO [05/26/16 14:27:08:24] Project root is: /tmp/TestRepositories/bead1664-a325-47bc-b642-25bc7b2f9792
INFO [05/26/16 14:27:08:25] Begin: Loading version variables from disk cache
  INFO [05/26/16 14:27:08:26] Cache file /tmp/TestRepositories/bead1664-a325-47bc-b642-25bc7b2f9792/.git/gitversion_cache/0916657566CB571FD4B170ED15E526BDFBA066B9.yml not found.
INFO [05/26/16 14:27:08:26] End: Loading version variables from disk cache (Took: 12.91ms)
INFO [05/26/16 14:27:08:27] Using latest commit on specified branch
INFO [05/26/16 14:27:08:27] Running against branch: master (d38ffad81d8d9d969056fc450987530e54c368db)
INFO [05/26/16 14:27:08:27] Begin: Calculating base versions
  INFO [05/26/16 14:27:08:28] Fallback base version: 0.1.0 with commit count source d38ffad81d8d9d969056fc450987530e54c368db
  INFO [05/26/16 14:27:08:31] Base version used: Fallback base version: 0.1.0 with commit count source d38ffad81d8d9d969056fc450987530e54c368db
INFO [05/26/16 14:27:08:31] End: Calculating base versions (Took: 40.13ms)
INFO [05/26/16 14:27:08:31] Skipping version increment
INFO [05/26/16 14:27:08:32] 0 commits found between d38ffad81d8d9d969056fc450987530e54c368db and d38ffad81d8d9d969056fc450987530e54c368db
INFO [05/26/16 14:27:08:33] Begin: Creating dictionary
INFO [05/26/16 14:27:08:33] End: Creating dictionary (Took: 0.23ms)
INFO [05/26/16 14:27:08:33] Begin: Storing version variables to cache file /tmp/TestRepositories/bead1664-a325-47bc-b642-25bc7b2f9792/.git/gitversion_cache/0916657566CB571FD4B170ED15E526BDFBA066B9.yml
INFO [05/26/16 14:27:08:33] End: Storing version variables to cache file /tmp/TestRepositories/bead1664-a325-47bc-b642-25bc7b2f9792/.git/gitversion_cache/0916657566CB571FD4B170ED15E526BDFBA066B9.yml (Took: 0.47ms)
WARN [05/26/16 14:27:08:33] Failed to check build server 'ContinuaCi': No access to the given key
INFO [05/26/16 14:27:08:33] Creating dynamic repository at '/tmp/GitVersion'
INFO [05/26/16 14:27:08:33] Retrieving git info from url 'https://github.com/GitTools/GitVersion.git'
INFO [05/26/16 14:27:09:95] Project root is: 
=> ExecuteCoreTests.WorkingDirectoryWithoutGit


1) Error : ExecuteCoreTests.DynamicRepositoriesShouldNotErrorWithFailedToFindGitDirectory
System.Exception : Failed to prepare or find the .git directory in path ''.
  at GitVersion.ExecuteCore.ExecuteGitVersion (System.String targetUrl, System.String dynamicRepositoryLocation, GitVersion.Authentication authentication, System.String targetBranch, Boolean noFetch, System.String workingDirectory, System.String commitId, GitVersion.Config overrideConfig) <0x4035f230 + 0x00405> in <filename unknown>:0 
  at ExecuteCoreTests+<DynamicRepositoriesShouldNotErrorWithFailedToFindGitDirectory>c__AnonStorey4.<>m__0 (GitTools.Testing.EmptyRepositoryFixture fixture, GitVersion.VersionVariables vv) <0x40378d30 + 0x000a0> in <filename unknown>:0 
  at ExecuteCoreTests.RepositoryScope (GitVersion.ExecuteCore executeCore, System.Action`2 fixtureAction) <0x4035ed20 + 0x00437> in <filename unknown>:0 
  at ExecuteCoreTests.DynamicRepositoriesShouldNotErrorWithFailedToFindGitDirectory () <0x40378b70 + 0x00186> in <filename unknown>:0 
  at (wrapper managed-to-native) System.Reflection.MonoMethod:InternalInvoke (System.Reflection.MonoMethod,object,object[],System.Exception&)
  at System.Reflection.MonoMethod.Invoke (System.Object obj, BindingFlags invokeAttr, System.Reflection.Binder binder, System.Object[] parameters, System.Globalization.CultureInfo culture) <0x40179da0 + 0x000b7> in <filename unknown>:0 

The difference between my run and the one on TravisCI is that I ran the test against my own Git Repository since we have a proxy server at the office and it complicates life with running git version behind it.

I suspect the issue might be related to permissions or access to the git repo from the server. Any thoughts?

[rubenmamo]

I have fixed the issue with the test. Basically I had copied the call to execute core from the other tests and used the same directory as the other tests (i.e. Environment.SystemDirectory). It seems that this property is set to empty string in TravisCI whilst it is needed to be filled with something in order for the process to proceed. I changed the test call to include a fake directory and the process worked.

In order to understand what was happening on TravisCI I had to add some additional logging, see commits 335bf47 and 76d0535. Do you want me to revert these changes to the logs or are you happy keeping them in?

[asbjornu]

If you look at the other tests in this file, fixture.RepositoryPath is what's used. Environment.SystemDirectory is used in the one test above just because it's guaranteed to:

1. Exist.
2. Not contain a .git folder.

[asbjornu]

@rubenmamo Great work on the test! I also think the logging is fine. I do have one comment regarding the path, though, so please just fix that for consistency and we should be good to go! 👍

[rubenmamo]

I changed the path as discussed

[asbjornu]

@rubenmamo Excellent! 👍

[asbjornu]

Merged in 66bc2b0.