Re: [GSoC][RFC] discussion about stashing with conflicts

[Duy Nguyen <pclouds@gmail.com>]

On Mon, Apr 8, 2019 at 12:49 PM Kapil Jain <jkapil.cs@gmail.com> wrote:
>
> On Mon, Apr 8, 2019 at 12:09 AM Thomas Gummerer <t.gummerer@gmail.com> wrote:
> >
> > On 04/07, Kapil Jain wrote:
> > >
> > > what is the use of ce_stage macro ?
> > > tells about stage of an index entry.
> > > if ce_stage says, stage #0 i.e staging area, then that index entry is
> > > in staging area
> > > and nothing needs to be done.
> >
> > I don't quite understand what you mean with "nothing needst to be
> > done" here.  In the context of teaching 'git stash' to handle unmerged
> > index entries, nothing that is not already being done needs to be done
> > with an index entry that is at stage #0.  The current implementation
> > already handles that correctly.
> >
> > > else a temporary index entry is created and repo_read_index_unmerged()
> > > calls other function and tries to add it to index.
> > > if it fails, it issues an error.
> >
> > Not sure what you mean here.  Index entries with higher stages are not
> > temporary, they are written out to the index file, and can then be
> > read back with 'repo_read_index()' for example.
>
> sorry, i failed to provide detailed explanation. below is what i meant.
>
> in repo_read_index_merged(),
> if ce_stage() macro says that this cache_entry is in stage #0 i.e.
> already merged,
> then the function doesn't try to add that entry into index.
>
> if (!ce_stage(ce))
>     continue;
>
> but when it is not in stage #0; the function, creates a temporary cache_entry,
>
> struct cache_entry *new_ce;
> new_ce = make_empty_cache_entry(istate, len);
>
> and tries to add it to index file.
>
> if (add_index_entry(istate, new_ce, ADD_CACHE_SKIP_DFCHECK))
>     return error(_("%s: cannot drop to stage #0"),
>              new_ce->name);
>
> now if this try of adding index entry is successful, then that entry
> is no longer unmerged, right ?
> so can we make `unmerged` variable 0.

There's a big block of comment on top saying return true if unmerged.
As you noticed, after the function returns, there will be no unmerged
entries left anyway. Always returning zero then does not even make
sense.

The main purpose of this function is probably just checking that there
are unmerged entries or not (and return 1 if so). The
add_index_entry() there is probably just to do some more validation.

Sometimes when I don't understand what some code does, I look at "git
log --patch". In this case, there a big explanation in ad3762042a
(read-cache: fix directory/file conflict handling in
read_index_unmerged(), 2018-07-31) that might help you.

Going further back to d147e501f3 (Builtin git-read-tree., 2006-05-23),
you can see that this function has two separate uses. One about the
return code, one about collapsing unmerged entries back to stage zero.
-- 
Duy