Re: [PATCH v3] git-apply: apply submodule changes

[Sven Verdoolaege <skimo@liacs.nl>]

On Sun, Aug 12, 2007 at 12:24:29PM -0700, Junio C Hamano wrote:
> Sven Verdoolaege <skimo@kotnet.org> writes:
> 
> >>  - what does ce have to do in this codepath?  read_old_data()
> >>    does not care about what is in the index (in fact, in the
> >>    index the entry can be a symlink when the path on the
> >>    filesystem is a regular file, and it reads from the regular
> >>    file as asked--it does not even look at ce by design).  
> >>    if you have a regular file there in the current version, ce
> >>    would say it is a regular file blob and you would not want
> >>    read_gitlink_or_skip() to say "Subproject commit xyz...".
> >
> > Hmmm... the documentation says that if --index is in effect
> > then the file to be patched in the work tree is supposed to be 
> > up-to-date.
> 
> But that is the job of check_patch(), not this function, isn't it?

Ah... so you want me to check that there has been no type change
to the submodule in check_patch (and then I can use my
read_gitlink_or_skip as is).  Is that right?

> >> The type-mismatch case to attempt to apply gitlink patch to a
> >> regular blob is covered much earlier in check_patch().  It
> >> complains if st_mode does not match patch->old_mode; I think you
> >> need to adjust it a bit to:
> >> 
> >>  - allow gitlink patch to a path that currently has nothing (no
> >>    submodule checked out) or a directory that has ".git/"
> >>    (i.e. submodule checked out).
> >> 
> >>  - reject gitlink patch otherwise.
> >
> > Are you talking about the case where --index is specified?
> 
> Talking about both cases, and the division of responsibility
> between check_patch() and apply_data().

I'll do that for the --index case, but I really think it doesn't
make sense for the other case.  If we're not in a git repo,
then the submodule, which may very well be present, is not
going to be a git repo either.

What could make sense is to enforce that in the --index case,
the submodule directory is either empty or a git repo and
that in the non --index case it is empty.

skimo