Fixed issues #863 and #870: invalid cache key computation and config file location #892

sergiorykov · 2016-06-04T17:42:10Z

ConfigurationProvider verifies consistency of config files (project root and working directories) before executing commands.
Cache key computation uses ConfigurationProvider to get valid config file path.

Appropriate tests are included.
#863 #870

…ootProjectDirectory

…ject root directory

sergiorykov · 2016-06-10T15:20:16Z

@asbjornu Can you take a look? It seems pull request is going to be outdated because of other pull requests are being merged.

asbjornu · 2016-06-13T14:27:59Z

src/GitVersionExe/Program.cs

@@ -108,6 +110,11 @@ static int VerifyArgumentsAndRun()
            return 0;
        }

+        private static void VerifyConfiguration(Arguments arguments, IFileSystem fileSystem)
+        {
+            var gitPreparer = new GitPreparer(arguments.TargetUrl, arguments.DynamicRepositoryLocation,arguments.Authentication,arguments.NoFetch,arguments.TargetPath);


Nitpick, but can you please format this according to the ReSharper style settings? We at least want spaces after commas, but if the line is very long, it would be nice with line breaks after each argument and the arguments aligned.

asbjornu · 2016-06-13T14:32:44Z

@sergiorykov I think this looks good! Thanks for taking the time to look into this and fixing it. I have a couple of in-code comments and also wonder if this is now tested well enough. Do you think it's possible to create a few tests verifying the different cache invalidation scenarios, so this don't regress in the future?

…ption contains expected message

This reverts commit 7a2e309.

…nfigFileSelection

…tVersion.yml

When /overrideconfig presents version must be calculated explicitly without saving in cache.

Ensures that overriden config will be applied whenever there cached version exists and willn't save results in cache

sergiorykov · 2016-06-14T13:52:13Z

Checked caching key calculation.

I've done a bit more - by process it's not good tone :(.

Calculating cache key depends on three things: git metadata, config file, /overrideconfig from command line.

git metadata changes for cache key calculation requires a bit more abstractions and I think it's not nessesary to add regression tests for it.

config file changes test requires configuring abstraction of git with two different tag-prefixes.
default config file will have first tag-prefix and version will be returned according to it. Then we will update config with second tag-prefix and should receive other version. (It was my case)
If you will say how to configure that context I will add tests in separate pull request.

overrides from command line - that case was ignored at all. I've added support for it with a test.

asbjornu · 2016-06-14T15:09:43Z

src/GitVersionCore.Tests/ConfigProviderTests.cs

+
+        WarningException exception = Should.Throw<WarningException>(() => { ConfigurationProvider.Verify(workingPath, repoPath, fileSystem); });
+
+        var expecedMessage = string.Format("Ambigous config file selection from '{0}' and '{1}'", workingDirectoryConfigFilePath, repositoryConfigFilePath);


Typo: It's "ambiguous". 😃

asbjornu · 2016-06-14T15:27:15Z

@sergiorykov Thanks again! I've made a few more comments inline. Please go over them and let me know what you think.

…Branches Will be eventually corrupted back after applying Resharper's clean code. Will need to do smth with it.

sergiorykov · 2016-06-19T15:42:54Z

Thank for your help and detailed code review!

asbjornu · 2016-06-20T08:34:37Z

docs/configuration.md

@@ -4,6 +4,8 @@ GitVersion 3.0 is mainly powered by configuration and no longer has branching st
 ## Configuration tool
 If you run `GitVersion init` you will be launched into a configuration tool, it can help you configure GitVersion the way you want it.

+It will create file 'GitVersion.yml' in directory where you launched initialization. It can be root repository directory or any subdirectory in case when you have single repository for more then one project or restricted to commit into subdirectory.


Perhaps this reads a bit better?

Once complete, the init command will create a GitVersion.yml file in the working directory. It can be the root repository directory or any subdirectory in case you have a single repository for more than one project or are restricted to commit into a subdirectory.

Accepted 96e41c7 :)

asbjornu · 2016-06-20T08:36:51Z

@sergiorykov I've replied and see that you need to do a rebase before this can be merged. 😃

JakeGinnivan · 2016-07-09T02:38:42Z

I can merge manually if you are busy @sergiorykov, is there anything outstanding to do on this PR?

sergiorykov · 2016-07-16T18:33:22Z

I'm really sorry - that time was purely devoted to work.

I will add corrections in a day or two and it will be ready to merge.

Extracted cache key calculation to GitVersionCacheKeyFactory

…e code that doesn't fear Reshaper

…after init

sergiorykov · 2016-07-16T20:47:16Z

It seems all comments are considered.

It's still hard to unit test cache key calculation, but it became clear what components are being used:

public class GitVersionCacheKeyFactory
{
        public static GitVersionCacheKey Create(IFileSystem fileSystem, GitPreparer gitPreparer, Config overrideConfig)
        {
            var gitSystemHash = GetGitSystemHash(gitPreparer, fileSystem);
            var configFileHash = GetConfigFileHash(fileSystem, gitPreparer);
            var repositorySnapshotHash = GetRepositorySnapshotHash(gitPreparer);
            var overrideConfigHash = GetOverrideConfigHash(overrideConfig);

            var compositeHash = GetHash(gitSystemHash, configFileHash, repositorySnapshotHash, overrideConfigHash);
            return new GitVersionCacheKey(compositeHash);
        }

JakeGinnivan · 2016-07-17T02:09:19Z

@sergiorykov if you are still around could you rebase this branch? Would love to release in 3.6.1 Then get 4.0.0 out after

JakeGinnivan · 2016-07-17T02:11:06Z

If you don't have time also let me know and ill do it :)

I will do it either way this arvo.

JakeGinnivan · 2016-07-17T03:05:44Z

Merged via #951

sergiorykov · 2016-07-18T09:23:36Z

Thanks!

Different timezones. ^)

sergiorykov and others added 4 commits June 4, 2016 19:10

checking ambigous and obsolete config files at workingDirectory and r…

f31647c

…ootProjectDirectory

select config file from working directory first and only then use pro…

e26c557

…ject root directory

added tests for verification config files + a bit of refactoring

c4d8a4d

Merge branch 'master' into master

eb83ac1

asbjornu reviewed Jun 13, 2016
View reviewed changes

sergiorykov added 10 commits June 14, 2016 12:13

code style

9853c79

Test ThrowsExceptionOnAmbigousConfigFileLocation verifies WarningExce…

87771fe

…ption contains expected message

* ConfigurationProvider.cs clean code

7a2e309

Revert "* ConfigurationProvider.cs clean code"

9d592ab

This reverts commit 7a2e309.

* ConfigurationProvider.cs refactoring: extracted WarnAboutAmbigousCo…

fa26078

…nfigFileSelection

* HelpWriter.cs fixed default config file name: GitVersion.yaml -> Gi…

ddaff32

…tVersion.yml

* GitVersionCache.cs refactoring: improved readibility

02036f9

fixed bug: /overrideconfig was ignored in caching results

764ee79

When /overrideconfig presents version must be calculated explicitly without saving in cache.

Tests for parsing option /overrideconfig

ca604a1

added test for applying /overrideconfig option from command line

0366f69

Ensures that overriden config will be applied whenever there cached version exists and willn't save results in cache

asbjornu reviewed Jun 14, 2016
View reviewed changes

sergiorykov added 2 commits June 19, 2016 17:58

fixed typo in warning exception messages: ambigous -> ambiguous

0eb743f

* ConfigurationProvider.cs reverted back rules formatting for Migrate…

cfd90da

…Branches Will be eventually corrupted back after applying Resharper's clean code. Will need to do smth with it.

* docs\configuration.md added comment about generation GitVersion.yml

f2db2a4

asbjornu reviewed Jun 20, 2016
View reviewed changes

sergiorykov added 4 commits July 16, 2016 22:49

added overrideConfig to cache key

dbc988b

Extracted cache key calculation to GitVersionCacheKeyFactory

* ConfigurationProvider.cs refactored MigrateBranches - clean&readabl…

c9ce4d8

…e code that doesn't fear Reshaper

added /ovverideconfig option to command line docs

c2a7d2e

* docs\configuration.md corrected text about location of config file …

96e41c7

…after init

JakeGinnivan mentioned this pull request Jul 17, 2016

Cache keys fixes #951

Merged

JakeGinnivan closed this Jul 17, 2016


		WarningException exception = Should.Throw<WarningException>(() => { ConfigurationProvider.Verify(workingPath, repoPath, fileSystem); });

		var expecedMessage = string.Format("Ambigous config file selection from '{0}' and '{1}'", workingDirectoryConfigFilePath, repositoryConfigFilePath);

Fixed issues #863 and #870: invalid cache key computation and config file location #892

Fixed issues #863 and #870: invalid cache key computation and config file location #892

Uh oh!

Conversation

sergiorykov commented Jun 4, 2016

Uh oh!

sergiorykov commented Jun 10, 2016

Uh oh!

asbjornu Jun 13, 2016

Choose a reason for hiding this comment

Uh oh!

asbjornu commented Jun 13, 2016

Uh oh!

sergiorykov commented Jun 14, 2016

Uh oh!

asbjornu Jun 14, 2016

Choose a reason for hiding this comment

Uh oh!

asbjornu commented Jun 14, 2016

Uh oh!

sergiorykov commented Jun 19, 2016

Uh oh!

asbjornu Jun 20, 2016

Choose a reason for hiding this comment

Uh oh!

sergiorykov Jul 16, 2016

Choose a reason for hiding this comment

Uh oh!

asbjornu commented Jun 20, 2016

Uh oh!

JakeGinnivan commented Jul 9, 2016

Uh oh!

sergiorykov commented Jul 16, 2016

Uh oh!

sergiorykov commented Jul 16, 2016

Uh oh!

JakeGinnivan commented Jul 17, 2016

Uh oh!

JakeGinnivan commented Jul 17, 2016

Uh oh!

JakeGinnivan commented Jul 17, 2016

Uh oh!

sergiorykov commented Jul 18, 2016

Uh oh!

Uh oh!