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

From: Kapil Jain <jkapil.cs@gmail.com>
Date: Apr 8, 2019, 05:48 UTC
Message-ID: <CAMknYEOAg1S8cNYdLPgJemxgikisNpmeuw74T0w+7PUo93stVg@mail.gmail.com>
In-Reply-To: <20190407183857.GA32487@hank.intra.tgummerer.com>

On Mon, Apr 8, 2019 at 12:09 AM Thomas Gummerer <t.gummerer@gmail.com> wrote:

Show 22 quoted lines

>
> 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.

Show 11 quoted lines

> >
> > 1) in repo_read_index_unmerged(), why don't we make the value of
> > `unmerged` 0, if adding index entry is successful; as the entry is no
> > longer unmerged ?
>
> Because the caller often wants to know if the index is unmerged in the
> first place, and would refuse to operate on such an index.  Read the
> comment documenting the function again, that explains this very
> nicely.  Then see how some callers actually use the function, and
> you'll see that they actually don't care about dropping the entry to
> stage 0,

>but they care about knowing whether the index is unmerged or
> not.

if they care about whether the index *is* unmerged, and that call to add_index_entry() is successful, then index is no longer unmerged (at least because of that index_entry). is it possible that they care about if index *was* unmerged ?

> So the question is, did you read this function in depth and understand
> what it does?  If you want to validate your understanding of the
> function, try to repeat what it does in your own words, and ask for us
> to correct you.

upcoming mail will do so.