-
Notifications
You must be signed in to change notification settings - Fork 651
modify cache key calc to use entire contents of .git/refs instead of timestamp #969
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…fy cache factory to traverse refs directory instead of just looking at the timestamp (which changes if renormalized)
} | ||
|
||
// lifted from https://msdn.microsoft.com/en-us/library/bb513869.aspx | ||
public static List<string> TraverseTree(string root) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this need to be public?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No
On Jul 24, 2016, at 11:07, Asbjørn Ulsberg [email protected] wrote:
In src/GitVersionCore/GitVersionCacheKeyFactory.cs:
return GetHash(dotGitDirectory, lastGitRefsChangedTicks.ToString());
return GetHash(traverse.ToArray());
}
// lifted from https://msdn.microsoft.com/en-us/library/bb513869.aspx
Does this need to be public?public static List<string> TraverseTree(string root)
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub, or mute the thread.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rename to what it actually does, ex. CalculateDirectoryContents
.
How heavy is this? Could we just hash the refs instead as well as HEAD. |
var targetUrl = "https://github.com/GitTools/GitVersion.git"; | ||
var targetBranch = "refs/head/master"; | ||
var gitPreparer = new GitPreparer(targetUrl, null, new Authentication(), false, fixture.RepositoryPath); | ||
//var cacheKey0 = GitVersionCacheKeyFactory.Create(fileSystem, gitPreparer, null); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove this line instead of commenting.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I left it there as a reminder that the cache key changes after Initialize, but ok.
@JakeGinnivan This checks the files and directories in refs only. On my TeamCity (with a bit more logging than in this PR):
22ms for the first run. Afterwards the data is cached by the filesystem or so, and it takes 8ms. Locally:
62ms (first run), afterwards 22ms. |
Thanks @DanielRose, should have looked closer. I thought was more than just refs :P |
We're going to introduce GitVersion and our first ci build (with GitLab CI) took about 47 minutes. Using this PR, the total time reduces to 2.5 minutes. I think this solves the problem. |
{ | ||
files = Directory.GetFiles(currentDir); | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove these empty lines (line 93 as well).
I've fixed up the style issues |
I didn't see the note in rebase that the range is excluding the first one - so it took me a while to figure out how to do it. Ref: #967 (original pull request) - should resolve #942