Ensure the index owner is set.

[arthurschreiber]

The index returned by git_merge_trees should be owned by the repository. Otherwise things like git_diff_index_to_workdir will crash when trying to generate a diff with the returned index.

Fixes libgit2/rugged#420.

[arthurschreiber]

@carlosmn This also needs to be backported to all maintenance release lines.

[carlosmn]

This doesn't quite feel right. This index is just one of many which happen to not represent the index file. We would still have the same crash if given an index which the a user has created themselves. That is,

Rugged::Index.new.diff

would crash even like this.

IMO this is an issue with the rugged implementation of Index#diff. In libgit2 we allow you to pass an arbitrary repo and index, so there's no reason for the repo to own the index.

I think that rb_git_index_diff should error out if the zero-arg form of this method is used with an unowned index. In that case, it should expand the "diffable" definition to include a repository.

[arthurschreiber]

@carlosmn The problem is not in Rugged. If you pass an index that is not associated with a repository to git_diff_index_to_workdir, there will be a Segmentation fault coming from libgit2.

It will come from: git_repository_index__weakptr in repository.c:790

   787  
   788      assert(out && repo);
   789  
-> 790      if (repo->_index == NULL) {
   791          git_buf index_path = GIT_BUF_INIT;
   792          git_index *index;
   793  

Here, repo will be null. This happens because the index iterator in git_diff_index_to_workdir will be created from an unowned index and the iterator will not have a repo assigned. Then, iterator__update_ignore_case will try to be set based on the repo's index setting, and accessing a member of the repo (which is null) causes the segfault.

Anyway, there's three issues here. First, I should be able to diff a workdir with an unowned index without
a crash. This is not fixed by this PR, as I guess @ethomson might now better what to do here.

The second issue is that the index returned by git_merge_trees is not owned by the repository, even though it should. It was generated by two trees coming from the repository, why should it not have this link set correctly?

The third is that #diff on an unowned index should raise an error. I'll prepare a fix for rugged for this special case.

[ethomson]

Right, so, per your second question: at present only the repository's index has a pointer back to the repo itself (and only it should really need one). So index->repo == repo->index always. The other indexes could know about the repo they're associated with, and we could have some other indication in the index about whether this is really the repository's index or not, but there are likely other assumptions around this that we make elsewhere in index.c.

That said, you're absolutely right that you should be able to diff an index that isn't the repository's index. However I don't think these indexes having a pointer back to the repository is necessary. We should plumb the caller's repository up into the diff mechanism, I would think, instead of always assuming that index->repo is set.

[arthurschreiber]

Okay, that's fine by me as well. I'll see what needs to be changed and modify this PR accordingly.