Skip to content

add a test to ensure cache key is same even if renormalized #967

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

Closed
wants to merge 4 commits into from

Conversation

tofutim
Copy link
Contributor

@tofutim tofutim commented Jul 22, 2016

Re: #942

If the gitPreparer normalize (in Initialize) is called multiple times, the cachekey will change each time, even though .git/refs has exactly the same content. This is because the cachekey is based on the timestamp rather than the contents. This PR changes the cachekey calculation so that it computes a hash based on the filenames and directory names in .git/refs.

{
// Perform whatever action is required in your scenario.
System.IO.FileInfo fi = new System.IO.FileInfo(file);
result.Add(fi.Name);
Copy link
Contributor

@DanielRose DanielRose Jul 22, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are just using the name of the file. Even if it is modified by normalization, this shouldn't change. What happens if you use the contents of the file?

Copy link
Contributor Author

@tofutim tofutim Jul 22, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right. I'm just using the name - and we could pull in the contents as well to be more precise. Though I think normalization is just bringing in the branches from origin. I could go with using the contents though.

I made the change in the next commit. The only thing is now we are opening a lot of files, and File I/O is always slooooow.

@tofutim
Copy link
Contributor Author

tofutim commented Jul 23, 2016

@rubenmamo Hi Ruben, can you confirm that these changes work in your scenario? Thanks so much.

@JakeGinnivan
Copy link
Contributor

We should probably have a 3.6.2 release right?

I have created support/3.6.0 to allow us to fix issues in the 3.6.x version. Can you re-target this PR to that branch?

@JakeGinnivan
Copy link
Contributor

master is currently v4. But this is a pretty decent bug which I think should be patched in the last release

@tofutim
Copy link
Contributor Author

tofutim commented Jul 24, 2016

@JakeGinnivan can i re-target the PR from this PR or do I need to make a new PR?

@tofutim
Copy link
Contributor Author

tofutim commented Jul 24, 2016

isaacs/github#18 sigh

Update. Sorry guys, I'm going to need a git lesson. Do I cherry pick the changes onto support/3.6.0? Is there a way to rebase on an older branch my changes?

@tofutim tofutim mentioned this pull request Jul 24, 2016
@rubenmamo
Copy link
Contributor

@totufim, apologies for not replying earlier. I wasn't in the office and couldn't check with the latest version. Dynamic repositories are not working in 3.6.1 and I suspect this would be the same for 3.6.2. The new issue is not caused by this change. I have created issue #981 which details the new issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants