Re: [PATCH 16/18] entry.c: update submodules when interesting

[Stefan Beller <sbeller@google.com>]

On Tue, Mar 7, 2017 at 3:04 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Junio C Hamano <gitster@pobox.com> writes:
>
>>> +                    if (!is_submodule_populated_gently(ce->name, &err)) {
>>> +                            struct stat sb;
>>> +                            if (lstat(ce->name, &sb))
>>> +                                    die(_("could not stat file '%s'"), ce->name);
>>> +                            if (!(st.st_mode & S_IFDIR))
>>> +                                    unlink_or_warn(ce->name);
>>
>> We found that the path ce->name is supposed to be a submodule that
>> is checked out, we found that the submodule is not yet populated, we
>> tried to make sure what is on that path, and its failure would cause
>> us to die().  All that is sensible.
>> ...
>> But if that unlink fails, shouldn't we die, just like the case where
>> we cannot tell what is at the path ce->name?
>>
>> And if that unlink succeeds, instead of having an empty directory,
>> we start the "move-head" call to switch from NULL to ce->oid without
>> having any directory.  Wouldn't we want to mkdir() here (and remove
>> mkdir() in submodule_move_head() if there is one---if there isn't
>> then I do not think this codepath would work)?
>
> In addition to mkdir(), would we also need the .git file that point
> into superproject's .git/modules/ too?

The move_head function takes care of it
Both creation as well as deletion are handled in the move_head function,
when either new or old is NULL.

We can drag that out of that function and have it at the appropriate places
manually.

Why do you think it is better design to have mkdir here and not in move_head?
(For me having everything in move_head was easier to understand,
maybe it is not for others)

Thanks,
Stefan