Skip to content
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

Teach status to recognize multiple files with identical contents #1550

Merged
merged 3 commits into from Oct 3, 2016

Conversation

ttaylorr
Copy link
Contributor

This pull-request teaches the git lfs status command how to recognize multiple files that share the same contents. By modifying the behavior of lfs.ScanIndex, we now maintain a slice of file metadata associated with a given OID, instead of a one-to-one association (as before).

ScanIndex now only accepts unique pointer OIDs (even if there are multiple pointers for a given OID, in the case of many files sharing the same contents), and then resolves them all at once to multiple files, per the data stored in the indexFileMap.

While working on this, @technoweenie and I found what may be a Git core bug having to do with checking a file that is both in the index and working tree (i.e., it appears in both git diff-index and git diff-index --cached). I stashed a failing test that I wrote to demonstrate this behavior in this Gist.

As an aside, this PR is a temporary measure. Maintaining these extra caches is not a long-term solution that I'd like to keep, this is more of a hack to solve this bug quickly.

/cc @technoweenie @sinbad @rubyist

Copy link
Contributor

@sinbad sinbad left a comment

Choose a reason for hiding this comment

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

I had to double-check the nil behaviour in a couple of cases but this looks fine. 👍

@ttaylorr ttaylorr merged commit e0db9cc into master Oct 3, 2016
@ttaylorr ttaylorr deleted the uniq-status branch October 3, 2016 14:59
@ttaylorr
Copy link
Contributor Author

ttaylorr commented Oct 3, 2016

Thanks for the review! 👋

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.

None yet

2 participants