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

index: canonicalize directory case when adding #3353

Merged
merged 2 commits into from Sep 8, 2015

Conversation

ethomson
Copy link
Member

@ethomson ethomson commented Aug 4, 2015

On case insensitive systems, when given a user-provided path in the
higher-level index addition functions (eg git_index_add_bypath /
git_index_add_frombuffer), examine the index to try to match the
given path to an existing directory.

Various mechanisms can cause the on-disk representation of a folder
to not match the representation in HEAD or the index - for example,
a case changing rename of some file a/file.txt to A/file.txt
will update the paths in the index, but not rename the folder on
disk.

If a user subsequently adds a/other.txt, then this should be stored
in the index as A/other.txt.

For example:

% git init .
% mkdir a
% echo hello > a/file.txt
% git add a
% git commit -mfoo
% git mv -f a/file.txt A/file.txt
% git status
On branch master
Changes to be committed:
  (use "git reset HEAD <file>..." to unstage)

        renamed:    a/file.txt -> A/file.txt

% dir
...snip...
08/04/2015  04:54 PM    <DIR>          a

% echo hi > a/other.txt
% git add a/other.txt
% git status
On branch master
Changes to be committed:
  (use "git reset HEAD <file>..." to unstage)

        new file:   A/other.txt

@ethomson
Copy link
Member Author

ethomson commented Aug 6, 2015

Haha, it turns out that I had my last-minute "only run this on core.ignorecase systems" check backwards. Oops! Corrected, I think that this is ready for review.

cl_git_mkfile("submod2/just_a_dir/file1.txt", "This is a file");
cl_git_mkfile("submod2/just_a_dir/file2.txt", "This is another file");
cl_git_mkfile("submod2/just_a_dir/file3.txt", "This is another file");
cl_git_mkfile("submod2/just_a_dir/file4.txt", "And another file");
Copy link
Member

Choose a reason for hiding this comment

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

This entry isn't looked up in the assertions below.

@ethomson
Copy link
Member Author

ethomson commented Aug 7, 2015

So as you mentioned: there is (was) another mechanism by which we would dup existing entries and keep the original filename as it existed in the index.

I actually think that this is probably not the generally correct behavior: instead, I think that git_index_add should use the given path so that one could perform a case-changing rename. However, I think that git_index_add_bypath and git_index_add_frombuffer should stay friendly to the callers and maintain the path as it exists in the index, if it's already there.

I laid down another commit to update this behavior. Let me know if you don't agree with this plan.

@carlosmn
Copy link
Member

I do like that approach. When you ask to insert a specific entry we do that, otherwise we take the case-sensitivity of the worktree into account.

(you seem to have some conflicts, presumably due to the changelog stuff)

On case insensitive systems, when given a user-provided path in the
higher-level index addition functions (eg `git_index_add_bypath` /
`git_index_add_frombuffer`), examine the index to try to match the
given path to an existing directory.

Various mechanisms can cause the on-disk representation of a folder
to not match the representation in HEAD or the index - for example,
a case changing rename of some file `a/file.txt` to `A/file.txt`
will update the paths in the index, but not rename the folder on
disk.

If a user subsequently adds `a/other.txt`, then this should be stored
in the index as `A/other.txt`.
On case insensitive platforms, allow `git_index_add` to provide a new
path for an existing index entry.  Previously, we would maintain the
case in an index entry without the ability to change it (except by
removing an entry and re-adding it.)

Higher-level functions (like `git_index_add_bypath` and
`git_index_add_frombuffers`) continue to keep the old path for easier
usage.
@ethomson
Copy link
Member Author

ethomson commented Sep 8, 2015

Rebased! I had been waiting on this in case the hash affected anything here. It doesn't, so this should be ready to go.

carlosmn added a commit that referenced this pull request Sep 8, 2015
index: canonicalize directory case when adding
@carlosmn carlosmn merged commit 6d6020d into libgit2:master Sep 8, 2015
max630 pushed a commit to max630/libgit2 that referenced this pull request Aug 28, 2016
index: canonicalize directory case when adding
(cherry picked from commit 6d6020d)

 Conflicts:
	CHANGELOG.md
	tests/index/bypath.c
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