git_merge() / git_index_add() errors with "invalid filemode" on 0664 mode file

[jeffhostetler]

git_index_add() errors with "invalid filemode" on files with mode 0664.
(It does accept files with modes 0644 and 0755.)

This issue came up during a call to git_merge() on Windows.

libgit2_clar.exe!git_index_add(git_index * index, const git_index_entry * source_entry) Line 1151
libgit2_clar.exe!checkout_conflict_add(checkout_data * data, const git_index_entry * conflict) Line 2051
libgit2_clar.exe!checkout_conflict_update_index(checkout_data * data, checkout_conflictdata * conflict) Line 2061
libgit2_clar.exe!checkout_create_conflicts(checkout_data * data) Line 2138
libgit2_clar.exe!git_checkout_iterator(git_iterator * target, git_index * index, const git_checkout_options * opts) Line 2469
libgit2_clar.exe!git_checkout_index(git_repository * repo, git_index * index, const git_checkout_options * opts) Line 2530
libgit2_clar.exe!git_merge(git_repository * repo, const git_annotated_commit * * their_heads, unsigned int their_heads_len, const git_merge_options * merge_opts, const git_checkout_options * given_checkout_opts) Line 2695

In this test:

• git clone https://github.com/git/git.git gitgit
• cd gitgit
• git checkout -b HACK e83c516 (the initial commit)
• echo hello > hello.txt
• git commit -a -m "Added hello"
• call git_merge() to effectively do: "git merge master"

On Windows, this exits with the invalid filemode. Odd that it is in the conflict code at the time. I haven't tried it on other platforms yet.

[ethomson]

Odd that it is in the conflict code at the time.

IIRC, when building the index for merge, we skip the git_index_add (and just build a collection of git_index_entrys ourself.) So if you had a damaged index entry, we would simply maintain it. This seems like the right thing to do, lest we run into errors like this. ;)

[ethomson]

Without even looking at the code, I think that conflicts should call an internal index add function that eschews sanity checks, while the public git_index_add should maintain those checks.

We could also make a public API that does this. I can imagine that a consumer of the library might want to maintain their junky data in the same way. (Maybe they're writing their own git-merge-whatever.) Or we could punt on that for later.

[carlosmn]

A git_index_add_unchecked() would be useful. If we'd want to write one for the conflict handling, it shouldn't be too bad to expose it (maybe in sys/).

[ethomson]

Note that #3032 may be related. Though we should have a unit test for this when we address that.

[carlosmn]

I'm wondering. What does git do in this case? Does it convert the filemode to the modern version? If it does that, it might be worth looking at whether we can take the list of historically-used filemodes and whitelist them to be converted into the modern ones.

[ethomson]

This should be fixed with #3550