Index updates vs. submodules

[tiennou]

I'm trying to port GitX index-wrangling code over to Objective-Git, and I'm getting back an error when calling (indirectly) git_index_update_all : "Failed to read file into stream: Is a directory". After taking a closer look, that error happens here because the path being processed is actually a changed submodule and not a plain file, so the update fails.

Am I'm doing it wrong, or is it not handled ?

[tiennou]

I think I could be checking beforehand if git_submodule_lookup has anything to say about that path, and then do a git_submodule_add_to_index instead of calling git_index_add_bypath, but I'm thinking it's a little heavy-handed...

[arrbee]

Yes, this is an annoying bug! Since this is an update, we actually have the entry->mode value available to us so we can check if the entry being updated in a submodule without a call. That would be a simple fix.

Of course, there are a variety of more complex cases that we're probably not dealing with correctly related to converting a submodule to a plain directory and other "type change" cases. For example, if you have a plain file in the index and replace it with a directory containing other files, then I suspect you'll get this same error regardless of submodules.

One approach is to replace that case with simply:

            error = git_index_add_bypath(index, path.ptr);
            if (error) {
                giterr_clear();

                error = git_index_remove_bypath(index, path.ptr);
                if (!error) /* back up foreach if we removed this */
                    i--;
            }

so instead of just ignoring GIT_ENOTFOUND we ignore any error from git_index_add_bypath. I have mixed feelings about this approach. Really we should compare with what Git would do in these various cases to make sure we're emulating it correctly.

Anyhow, good bug report! Thanks.

[tiennou]

Okay, I'll check what git does then. Talking about approaches, what would be the best way to handle those — I'm under the impression that that approach just ignores submodules, right ?

Right now the underlying fail happens in index_entry_init, because git_blob__create_from_paths doesn't like directories, so I'm inclined to add a few S_ISGITLINK checks in index.c that call submodules stuff so it works "as expected".

[arrbee]

Hmm, it looks like a few changes will be needed to accurately match git add -u here.

It appears that submodules are essentially ignored -- so if a submodule has been typechanged to something else, the new thing will be inserted into the index, and if something is typechanged to a submodule, the thing will be treated as deleted.

If something in the index was a file and has been replaced by a directory, it will be treated as deleted.

Unfortunately, it seems like it will be a little challenging to emulate these behaviors without adding an extra stat call to every item in the index - they currently are stated once and this would mean calling that twice (or significantly refactoring git_index_add_bypath / index_entry_init which is used to fill in a proposed updated index entry).

[carlosmn]

Is this issue still relevant?

[tiennou]

Yep, just checked and it still chokes on submodules.

[carlosmn]

Fixed (a while ago) via #3303