Commit hook annoyance

Commit hook annoyance

[Eli Zaretskii]

Is this really necessary?

  $ git push
  Counting objects: 94, done.
  Compressing objects: 100% (4/4), done.
  Writing objects: 100% (4/4), 742 bytes | 0 bytes/s, done.
  Total 4 (delta 3), reused 0 (delta 0)
  remote: *** Pattern "Conflicts:" has been detected.
  remote: *** (in commit 7fa8e2a29c9357024d21bdf09faa5cb930a4ffe5)
  remote: ***
  remote: *** This usually indicates a merge commit where some merge conflicts
  remote: *** had to be resolved, but where the "Conflicts:" section has not
  remote: *** been deleted from the revision history.
  remote: ***
  remote: *** Please edit the commit's revision history to either delete
  remote: *** the section, or to avoid using the pattern above by itself.
  remote: error: hook declined to update refs/heads/gdb-7.9-branch
  To git+ssh://sourceware.org/git/binutils-gdb.git
   ! [remote rejected] gdb-7.9-branch -> gdb-7.9-branch (hook declined)
  error: failed to push some refs to 'git+ssh://sourceware.org/git/binutils-gdb.git'

There  was no unresolved conflicts in the commit I tried to push.  Why
does the hook insist on forcing me to remove the "Conflicts:" part,
instead of looking for the merge-commit conflict markers?  Why do we
care about leaving the reference to original conflicts in the log?

Re: Commit hook annoyance

[H.J. Lu]

On Fri, Jan 16, 2015 at 8:00 AM, Eli Zaretskii <eliz@gnu.org> wrote:
> Is this really necessary?
>
>   $ git push
>   Counting objects: 94, done.
>   Compressing objects: 100% (4/4), done.
>   Writing objects: 100% (4/4), 742 bytes | 0 bytes/s, done.
>   Total 4 (delta 3), reused 0 (delta 0)
>   remote: *** Pattern "Conflicts:" has been detected.
>   remote: *** (in commit 7fa8e2a29c9357024d21bdf09faa5cb930a4ffe5)
>   remote: ***
>   remote: *** This usually indicates a merge commit where some merge conflicts
>   remote: *** had to be resolved, but where the "Conflicts:" section has not
>   remote: *** been deleted from the revision history.
>   remote: ***
>   remote: *** Please edit the commit's revision history to either delete
>   remote: *** the section, or to avoid using the pattern above by itself.
>   remote: error: hook declined to update refs/heads/gdb-7.9-branch
>   To git+ssh://sourceware.org/git/binutils-gdb.git
>    ! [remote rejected] gdb-7.9-branch -> gdb-7.9-branch (hook declined)
>   error: failed to push some refs to 'git+ssh://sourceware.org/git/binutils-gdb.git'
>
> There  was no unresolved conflicts in the commit I tried to push.  Why
> does the hook insist on forcing me to remove the "Conflicts:" part,
> instead of looking for the merge-commit conflict markers?  Why do we
> care about leaving the reference to original conflicts in the log?

Do you have a commit which has a conflict and is fixed by another
commit?

-- 
H.J.

Re: Commit hook annoyance

[Joel Brobecker]

> Is this really necessary?
> 
>   $ git push
>   Counting objects: 94, done.
>   Compressing objects: 100% (4/4), done.
>   Writing objects: 100% (4/4), 742 bytes | 0 bytes/s, done.
>   Total 4 (delta 3), reused 0 (delta 0)
>   remote: *** Pattern "Conflicts:" has been detected.
>   remote: *** (in commit 7fa8e2a29c9357024d21bdf09faa5cb930a4ffe5)
>   remote: ***
>   remote: *** This usually indicates a merge commit where some merge conflicts
>   remote: *** had to be resolved, but where the "Conflicts:" section has not
>   remote: *** been deleted from the revision history.
>   remote: ***
>   remote: *** Please edit the commit's revision history to either delete
>   remote: *** the section, or to avoid using the pattern above by itself.
>   remote: error: hook declined to update refs/heads/gdb-7.9-branch
>   To git+ssh://sourceware.org/git/binutils-gdb.git
>    ! [remote rejected] gdb-7.9-branch -> gdb-7.9-branch (hook declined)
>   error: failed to push some refs to 'git+ssh://sourceware.org/git/binutils-gdb.git'
> 
> There  was no unresolved conflicts in the commit I tried to push.  Why
> does the hook insist on forcing me to remove the "Conflicts:" part,
> instead of looking for the merge-commit conflict markers?  Why do we
> care about leaving the reference to original conflicts in the log?

There is a commit whose revision log has a line that has "Conflict:"
in its revision log. This usually happens when you cherry-pick
a commit, resolve merge conflicts, and then commit again. Git
decides to append a section at the end off the new revision log
which lists the files where there were some conflicts.

In our experience at AdaCore, these are useless, and people often
don't even notice them. Hence the check.

-- 
Joel

Re: Commit hook annoyance

[Joel Brobecker]

> > There  was no unresolved conflicts in the commit I tried to push.  Why
> > does the hook insist on forcing me to remove the "Conflicts:" part,
> > instead of looking for the merge-commit conflict markers?  Why do we
> > care about leaving the reference to original conflicts in the log?

Sorry - re-reading your question, I am now realizing that I was not
really answering what you asked. So, let me try it again.

I think we do care, IMO, because the information is really useless.
And also, as I said, users often don't even realize that git does
that for them. I'd rather have a little hassle right now, in favor
of sanitized revision logs forever.

I don't think it's a big hassle to have to remove them, myself.
But if the consensus to remove the check, we can enhance the hooks
to make it optional.

-- 
Joel

Re: Commit hook annoyance

[Eli Zaretskii]

> Date: Fri, 16 Jan 2015 08:10:55 -0800
> From: "H.J. Lu" <hjl.tools@gmail.com>
> Cc: Joel Brobecker <brobecker@adacore.com>, GDB <gdb-patches@sourceware.org>
> 
> On Fri, Jan 16, 2015 at 8:00 AM, Eli Zaretskii <eliz@gnu.org> wrote:
> > Is this really necessary?
> >
> >   $ git push
> >   Counting objects: 94, done.
> >   Compressing objects: 100% (4/4), done.
> >   Writing objects: 100% (4/4), 742 bytes | 0 bytes/s, done.
> >   Total 4 (delta 3), reused 0 (delta 0)
> >   remote: *** Pattern "Conflicts:" has been detected.
> >   remote: *** (in commit 7fa8e2a29c9357024d21bdf09faa5cb930a4ffe5)
> >   remote: ***
> >   remote: *** This usually indicates a merge commit where some merge conflicts
> >   remote: *** had to be resolved, but where the "Conflicts:" section has not
> >   remote: *** been deleted from the revision history.
> >   remote: ***
> >   remote: *** Please edit the commit's revision history to either delete
> >   remote: *** the section, or to avoid using the pattern above by itself.
> >   remote: error: hook declined to update refs/heads/gdb-7.9-branch
> >   To git+ssh://sourceware.org/git/binutils-gdb.git
> >    ! [remote rejected] gdb-7.9-branch -> gdb-7.9-branch (hook declined)
> >   error: failed to push some refs to 'git+ssh://sourceware.org/git/binutils-gdb.git'
> >
> > There  was no unresolved conflicts in the commit I tried to push.  Why
> > does the hook insist on forcing me to remove the "Conflicts:" part,
> > instead of looking for the merge-commit conflict markers?  Why do we
> > care about leaving the reference to original conflicts in the log?
> 
> Do you have a commit which has a conflict and is fixed by another
> commit?

No, it was a cherry-pick that created a conflict, whih I resolved
before committing.  But I didn't bother to remove the "Conflicts:"
part from the automatic commit message that Git lets me edit, when I
invoke "git cherry-pick -e".

Re: Commit hook annoyance

[Eli Zaretskii]

> Date: Fri, 16 Jan 2015 20:18:41 +0400
> From: Joel Brobecker <brobecker@adacore.com>
> Cc: gdb-patches@sourceware.org
> 
> There is a commit whose revision log has a line that has "Conflict:"
> in its revision log. This usually happens when you cherry-pick
> a commit, resolve merge conflicts, and then commit again. Git
> decides to append a section at the end off the new revision log
> which lists the files where there were some conflicts.

Yes, that's what happened.  Quite normal.

> In our experience at AdaCore, these are useless, and people often
> don't even notice them. Hence the check.

Being useless doesn't mean harmful.  (I do notice that part, but I'm
used to leaving it there.)  Rejecting a push is an annoyance I'd
rather do without.  Moreover, that line is a record of the fact that
there was a conflict, which might be useful one day.

So how about removing that particular reason for rejecting a push?

Thanks.

Re: Commit hook annoyance

[Doug Evans]

On Fri, Jan 16, 2015 at 8:41 AM, Eli Zaretskii <eliz@gnu.org> wrote:
>> Date: Fri, 16 Jan 2015 20:18:41 +0400
>> From: Joel Brobecker <brobecker@adacore.com>
>> Cc: gdb-patches@sourceware.org
>>
>> There is a commit whose revision log has a line that has "Conflict:"
>> in its revision log. This usually happens when you cherry-pick
>> a commit, resolve merge conflicts, and then commit again. Git
>> decides to append a section at the end off the new revision log
>> which lists the files where there were some conflicts.
>
> Yes, that's what happened.  Quite normal.
>
>> In our experience at AdaCore, these are useless, and people often
>> don't even notice them. Hence the check.
>
> Being useless doesn't mean harmful.  (I do notice that part, but I'm
> used to leaving it there.)  Rejecting a push is an annoyance I'd
> rather do without.  Moreover, that line is a record of the fact that
> there was a conflict, which might be useful one day.
>
> So how about removing that particular reason for rejecting a push?

I dunno, let's not rush into a change.

Having caught up on the thread, at the moment I'm kinda liking the check.