git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
* git status when merging non-conflicted 3-way merge says "All conflicts fixed"
@ 2021-05-25  7:03 Bagas Sanjaya
  2021-05-26 14:30 ` Elijah Newren
  0 siblings, 1 reply; 7+ messages in thread
From: Bagas Sanjaya @ 2021-05-25  7:03 UTC (permalink / raw)
  To: Git Users

Hi,

Supposed that we have following commit graph:

----A----B----C----D <- master
               \
                ----E <- e

When we merge e branch by `git merge e`, obviously we will do 3-way
merge. Assumed that the merge doesn't conflict, Git will fire up
editor to edit `COMMIT_EDITMSG` for us to enter merge commit
message. Then we abort the commit by either delete all the lines
there, or comment all of them.

But when we check status by `git status`, Git says:

> On branch master
> All conflicts fixed but you are still merging.
>   (use "git commit" to conclude merge)

That message above is misleading, because we know that our merge
doesn't conflict (3-way merge applied successfully without conflict). 
However, it makes sense only when we have resolved all conflicts
on the conflicted merge.

So for non-conflicted merge, we can say instead:

> On branch <branch>
> You are still merging, and the merge applied without any conflicts.
>   (use "git commit" to conclude merge)

Thanks.

-- 
An old man doll... just what I always wanted! - Clara

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: git status when merging non-conflicted 3-way merge says "All conflicts fixed"
  2021-05-25  7:03 git status when merging non-conflicted 3-way merge says "All conflicts fixed" Bagas Sanjaya
@ 2021-05-26 14:30 ` Elijah Newren
  2021-05-26 15:13   ` Phillip Wood
  2021-05-26 16:06   ` Igor Djordjevic
  0 siblings, 2 replies; 7+ messages in thread
From: Elijah Newren @ 2021-05-26 14:30 UTC (permalink / raw)
  To: Bagas Sanjaya; +Cc: Git Mailing List

On Tue, May 25, 2021 at 1:22 AM Bagas Sanjaya <bagasdotme@gmail.com> wrote:
>
> Hi,
>
> Supposed that we have following commit graph:
>
> ----A----B----C----D <- master
>                \
>                 ----E <- e
>
> When we merge e branch by `git merge e`, obviously we will do 3-way
> merge. Assumed that the merge doesn't conflict, Git will fire up
> editor to edit `COMMIT_EDITMSG` for us to enter merge commit
> message. Then we abort the commit by either delete all the lines
> there, or comment all of them.
>
> But when we check status by `git status`, Git says:
>
> > On branch master
> > All conflicts fixed but you are still merging.
> >   (use "git commit" to conclude merge)
>
> That message above is misleading, because we know that our merge
> doesn't conflict (3-way merge applied successfully without conflict).
> However, it makes sense only when we have resolved all conflicts
> on the conflicted merge.

Once upon a time, that message would have always been right.  Then a
--no-commit option was introduced to git merge, and editing of commit
messages for merges was also added.  As you note, both of those can
yield cases where the message is misleading/surprising.

> So for non-conflicted merge, we can say instead:
>
> > On branch <branch>
> > You are still merging, and the merge applied without any conflicts.
> >   (use "git commit" to conclude merge)

At the time this message is printed, there is no way for us to know
whether there had been conflicts.  We'd have to record that
information somewhere (probably the index, though introducing another
index format just for this seems like a really high lift for such a
small thing, and may conflict with other efforts to extend the index
format, such as the sparse-index work), OR re-do the merge when the
user runs status just to find out whether there had been conflicts
(which seems like overkill, and would require you to know which merge
backend had been used and with which flags so you could re-check with
the same one; further, three of the merge backends -- recursive,
resolve, and octopus -- all update the working tree and index and thus
could not be used for a case like this).

Seems like opening a really big can of worms.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: git status when merging non-conflicted 3-way merge says "All conflicts fixed"
  2021-05-26 14:30 ` Elijah Newren
@ 2021-05-26 15:13   ` Phillip Wood
  2021-05-26 16:07     ` Elijah Newren
  2021-05-26 16:06   ` Igor Djordjevic
  1 sibling, 1 reply; 7+ messages in thread
From: Phillip Wood @ 2021-05-26 15:13 UTC (permalink / raw)
  To: Elijah Newren, Bagas Sanjaya; +Cc: Git Mailing List

On 26/05/2021 15:30, Elijah Newren wrote:
> On Tue, May 25, 2021 at 1:22 AM Bagas Sanjaya <bagasdotme@gmail.com> wrote:
>>
>> Hi,
>>
>> Supposed that we have following commit graph:
>>
>> ----A----B----C----D <- master
>>                 \
>>                  ----E <- e
>>
>> When we merge e branch by `git merge e`, obviously we will do 3-way
>> merge. Assumed that the merge doesn't conflict, Git will fire up
>> editor to edit `COMMIT_EDITMSG` for us to enter merge commit
>> message. Then we abort the commit by either delete all the lines
>> there, or comment all of them.
>>
>> But when we check status by `git status`, Git says:
>>
>>> On branch master
>>> All conflicts fixed but you are still merging.
>>>    (use "git commit" to conclude merge)
>>
>> That message above is misleading, because we know that our merge
>> doesn't conflict (3-way merge applied successfully without conflict).
>> However, it makes sense only when we have resolved all conflicts
>> on the conflicted merge.
> 
> Once upon a time, that message would have always been right.  Then a
> --no-commit option was introduced to git merge, and editing of commit
> messages for merges was also added.  As you note, both of those can
> yield cases where the message is misleading/surprising.
> 
>> So for non-conflicted merge, we can say instead:
>>
>>> On branch <branch>
>>> You are still merging, and the merge applied without any conflicts.
>>>    (use "git commit" to conclude merge)
> 
> At the time this message is printed, there is no way for us to know
> whether there had been conflicts.  We'd have to record that
> information somewhere (probably the index, though introducing another
> index format just for this seems like a really high lift for such a
> small thing, and may conflict with other efforts to extend the index
> format, such as the sparse-index work),

Can we use the information that `git update-index --unresolve` uses to 
tell that there were conflicts? I'm not clear when that data gets 
cleared from the index - if it's not cleared when we commit then it wont 
be much use for this.

Best Wishes

Phillip

> OR re-do the merge when the
> user runs status just to find out whether there had been conflicts
> (which seems like overkill, and would require you to know which merge
> backend had been used and with which flags so you could re-check with
> the same one; further, three of the merge backends -- recursive,
> resolve, and octopus -- all update the working tree and index and thus
> could not be used for a case like this).
> 
> Seems like opening a really big can of worms.
> 

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: git status when merging non-conflicted 3-way merge says "All conflicts fixed"
  2021-05-26 14:30 ` Elijah Newren
  2021-05-26 15:13   ` Phillip Wood
@ 2021-05-26 16:06   ` Igor Djordjevic
  2021-05-26 16:08     ` Elijah Newren
  1 sibling, 1 reply; 7+ messages in thread
From: Igor Djordjevic @ 2021-05-26 16:06 UTC (permalink / raw)
  To: Elijah Newren, Bagas Sanjaya; +Cc: Git Mailing List

On 26/05/2021 16:30, Elijah Newren wrote:
> On Tue, May 25, 2021 at 1:22 AM Bagas Sanjaya <bagasdotme@gmail.com> wrote:
> >
> > Hi,
> >
> > Supposed that we have following commit graph:
> >
> > ----A----B----C----D <- master
> >                \
> >                 ----E <- e
> >
> > When we merge e branch by `git merge e`, obviously we will do 3-way
> > merge. Assumed that the merge doesn't conflict, Git will fire up
> > editor to edit `COMMIT_EDITMSG` for us to enter merge commit
> > message. Then we abort the commit by either delete all the lines
> > there, or comment all of them.
> >
> > But when we check status by `git status`, Git says:
> >
> > > On branch master
> > > All conflicts fixed but you are still merging.
> > >   (use "git commit" to conclude merge)
> >
> > That message above is misleading, because we know that our merge
> > doesn't conflict (3-way merge applied successfully without conflict).
> > However, it makes sense only when we have resolved all conflicts
> > on the conflicted merge.
> 
> Once upon a time, that message would have always been right.  Then a
> --no-commit option was introduced to git merge, and editing of commit
> messages for merges was also added.  As you note, both of those can
> yield cases where the message is misleading/surprising.
> 
> > So for non-conflicted merge, we can say instead:
> >
> > > On branch <branch>
> > > You are still merging, and the merge applied without any conflicts.
> > >   (use "git commit" to conclude merge)
> 
> At the time this message is printed, there is no way for us to know
> whether there had been conflicts.  We'd have to record that
> information somewhere (probably the index, though introducing another
> index format just for this seems like a really high lift for such a
> small thing, and may conflict with other efforts to extend the index
> format, such as the sparse-index work), OR re-do the merge when the
> user runs status just to find out whether there had been conflicts
> (which seems like overkill, and would require you to know which merge
> backend had been used and with which flags so you could re-check with
> the same one; further, three of the merge backends -- recursive,
> resolve, and octopus -- all update the working tree and index and thus
> could not be used for a case like this).
> 
> Seems like opening a really big can of worms.

All said, would it be an improvement if current message would be 
simply reworded to say "No conflicts but you are still merging"
(instead of "All conflicts fixed but you are still merging"), thus 
stating the fact (there are no conflicts, indeed) but not implying 
how we got there (whether conflicts were fixed, or there were none to 
begin with)...?

Regards, Buga

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: git status when merging non-conflicted 3-way merge says "All conflicts fixed"
  2021-05-26 15:13   ` Phillip Wood
@ 2021-05-26 16:07     ` Elijah Newren
  2021-05-26 22:54       ` Junio C Hamano
  0 siblings, 1 reply; 7+ messages in thread
From: Elijah Newren @ 2021-05-26 16:07 UTC (permalink / raw)
  To: Phillip Wood; +Cc: Bagas Sanjaya, Git Mailing List

Hi Phillip,

On Wed, May 26, 2021 at 8:13 AM Phillip Wood <phillip.wood123@gmail.com> wrote:
>
> On 26/05/2021 15:30, Elijah Newren wrote:
> > On Tue, May 25, 2021 at 1:22 AM Bagas Sanjaya <bagasdotme@gmail.com> wrote:
> >>
> >> Hi,
> >>
> >> Supposed that we have following commit graph:
> >>
> >> ----A----B----C----D <- master
> >>                 \
> >>                  ----E <- e
> >>
> >> When we merge e branch by `git merge e`, obviously we will do 3-way
> >> merge. Assumed that the merge doesn't conflict, Git will fire up
> >> editor to edit `COMMIT_EDITMSG` for us to enter merge commit
> >> message. Then we abort the commit by either delete all the lines
> >> there, or comment all of them.
> >>
> >> But when we check status by `git status`, Git says:
> >>
> >>> On branch master
> >>> All conflicts fixed but you are still merging.
> >>>    (use "git commit" to conclude merge)
> >>
> >> That message above is misleading, because we know that our merge
> >> doesn't conflict (3-way merge applied successfully without conflict).
> >> However, it makes sense only when we have resolved all conflicts
> >> on the conflicted merge.
> >
> > Once upon a time, that message would have always been right.  Then a
> > --no-commit option was introduced to git merge, and editing of commit
> > messages for merges was also added.  As you note, both of those can
> > yield cases where the message is misleading/surprising.
> >
> >> So for non-conflicted merge, we can say instead:
> >>
> >>> On branch <branch>
> >>> You are still merging, and the merge applied without any conflicts.
> >>>    (use "git commit" to conclude merge)
> >
> > At the time this message is printed, there is no way for us to know
> > whether there had been conflicts.  We'd have to record that
> > information somewhere (probably the index, though introducing another
> > index format just for this seems like a really high lift for such a
> > small thing, and may conflict with other efforts to extend the index
> > format, such as the sparse-index work),
>
> Can we use the information that `git update-index --unresolve` uses to
> tell that there were conflicts? I'm not clear when that data gets
> cleared from the index - if it's not cleared when we commit then it wont
> be much use for this.
>

Wow, I was totally unaware of the resolve-undo extension for the
index.  Thanks for the pointer.  So, it looks like we already _can_
record some kind of specific unmerged information.  Perhaps that
information could be used as a proxy for "were there conflicts",
though I'm still a bit hesitant.  Some questions:
  * Can we rely on the extension being written and populated? (does it
get populated by all relevant codepaths in git?)
  * Even if all codepaths in git will populate it, might the lack of
this extension (or its lack of entries) imply that the user used some
other tool (jgit, something in libgit2, etc.) rather than that there
were no conflicts?
  * Can we rely on this for all types of conflicts?  For all merge
backends?  Are there conflict types that don't result in a higher
stage entry being recorded in the index?

Regarding lack of higher stage entries; some examples of where this
could possibly occur (there may be more, I just spent a few minutes
thinking about it):
* Files/directories in the working directory in the way of paths you
want to write from the merge (merge-recursive will not necessarily
abort early on these due to renames).
* "directory rename split" -- don't know where to rename individual
files added to old dir because half of old dir went to newdir1 and
half to newdir2
* A submodule is fast-forwarded by the merge operation, but updating
the submodule fails due to its working directory being unclean
(theoretical; we don't update submodules currently)
* Would directory/file conflicts (perhaps even combined with
rename/delete) always result in higher stage entries in the index?
I'm not sure if they always have historically; it seems likely that
some historical merge backends (or current or future external merge
tools) might yield a conflict without writing a higher stage entry in
the index for such a type of conflict.
* working tree and gitdir on different filesystems; working tree on a
full filesystem so we can't write out files even though we wrote out
new index and tree and blob objects successfully.

So, I'd still be more comfortable if we could just record some bit
"was_conflicted?" which gets written.  I'd rather not rely on a lack
of the bit being present to imply a yes or a no.


So, still seems like a can of worms to me.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: git status when merging non-conflicted 3-way merge says "All conflicts fixed"
  2021-05-26 16:06   ` Igor Djordjevic
@ 2021-05-26 16:08     ` Elijah Newren
  0 siblings, 0 replies; 7+ messages in thread
From: Elijah Newren @ 2021-05-26 16:08 UTC (permalink / raw)
  To: Igor Djordjevic; +Cc: Bagas Sanjaya, Git Mailing List

On Wed, May 26, 2021 at 9:06 AM Igor Djordjevic
<igor.d.djordjevic@gmail.com> wrote:
>
> On 26/05/2021 16:30, Elijah Newren wrote:
> > On Tue, May 25, 2021 at 1:22 AM Bagas Sanjaya <bagasdotme@gmail.com> wrote:
> > >
> > > Hi,
> > >
> > > Supposed that we have following commit graph:
> > >
> > > ----A----B----C----D <- master
> > >                \
> > >                 ----E <- e
> > >
> > > When we merge e branch by `git merge e`, obviously we will do 3-way
> > > merge. Assumed that the merge doesn't conflict, Git will fire up
> > > editor to edit `COMMIT_EDITMSG` for us to enter merge commit
> > > message. Then we abort the commit by either delete all the lines
> > > there, or comment all of them.
> > >
> > > But when we check status by `git status`, Git says:
> > >
> > > > On branch master
> > > > All conflicts fixed but you are still merging.
> > > >   (use "git commit" to conclude merge)
> > >
> > > That message above is misleading, because we know that our merge
> > > doesn't conflict (3-way merge applied successfully without conflict).
> > > However, it makes sense only when we have resolved all conflicts
> > > on the conflicted merge.
> >
> > Once upon a time, that message would have always been right.  Then a
> > --no-commit option was introduced to git merge, and editing of commit
> > messages for merges was also added.  As you note, both of those can
> > yield cases where the message is misleading/surprising.
> >
> > > So for non-conflicted merge, we can say instead:
> > >
> > > > On branch <branch>
> > > > You are still merging, and the merge applied without any conflicts.
> > > >   (use "git commit" to conclude merge)
> >
> > At the time this message is printed, there is no way for us to know
> > whether there had been conflicts.  We'd have to record that
> > information somewhere (probably the index, though introducing another
> > index format just for this seems like a really high lift for such a
> > small thing, and may conflict with other efforts to extend the index
> > format, such as the sparse-index work), OR re-do the merge when the
> > user runs status just to find out whether there had been conflicts
> > (which seems like overkill, and would require you to know which merge
> > backend had been used and with which flags so you could re-check with
> > the same one; further, three of the merge backends -- recursive,
> > resolve, and octopus -- all update the working tree and index and thus
> > could not be used for a case like this).
> >
> > Seems like opening a really big can of worms.
>
> All said, would it be an improvement if current message would be
> simply reworded to say "No conflicts but you are still merging"
> (instead of "All conflicts fixed but you are still merging"), thus
> stating the fact (there are no conflicts, indeed) but not implying
> how we got there (whether conflicts were fixed, or there were none to
> begin with)...?
>
> Regards, Buga

Ooh, now that seems reasonable.  Much cleaner solution.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: git status when merging non-conflicted 3-way merge says "All conflicts fixed"
  2021-05-26 16:07     ` Elijah Newren
@ 2021-05-26 22:54       ` Junio C Hamano
  0 siblings, 0 replies; 7+ messages in thread
From: Junio C Hamano @ 2021-05-26 22:54 UTC (permalink / raw)
  To: Elijah Newren; +Cc: Phillip Wood, Bagas Sanjaya, Git Mailing List

Elijah Newren <newren@gmail.com> writes:

>   * Can we rely on the extension being written and populated? (does it
> get populated by all relevant codepaths in git?)

The only place record_resolve_undo() gets called is in the deep guts
of in-core index manipulation where a conflicted entry is removed
(either because the resolution decided was to remove the path, or
the resolution is about to add the final contents for the path at
the stage #0), so yes, as long as you are manipulating the in-core
index using our API, you should be recording the action.  Otherwise
we have a bug, as "--unresolve" has to work correctly all the time.

>   * Even if all codepaths in git will populate it, might the lack of
> this extension (or its lack of entries) imply that the user used some
> other tool (jgit, something in libgit2, etc.) rather than that there
> were no conflicts?

It is possible that third-party tools are buggy and corrupt the
on-disk index that way.

>   * Can we rely on this for all types of conflicts?  For all merge
> backends?  Are there conflict types that don't result in a higher
> stage entry being recorded in the index?

Interesting thought.  I am not confident that the recent "directory
rename" stuff does not break the undo information along that line.

But at least, the idea is that resolve-undo should be able to
reproduce the state immediately after any mergy operation stopped
due to conflicts by resurrecting the higher stage entries.

But I do not think "we had a conflict" is so black and white to
begin with.  If you have a rerere record and rerere.autoupdate
enabled (not recommended), does it mean you didn't have a conflict
when rerere kicks in and you did not have to touch anything to
resolve it this time, or was there a conflict but it was
auto-reoslved?  I think the "unresolve" mechanism should still
record the higher stage entries that gets created first before
rerere.autoupdate removes them, which matches the latter
interpretation that I happen to agree with.

For that matter, if both sides have made changes to the same file
nearby, it is more coherent if we declare that there is a conflict
whether the changes "overlap", as the definition of overlapping is
fuzzy and depending on the inter-hunk context used during the mergy
operation, what humans would consider "close but not overlapping"
gets flagged as a "close enough to warrant manual inspection"
conflict (and you can think of rerere as a way to automate that
'manual inspection' step reusing the prior resolution).

So I'd vote for "No conflicts remain, but you haven't committed",
which would be the most sensible way to phrase the situation.

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2021-05-26 22:54 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-25  7:03 git status when merging non-conflicted 3-way merge says "All conflicts fixed" Bagas Sanjaya
2021-05-26 14:30 ` Elijah Newren
2021-05-26 15:13   ` Phillip Wood
2021-05-26 16:07     ` Elijah Newren
2021-05-26 22:54       ` Junio C Hamano
2021-05-26 16:06   ` Igor Djordjevic
2021-05-26 16:08     ` Elijah Newren

Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).