Re: [PATCH] wt-status.c: set commitable bit if there is a meaningful merge.

[Stephen & Linda Smith <ischis2@cox.net>]

On Tuesday, February 16, 2016 07:33:54 PM Junio C Hamano wrote:
> > ---
> 
> I think I mislead you into a slightly wrong direction.  While the
> single liner does improve the situation, I think this is merely a
> band-aid upon closer inspection.  For example, if you changed your
> "commit --dry-run" in your test to "commit --dry-run --short", you
> would notice that the test would fail.
> 
I understand.

> In fact, "commit --dry-run" is already broken without this "a merge
> ends up in a no-op" corner case.  The management of s->commitable
> flag and dry_run_commit() that uses it are unfortunately more broken
> than I originally thought.
> 
> If we check for places where s->committable is set, we notice that
> there is only one location: wt_status_print_updated().  This function
> runs an equivalent of "diff-index --cached" and flips s->committable
> on when it sees any difference.
> 
> This function is only called from wt_status_print(), which in turn
> is only called from run_status() in commit.c when the status format
> is unspecified or set to STATUS_FORMAT_LONG.
> 
> So if you do this:
> 
>     $ git reset --hard HEAD
>     $ >a-new-file && git add a-new-file
>     $ git commit --dry-run --short; echo $?
> 
> you'd get "No, there is nothing interesting to commit", which is
> clearly bogus.
> 
> I said s->committable is flipped on only when there is any change in
> "diff-index --cached".  There is nothing that flips it off, by
> noticing that there are unmerged paths, for example.  This is
> another brokenness around "git commit --dry-run".  Imagine that you
> are in a middle of a conflicted cherry-pick.  You did "git add" on a
> resolved path and you still have another path whose conflict has not
> been resolved.  If you run a "git commit --dry-run", you will hear
> "Yes, you can make a meaningful commit", which again is clearly
> bogus.
> 
Makes sense.

> These things need to be eventually fixed, and I think the fix will
> involve revamping how we compute s->committable flag.  Most likely,
> we won't be doing any of that in any wt_status function whose name
> has "print" or "show" in it.  As the original designer of the wt_*
> suite (before these multiple output formats are added), I would say
> everything should happen inside the "collect" phase, if we wanted to
> make s->committable bit usable.
Tonight I started work on a patch to remove the two locations where 
committable was set in  the *print* and *show* functions.  

I believe that what you mean by the "collect" phase is the set of functions 
that are in wt_status.c and have collect in the function name.

> 
> So in the sense, eventually the code updated by this patch will have
> to be discarded when we fix the "commit --dry-run" in the right way,
> but in the meantime, the patch does not make things worse, so let's
> think about queuing it as-is for now as a stop-gap measure.
> 
I'm happy with moveing the patch from pu (where it is now) to next.   I've
re-started my work on this.

> Thanks.