git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* commiting while the current version is in conflict
@ 2008-10-16 22:10 Richard Hartmann
  2008-10-16 22:48 ` Miklos Vajna
  2008-10-16 23:42 ` Jakub Narebski
  0 siblings, 2 replies; 17+ messages in thread
From: Richard Hartmann @ 2008-10-16 22:10 UTC (permalink / raw)
  To: git

Hi all,

I fooled around with git a liitle bit and noticed something
rather strange. I merged two branches, creating a conflict
on purpose. When I then did a

 git commit -a

all changes were submitted. Of course, I now have a
file with the conflict markers inlined in my repository. Not
a good thing, imo. Is there a way to make git block all
conflicting versions?

Also, I would be interested in the design decissions
behind the current behaviour. Any pointers?


Thanks,
Richard

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

* Re: commiting while the current version is in conflict
  2008-10-16 22:10 Richard Hartmann
@ 2008-10-16 22:48 ` Miklos Vajna
  2008-10-16 23:00   ` Shawn O. Pearce
  2008-10-16 23:07   ` Richard Hartmann
  2008-10-16 23:42 ` Jakub Narebski
  1 sibling, 2 replies; 17+ messages in thread
From: Miklos Vajna @ 2008-10-16 22:48 UTC (permalink / raw)
  To: Richard Hartmann; +Cc: git

[-- Attachment #1: Type: text/plain, Size: 650 bytes --]

On Fri, Oct 17, 2008 at 12:10:55AM +0200, Richard Hartmann <richih.mailinglist@gmail.com> wrote:
> all changes were submitted. Of course, I now have a
> file with the conflict markers inlined in my repository. Not
> a good thing, imo. Is there a way to make git block all
> conflicting versions?

Write a pre-commit hook that checks for conflict markers?

> Also, I would be interested in the design decissions
> behind the current behaviour. Any pointers?

Not sure, but in general blocking conflict markers by default would be a
bad idea IMHO, several markup language (asciidoc, for example) makes use
of the >>>, === and such character sequences.

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

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

* Re: commiting while the current version is in conflict
  2008-10-16 22:48 ` Miklos Vajna
@ 2008-10-16 23:00   ` Shawn O. Pearce
  2008-10-16 23:26     ` Richard Hartmann
  2008-10-16 23:07   ` Richard Hartmann
  1 sibling, 1 reply; 17+ messages in thread
From: Shawn O. Pearce @ 2008-10-16 23:00 UTC (permalink / raw)
  To: Miklos Vajna; +Cc: Richard Hartmann, git

Miklos Vajna <vmiklos@frugalware.org> wrote:
> On Fri, Oct 17, 2008 at 12:10:55AM +0200, Richard Hartmann <richih.mailinglist@gmail.com> wrote:
> > all changes were submitted. Of course, I now have a
> > file with the conflict markers inlined in my repository. Not
> > a good thing, imo. Is there a way to make git block all
> > conflicting versions?
> 
> Write a pre-commit hook that checks for conflict markers?

The sample pre-commit hook checks for these.  Its really hande to
have enabled.
 
> > Also, I would be interested in the design decissions
> > behind the current behaviour. Any pointers?
> 
> Not sure, but in general blocking conflict markers by default would be a
> bad idea IMHO, several markup language (asciidoc, for example) makes use
> of the >>>, === and such character sequences.

Not only that, but "git commit -a" did exactly what you asked it to do:

	git add -u
	git commit

and git add -u is basically a faster way to do something like this pseudo-shell:

	for path in $(git status | grep modified:)
	do
		git add $path
	done

and merge conflicts are "resolved" by you running "git add $path"
after you have finished fixing that path.

Moral of the story is, don't use "git commit -a".  Use only "git commit"
and stage files individually.  That way when you are in a merge conflict
you won't be in the habit of writing "git commit -a" and staging everything
from the working tree implicitly.

-- 
Shawn.

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

* Re: commiting while the current version is in conflict
  2008-10-16 22:48 ` Miklos Vajna
  2008-10-16 23:00   ` Shawn O. Pearce
@ 2008-10-16 23:07   ` Richard Hartmann
  2008-10-16 23:23     ` Shawn O. Pearce
  1 sibling, 1 reply; 17+ messages in thread
From: Richard Hartmann @ 2008-10-16 23:07 UTC (permalink / raw)
  To: Miklos Vajna; +Cc: git

On Fri, Oct 17, 2008 at 00:48, Miklos Vajna <vmiklos@frugalware.org> wrote:

> Not sure, but in general blocking conflict markers by default would be a
> bad idea IMHO, several markup language (asciidoc, for example) makes use
> of the >>>, === and such character sequences.

Doesn't git keep metadata about conflicts, as well?


Richard

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

* Re: commiting while the current version is in conflict
  2008-10-16 23:07   ` Richard Hartmann
@ 2008-10-16 23:23     ` Shawn O. Pearce
  2008-10-16 23:31       ` Richard Hartmann
  0 siblings, 1 reply; 17+ messages in thread
From: Shawn O. Pearce @ 2008-10-16 23:23 UTC (permalink / raw)
  To: Richard Hartmann; +Cc: Miklos Vajna, git

Richard Hartmann <richih.mailinglist@gmail.com> wrote:
> On Fri, Oct 17, 2008 at 00:48, Miklos Vajna <vmiklos@frugalware.org> wrote:
> 
> > Not sure, but in general blocking conflict markers by default would be a
> > bad idea IMHO, several markup language (asciidoc, for example) makes use
> > of the >>>, === and such character sequences.
> 
> Doesn't git keep metadata about conflicts, as well?

Yes, in the index.  But it erases it when you stage the file with
"git add".

Go look at my prior message about how "git commit -a" is staging
the files prior to commit.  That makes git commit think everything
has been resolved, because you've told git, everything is resolved.

-- 
Shawn.

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

* Re: commiting while the current version is in conflict
  2008-10-16 23:00   ` Shawn O. Pearce
@ 2008-10-16 23:26     ` Richard Hartmann
  2008-10-17  1:16       ` Avery Pennarun
  0 siblings, 1 reply; 17+ messages in thread
From: Richard Hartmann @ 2008-10-16 23:26 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: Miklos Vajna, git

On Fri, Oct 17, 2008 at 01:00, Shawn O. Pearce <spearce@spearce.org> wrote:

> The sample pre-commit hook checks for these.  Its really hande to
> have enabled.

Thanks.


> and merge conflicts are "resolved" by you running "git add $path"
> after you have finished fixing that path.

True, git add is an implicit resolving, I did not think about it this way.
Personally, I think that git should break at this point, but that's
just me.

The obvious fix would be a pre-add hook. Does anyone else think
this would make sense?


Judging from the code in the pre-commit script, git does not
keep conflict information in its metadata cache, but tries to guess
conflicts from the file's contents/ This seems to be a strange
thing to do, imo. What's the reason for this?


Richard

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

* Re: commiting while the current version is in conflict
  2008-10-16 23:23     ` Shawn O. Pearce
@ 2008-10-16 23:31       ` Richard Hartmann
  0 siblings, 0 replies; 17+ messages in thread
From: Richard Hartmann @ 2008-10-16 23:31 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: Miklos Vajna, git

On Fri, Oct 17, 2008 at 01:23, Shawn O. Pearce <spearce@spearce.org> wrote:


> Yes, in the index.  But it erases it when you stage the file with
> "git add".
>
> Go look at my prior message about how "git commit -a" is staging
> the files prior to commit.  That makes git commit think everything
> has been resolved, because you've told git, everything is resolved.

This makes a good part of my latest email obsolete. Asynchronous
communication ftw ;)


Richard

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

* Re: commiting while the current version is in conflict
@ 2008-10-16 23:39 Junio Hamano
  2008-10-17  7:21 ` Richard Hartmann
  0 siblings, 1 reply; 17+ messages in thread
From: Junio Hamano @ 2008-10-16 23:39 UTC (permalink / raw)
  To: Richard Hartmann; +Cc: Shawn O. Pearce, Miklos Vajna, git

> Judging from the code in the pre-commit script, git does not
> keep conflict information in its metadata cache, but tries to guess
> conflicts from the file's contents/ This seems to be a strange
> thing to do, imo. What's the reason for this?

Because 

 (0) You are wrong to assume that git does not keep conflict
     information; we can tell if the index is "unmerged" to see
     if you still have unresolved conflicts;

 (1) When the index is unmerged, git-commit will stop even
     before getting to pre-commit hook, so there is no point
     for pre-commit hook to check if the index is unmerged;

 (2) pre-commit hook is a last ditch effort to help ignorant
     users who have already done "git add" without thinking and
     lost the "unmerged" state.  It has to look at and guess at
     the contents for that.

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

* Re: commiting while the current version is in conflict
  2008-10-16 22:10 Richard Hartmann
  2008-10-16 22:48 ` Miklos Vajna
@ 2008-10-16 23:42 ` Jakub Narebski
  2008-10-17  7:25   ` Richard Hartmann
  1 sibling, 1 reply; 17+ messages in thread
From: Jakub Narebski @ 2008-10-16 23:42 UTC (permalink / raw)
  To: Richard Hartmann; +Cc: git

"Richard Hartmann" <richih.mailinglist@gmail.com> writes:

> I fooled around with git a liitle bit and noticed something
> rather strange. I merged two branches, creating a conflict
> on purpose. When I then did a
> 
>  git commit -a
> 
> all changes were submitted. Of course, I now have a
> file with the conflict markers inlined in my repository. Not
> a good thing, imo. Is there a way to make git block all
> conflicting versions?

The default pre-commit hook shipped with git includes this check.  
All you have to do is enable it: it has to be named 'pre-commit' and
it has to be exacutable.  In older git version you would do:
  $ chmod a+x .git/hooks/pre-commit
while in never versions it would be
  $ mv .git/hooks/pre-commit.sample .git/hooks/pre-commit
 
> Also, I would be interested in the design decissions
> behind the current behaviour. Any pointers?

It is a git policy that it ships with all hooks disabled.

P.S. You can always bypass pre-commit hook (for example if comitting
AsciiDoc files, which may include something that looks like conflict
markers, or if you are committing test which includes conflicted file
as one of test vectors), by using `--no-verify' option to git-commit.

-- 
Jakub Narebski
Poland
ShadeHawk on #git

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

* Re: commiting while the current version is in conflict
  2008-10-16 23:26     ` Richard Hartmann
@ 2008-10-17  1:16       ` Avery Pennarun
  0 siblings, 0 replies; 17+ messages in thread
From: Avery Pennarun @ 2008-10-17  1:16 UTC (permalink / raw)
  To: Richard Hartmann; +Cc: Shawn O. Pearce, Miklos Vajna, git

On Thu, Oct 16, 2008 at 7:26 PM, Richard Hartmann
<richih.mailinglist@gmail.com> wrote:
> On Fri, Oct 17, 2008 at 01:00, Shawn O. Pearce <spearce@spearce.org> wrote:
>> and merge conflicts are "resolved" by you running "git add $path"
>> after you have finished fixing that path.
>
> True, git add is an implicit resolving, I did not think about it this way.
> Personally, I think that git should break at this point, but that's
> just me.
>
> The obvious fix would be a pre-add hook. Does anyone else think
> this would make sense?

That would be awesome.  I've frequently been bitten by accidentally
running "git add" on a file that I *think* I've resolved the conflicts
on, but it turns out I missed a commit marker or two.  And then the
handy conflict-style "git diff" is no longer available, among other
things.  If "git add" could prevent me from adding the file at all, it
might save me some trouble.

On the other hand, an even better alternative would be to have a way
to "unadd" a file and bring back the conflict information, but I don't
know how that would really work.

Have fun,

Avery

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

* Re: commiting while the current version is in conflict
  2008-10-16 23:39 commiting while the current version is in conflict Junio Hamano
@ 2008-10-17  7:21 ` Richard Hartmann
  2008-10-17  8:37   ` Junio C Hamano
  2008-10-17  9:16   ` Jakub Narebski
  0 siblings, 2 replies; 17+ messages in thread
From: Richard Hartmann @ 2008-10-17  7:21 UTC (permalink / raw)
  To: Junio Hamano; +Cc: Shawn O. Pearce, Miklos Vajna, git

On Fri, Oct 17, 2008 at 01:39, Junio Hamano <junio@twinsun.com> wrote:

>  (0) You are wrong to assume that git does not keep conflict
>     information; we can tell if the index is "unmerged" to see
>     if you still have unresolved conflicts;

That information is already lost at the point when the
pre-commit hook is executed. Thus, I more or less had to
arrive at the wrong conclusion :)


>  (1) When the index is unmerged, git-commit will stop even
>     before getting to pre-commit hook, so there is no point
>     for pre-commit hook to check if the index is unmerged;

I realize this, now.


>  (2) pre-commit hook is a last ditch effort to help ignorant
>     users who have already done "git add" without thinking and
>     lost the "unmerged" state.  It has to look at and guess at
>     the contents for that.

Ignoring the ad hominem attack, I would argue that the two
distinct mental concepts of 'I want to commit this in the next
commit' and 'I have resolved this conflict' should have two
distinct commands.
To err is human, which is why rm -i exists. Else, you could
just use alias rm='rm -rf'.

Also, within certain boundaries, a tool should adapt to the
user, not vice versa.

In my opinion, a pre-add hook which is able to warn the user
that something they are about to add is still in conflict would
be best. The one piece of feedback I had up to now was
very positive.


Richard

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

* Re: commiting while the current version is in conflict
  2008-10-16 23:42 ` Jakub Narebski
@ 2008-10-17  7:25   ` Richard Hartmann
  0 siblings, 0 replies; 17+ messages in thread
From: Richard Hartmann @ 2008-10-17  7:25 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: git

On Fri, Oct 17, 2008 at 01:42, Jakub Narebski <jnareb@gmail.com> wrote:

> [a lot of detailed information]

Thanks! 'Unfortunately', I figured all this out by myself, in the
meantime, but I really appreciate that you took the time to
explain all this .


Thanks!
Richard

PS: Well, I lie, I did not figure out the mv $1.sample $1 bit
as my git version still uses the old format.

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

* Re: commiting while the current version is in conflict
  2008-10-17  7:21 ` Richard Hartmann
@ 2008-10-17  8:37   ` Junio C Hamano
  2008-10-17  9:32     ` Richard Hartmann
  2008-10-17  9:16   ` Jakub Narebski
  1 sibling, 1 reply; 17+ messages in thread
From: Junio C Hamano @ 2008-10-17  8:37 UTC (permalink / raw)
  To: Richard Hartmann; +Cc: Shawn O. Pearce, Miklos Vajna, git

"Richard Hartmann" <richih.mailinglist@gmail.com> writes:

>>  (2) pre-commit hook is a last ditch effort to help ignorant
>>     users who have already done "git add" without thinking and
>>     lost the "unmerged" state.  It has to look at and guess at
>>     the contents for that.
>
> Ignoring the ad hominem attack, I would argue that the two

Eh, Sorry about that --- I did not mean "ignorant" in that sense.  Perhaps
replacing the word with "unfortunate" would sound nicer?

> To err is human, which is why rm -i exists. Else, you could
> just use alias rm='rm -rf'.
>
> Also, within certain boundaries, a tool should adapt to the
> user, not vice versa.

Don't you realize that is what the hook is doing already?  After making
such an error, the definitive information is lost, because the user told
the tool that the resolution is done and the file is ready to be
committed) by mistake.  Even then the hook is trying its best to help the
user.

Replace your "rm=rm -rf" sentence with "why undelete exists" and read it
again.  No, filesystem undelete does not always work (disk block could
have been recycled for other files), but it could help you when you did
remove a file you wanted to keep by mistake when it can.

As to pre-add hook, I am not enthused, but if somebody sends in a clean
patch, I wouldn't be opposed to it at least in principle.

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

* Re: commiting while the current version is in conflict
  2008-10-17  7:21 ` Richard Hartmann
  2008-10-17  8:37   ` Junio C Hamano
@ 2008-10-17  9:16   ` Jakub Narebski
  2008-10-17  9:35     ` Richard Hartmann
  2008-10-17  9:36     ` Junio C Hamano
  1 sibling, 2 replies; 17+ messages in thread
From: Jakub Narebski @ 2008-10-17  9:16 UTC (permalink / raw)
  To: Richard Hartmann; +Cc: Junio Hamano, Shawn O. Pearce, Miklos Vajna, git

"Richard Hartmann" <richih.mailinglist@gmail.com> writes:

> On Fri, Oct 17, 2008 at 01:39, Junio Hamano <junio@twinsun.com> wrote:

> >  (2) pre-commit hook is a last ditch effort to help ignorant
> >     users who have already done "git add" without thinking and
> >     lost the "unmerged" state.  It has to look at and guess at
> >     the contents for that.
> 
> Ignoring the ad hominem attack, I would argue that the two
> distinct mental concepts of 'I want to commit this in the next
> commit' and 'I have resolved this conflict' should have two
> distinct commands.
>
> To err is human, which is why rm -i exists. Else, you could
> just use alias rm='rm -rf'.

>From time to time somebody proposes to add a command which is used
only to say that given conflict got resolved, i.e. yet another
porcelain "around" git-update-index plumbing (in addition to git-add,
git-mv and git-rm).  One of problems is how to call it: git-resolve,
git-resolved, git-mark-resolved?

BTW. while I usually use "git commit -a", when comitting merge commit
I usually use explicit "git add" together "git commit" (without '-a').
-- 
Jakub Narebski
Poland
ShadeHawk on #git

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

* Re: commiting while the current version is in conflict
  2008-10-17  8:37   ` Junio C Hamano
@ 2008-10-17  9:32     ` Richard Hartmann
  0 siblings, 0 replies; 17+ messages in thread
From: Richard Hartmann @ 2008-10-17  9:32 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Shawn O. Pearce, Miklos Vajna, git

On Fri, Oct 17, 2008 at 10:37, Junio C Hamano <gitster@pobox.com> wrote:

> Eh, Sorry about that --- I did not mean "ignorant" in that sense.  Perhaps
> replacing the word with "unfortunate" would sound nicer?

A lot yes. No harm done, apologies for picking the bad interpretation
of ignorant.


> Don't you realize that is what the hook is doing already?  After making
> such an error, the definitive information is lost, because the user told
> the tool that the resolution is done and the file is ready to be
> committed) by mistake.  Even then the hook is trying its best to help the
> user.

You misunderstood me, there. I was thinking about the pre-add hook
while writing the above. Doing anything at commit time is obviously
wrong.


> As to pre-add hook, I am not enthused, but if somebody sends in a clean
> patch, I wouldn't be opposed to it at least in principle.

An implicit need for it does apparently exist. Else, the default hook for
pre-commit would not need to check for conflicts.


Richard

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

* Re: commiting while the current version is in conflict
  2008-10-17  9:16   ` Jakub Narebski
@ 2008-10-17  9:35     ` Richard Hartmann
  2008-10-17  9:36     ` Junio C Hamano
  1 sibling, 0 replies; 17+ messages in thread
From: Richard Hartmann @ 2008-10-17  9:35 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: Junio Hamano, Shawn O. Pearce, Miklos Vajna, git

On Fri, Oct 17, 2008 at 11:16, Jakub Narebski <jnareb@gmail.com> wrote:

> From time to time somebody proposes to add a command which is used
> only to say that given conflict got resolved, i.e. yet another
> porcelain "around" git-update-index plumbing (in addition to git-add,
> git-mv and git-rm).  One of problems is how to call it: git-resolve,
> git-resolved, git-mark-resolved?

In that case, you would still need a pre-add hook.
The problem is not that people can't mark resolved without adding.
It's that they can't add without checking (automatically) if there
are no conflicts.


Richard

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

* Re: commiting while the current version is in conflict
  2008-10-17  9:16   ` Jakub Narebski
  2008-10-17  9:35     ` Richard Hartmann
@ 2008-10-17  9:36     ` Junio C Hamano
  1 sibling, 0 replies; 17+ messages in thread
From: Junio C Hamano @ 2008-10-17  9:36 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: Richard Hartmann, Shawn O. Pearce, Miklos Vajna, git

Jakub Narebski <jnareb@gmail.com> writes:

> From time to time somebody proposes to add a command which is used
> only to say that given conflict got resolved, i.e. yet another
> porcelain "around" git-update-index plumbing (in addition to git-add,
> git-mv and git-rm).  One of problems is how to call it: git-resolve,
> git-resolved, git-mark-resolved?
>
> BTW. while I usually use "git commit -a", when comitting merge commit
> I usually use explicit "git add" together "git commit" (without '-a').

There are three things to keep in mind while thinking about this:

 * "git add" _always_ is to mark "this path now is in a good shape and is
   ready to be committed", whether you are doing a conflict resolution of
   a merge or making a normal commit.

 * you cannot partially commit a merge, as the resulting tree won't be a
   proper merge (i.e. the change from either parents do not describe what
   happened).

 * during a conflicted merge, cleanly merged paths are already staged in
   the index.

Which means that the only paths you would "git add" during a conflicted
merge are the paths you resolved (unless you are creating an evil merge),
and there is no point having a separate "git resolved" -- such a command
will be nothing but an alias to "git add" anyway.

We could add a training wheel mode to "git commit -a" (or "git add -u")
that warns about unmerged paths and asks confirmation, but I suspect that
it would really annoy people who used git for more than 2 weeks if we made
it the default, and on the other hand if it is not the default, it would
not help new people that much.  It is nice to try to help new people from
shooting themselves in the foot, but we need to draw a line somewhere so
that we do not hurt people by being obnoxious.  After all, even these new
people will graduate from the "new" status soon.  My gut feeling is that
helping "oh, I staged the file that I wasn't ready to commit" is on the
other side of that line, especially since the example pre-commit hook
safety is easily available.  If somebody wants to add a pre-add hook that
is run by "git add" Porcelain (but never by "update-index"), that would
add the safety net, too.

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

end of thread, other threads:[~2008-10-17  9:38 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-10-16 23:39 commiting while the current version is in conflict Junio Hamano
2008-10-17  7:21 ` Richard Hartmann
2008-10-17  8:37   ` Junio C Hamano
2008-10-17  9:32     ` Richard Hartmann
2008-10-17  9:16   ` Jakub Narebski
2008-10-17  9:35     ` Richard Hartmann
2008-10-17  9:36     ` Junio C Hamano
  -- strict thread matches above, loose matches on Subject: below --
2008-10-16 22:10 Richard Hartmann
2008-10-16 22:48 ` Miklos Vajna
2008-10-16 23:00   ` Shawn O. Pearce
2008-10-16 23:26     ` Richard Hartmann
2008-10-17  1:16       ` Avery Pennarun
2008-10-16 23:07   ` Richard Hartmann
2008-10-16 23:23     ` Shawn O. Pearce
2008-10-16 23:31       ` Richard Hartmann
2008-10-16 23:42 ` Jakub Narebski
2008-10-17  7:25   ` Richard Hartmann

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 NNTP newsgroup(s).