Improving auto conflict resolving while merge

Improving auto conflict resolving while merge

[KES]

Hi.

**To not apply big patches, I will supply links to public repository

Lets start

1. git clone https://github.com/KES777/Plack

2. git show ed485bf75a6

Nothing interesting. Usual merge conflict while merge branch 'feature/doc_contribute' to 'master'

But I was interested by this part:

   my $app = sub {
       my $env =  shift;
 
+<<<<<<< HEAD
+=======
+  my $app = sub {
+      my $env =  shift;
+
+>>>>>>> master

I know that some code were cherry picked(??) by maintainer from by branch. Ans this is the part which are same in master and branch.
So I not expect conflict here. I start to dig.

I compare Log4perl.pm files from branch and master 
3. git checkout feature/doc_contribution
4. cd lib/Plack/Middleware
5. git diff d095870 Log4perl.pm

I see that both master and branch has same resulting changes:
@@ -36,10 +35,10 @@ __END__
 
 =head1 NAME
 
-Plack::Middleware::Log4perl - Uses Log::Log4perl to configure logger
+Plack::Middleware::Log4perl - Uses Log::Log4perl to configure psgix.logger
 
 =head1 SYNOPSIS
-
+  # How to use logger in your app
   my $app = sub {
       my $env =  shift;
 
@@ -53,18 +52,25 @@ Plack::Middleware::Log4perl - Uses Log::Log4perl to configure logger

But this part of code were edited in both branches too:

The branch were started at this commit: 0a5ff8427

6. git diff 0a5ff8427

@@ -35,14 +35,28 @@ __END__
 
 =head1 NAME
 
-Plack::Middleware::Log4perl - Uses Log::Log4perl to configure logger
+Plack::Middleware::Log4perl - Uses Log::Log4perl to configure psgix.logger
 
 =head1 SYNOPSIS
+  # How to use logger in your app
+  my $app = sub {
+      my $env =  shift;
 
-  use Log::Log4perl;
+      $env->{'psgix.logger'}({ level => 'error', message => 'Hi' });
+
+      return [
+          '200',
+          [ 'Content-Type' => 'text/plain' ],
+          [ "Hello World" ],
+      ];
+  };

7. git checkout master 
8. git diff 0a5ff8427

@@ -39,28 +40,41 @@ Plack::Middleware::Log4perl - Uses Log::Log4perl to configure logger
 
 =head1 SYNOPSIS
 
-  use Log::Log4perl;
+  my $app = sub {
+      my $env =  shift;
+
+      $env->{'psgix.logger'}({ level => 'error', message => 'Hi' });
+
+      return [
+          '200',
+          [ 'Content-Type' => 'text/plain' ],
+          [ "Hello World" ],
+      ];
+  };
+

I repeat for emphasis. In the resulting file we have same code and not expect conflict.

At this point I have same changes on master and branch.
master head is d095870
branch head is a3db36a
fork point is 0a5ff84

Comparing step-by-step 
(A)   git diff 0a5ff84..d095870
(B)   git diff 0a5ff84..a3db36a
Find patches which apply to same lines
 =head1 SYNOPSIS
 
-  use Log::Log4perl;
+  my $app = sub {
+      my $env =  shift;
+
+      $env->{'psgix.logger'}({ level => 'error', message => 'Hi' });
+
+      return [
+          '200',
+          [ 'Content-Type' => 'text/plain' ],
+          [ "Hello World" ],
+      ];
+  };
+

 =head1 SYNOPSIS
+  # How to use logger in your app
+  my $app = sub {
+      my $env =  shift;
 
-  use Log::Log4perl;
+      $env->{'psgix.logger'}({ level => 'error', message => 'Hi' });
+
+      return [
+          '200',
+          [ 'Content-Type' => 'text/plain' ],
+          [ "Hello World" ],
+      ];
+  };

And check with 
(C)  git diff a3db36a..d095870
 =head1 SYNOPSIS
-  # How to use logger in your app
+
   my $app = sub {
       my $env =  shift;
 
(D) So this merge conflict (the result of `git checkout master; git merge feature/doc_contribution`)
@@@ -36,13 -35,13 +36,19 @@@ __END_
  
  =head1 NAME
  
- Plack::Middleware::Log4perl - Uses Log::Log4perl to configure logger
+ Plack::Middleware::Log4perl - Uses Log::Log4perl to configure psgix.logger
  
  =head1 SYNOPSIS
+   # How to use logger in your app
+   my $app = sub {
+       my $env =  shift;
  
++<<<<<<< HEAD
 +  my $app = sub {
 +      my $env =  shift;
 +
++=======
++>>>>>>> feature/doc_contribution
        $env->{'psgix.logger'}({ level => 'error', message => 'Hi' });
  
        return [

can be simplified to:

@@@ -36,13 -35,13 +36,19 @@@ __END_
  
  =head1 NAME
  
- Plack::Middleware::Log4perl - Uses Log::Log4perl to configure logger
+ Plack::Middleware::Log4perl - Uses Log::Log4perl to configure psgix.logger
  
  =head1 SYNOPSIS
+   # How to use logger in your app
    my $app = sub {
        my $env =  shift;
  

NEXT
Next merge conflict seems this:
(A)
+  };
+
 
+  # Use your own Log4perl configuration
+  use Log::Log4perl;
   Log::Log4perl::init('/path/to/log4perl.conf');

(B)
+  };
 
+
+  # Initialization. Case#1
+  use Log::Log4perl;
   Log::Log4perl::init('/path/to/log4perl.conf');

Comparing to (C)
@@ -53,18 +52,25 @@
   };
 
 
-  # Use your own Log4perl configuration
+  # Initialization. Case#1
   use Log::Log4perl;
   Log::Log4perl::init('/path/to/log4perl.conf');

So current merge conflict (D) 
@@@ -51,20 -50,27 +57,33 @@@
            [ "Hello World" ],
        ];
    };
++<<<<<<< HEAD
 +
 +
 +  # Use your own Log4perl configuration
++=======
+ 
+ 
+   # Initialization. Case#1
++>>>>>>> feature/doc_contribution
    use Log::Log4perl;
    Log::Log4perl::init('/path/to/log4perl.conf');

Can be simplified to: (E) 
@@@ -51,20 -50,27 +57,33 @@@
            [ "Hello World" ],
        ];
    };


++<<<<<<< HEAD
 +  # Use your own Log4perl configuration
++=======
+   # Initialization. Case#1
++>>>>>>> feature/doc_contribution
    use Log::Log4perl;
    Log::Log4perl::init('/path/to/log4perl.conf');

NOTICE: in (C) only two lines are different.

NEXT
(A)
-  # Or let middleware to configure log4perl
-  enable "Log4perl", category => "plack", conf => '/path/to/log.conf';
+  # Configure with Log4perl middleware options
+  builder {
+      enable "Log4perl", category => "plack", conf => '/path/to/log4perl.conf';
+      $app;
+  }

(B)
-  # Or let middleware to configure log4perl
-  enable "Log4perl", category => "plack", conf => '/path/to/log.conf';
+
+  # Initialization. Case#2
+  # Let middleware to configure log4perl
+  builder {
+      enable "Log4perl", category => "plack", conf => '/path/to/log4perl.conf';
+      $app;
+  }

(C)


-  # Configure with Log4perl middleware options
+  # Initialization. Case#2
+  # Let middleware to configure log4perl
   builder {
       enable "Log4perl", category => "plack", conf => '/path/to/log4perl.conf';
       $app;


(D)
  
++<<<<<<< HEAD
 +  # Configure with Log4perl middleware options
++=======
+ 
+   # Initialization. Case#2
+   # Let middleware to configure log4perl
++>>>>>>> feature/doc_contribution
    builder {
        enable "Log4perl", category => "plack", conf => '/path/to/log4perl.conf';
        $app;

->(E) Notice, that in (C) one line is deleted and two added
++<<<<<<< HEAD
 +  # Configure with Log4perl middleware options
++=======
+   # Initialization. Case#2
+   # Let middleware to configure log4perl
++>>>>>>> feature/doc_contribution
    builder {
        enable "Log4perl", category => "plack", conf => '/path/to/log4perl.conf';
        $app;


NEXT
(A)
 =head1 DESCRIPTION
 
 Log4perl is a L<Plack::Middleware> component that allows you to use
-L<Log::Log4perl> to configure the logging object, C<psgix.logger>.
+L<Log::Log4perl> to configure the logging object C<psgix.logger> for a
+given category.
 
 =head1 CONFIGURATION

(B)
 =head1 DESCRIPTION
 
-Log4perl is a L<Plack::Middleware> component that allows you to use
-L<Log::Log4perl> to configure the logging object, C<psgix.logger>.
+Log4perl is a L<Plack::Middleware> component that initialize the logging object
+C<psgix.logger> by L<Log::Log4perl> logger with giving C<category>.
 
 =head1 CONFIGURATION

(C)
@@ -72,9 +78,8 @@
 
 =head1 DESCRIPTION
 
-Log4perl is a L<Plack::Middleware> component that allows you to use
-L<Log::Log4perl> to configure the logging object C<psgix.logger> for a
-given category.
+Log4perl is a L<Plack::Middleware> component that initialize the logging object
+C<psgix.logger> by L<Log::Log4perl> logger with giving C<category>.
 
 =head1 CONFIGURATION

(D)
@@@ -72,9 -78,8 +91,14 @@@
  
  =head1 DESCRIPTION
  
++<<<<<<< HEAD
 +Log4perl is a L<Plack::Middleware> component that allows you to use
 +L<Log::Log4perl> to configure the logging object C<psgix.logger> for a
 +given category.
++=======
+ Log4perl is a L<Plack::Middleware> component that initialize the logging object
+ C<psgix.logger> by L<Log::Log4perl> logger with giving C<category>.
++>>>>>>> feature/doc_contribution
  
  =head1 CONFIGURATION
  

In this example (E) is same as (D)



Feel free to ask how steps are done if you are not clear with mine explanation.

Re: Improving auto conflict resolving while merge

[Jeff King]

On Mon, Sep 07, 2015 at 08:15:46PM +0300, KES wrote:

> 1. git clone https://github.com/KES777/Plack
> [...]
> At this point I have same changes on master and branch.
> master head is d095870
> branch head is a3db36a
> fork point is 0a5ff84

I tried re-merging d095870 and a3db36a. I think what is happening is
that even though you have many similar hunks near each, there are still
some differences in the area, and they are close enough that the merge
isn't sure what is right. We err on the side of conservatism with
conflicts, because it's hard to say what is dependent and what is not.

For example, if you have the content:

  foo();
  bar();
  baz();

and one side makes it:

  foo();
  x = 1;
  bar();
  baz();

and the other side does:

  foo();
  bar();
  y = 2;
  baz();

you _could_ argue that those changes are independent (and write a merge
algorithm that silently merges them). They're touching two different
lines, and doing two different things. But it's close enough that
there's a good chance the two need to be reconciled, and a human should
at least take a look.

I think what further confuses things in your case is that the content
added by the two sides contains a lot of similar lines (because of the
cherry-pick), and the default "merge" conflict-markers try to shrink the
size of the conflicts.

Try running:

  git config merge.conflictstyle diff3

and re-doing the merge. It will give you a much better sense of how git
is breaking down the hunks (because it does not try to shrink the
conflicts, and because it shows the base content for each conflict).

-Peff

Re: Improving auto conflict resolving while merge

[Jeff King]

On Tue, Sep 08, 2015 at 09:08:09PM -0300, Eugen Konkov wrote:

> JK> you _could_ argue that those changes are independent But it's close 
> JK> enough that there's a good chance the two need to be reconciled, 
> JK> and a human should at least take a look.
> 
> You are right and your words make sense. But this thought may apply
> for this: We have one method/function about 200 lines. One author make
> change at line 1 of this method and other on 199 line. Both changes
> are done in one method so **human should at least take a look**

Right, there is definitely a concept of "close enough" here, and git
cannot catch everything. Semantic changes may even happen across files
(e.g., function interface changes). So you do need to rely on things
like testing and compilation to verify a merge result.

But I would agree there is room for being able to tune the "closeness"
of changes that cause a conflict. Right now that isn't implemented, and
I'm not familiar enough with the xdiff merge code to even point you in
the right direction.

-Peff

Re[2]: Improving auto conflict resolving while merge

[Eugen Konkov]

Hi, Jeff.

JK> For example, if you have the content:

JK>   foo();
JK>   bar();
JK>   baz();

JK> and one side makes it:

JK>   foo();
JK>   x = 1;
JK>   bar();
JK>   baz();

JK> and the other side does:

JK>   foo();
JK>   bar();
JK>   y = 2;
JK>   baz();

JK> you _could_ argue that those changes are independent But it's close 
JK> enough that there's a good chance the two need to be reconciled, 
JK> and a human should at least take a look.

You are right and your words make sense. But this thought may apply for this: We have one method/function about 200 lines. One author make change at line 1 of this method and other on 199 line. Both changes are done in one method so **human should at least take a look**

Example 2:
one author make change in method 1
second author make change in method 2
Method 1 is called from method 2. Those changes work fine on its own branch and does not work together. **human should at least take a look**

This task is not for human and must be left for test system. So your example and two mine must not rise conflicts. This make useless noise. 

Another example of useless merge conflicts

A--C--C'--F--?     master
 \  \       /
  B--C--D--G       feature


Here I start branch feature.
While developing update my branch from master by (C) changes
While developing feature (C) were changed by (C') on master
When I merge feature branch back to master I will get merge conflicts on lines in patch (C). I am not author of (C) and I do not care about it. Even  more I can not care and must not.

Implemented option (like --theirs in last example) to not be so conservative and do not rise merge conflicts for enough close changes will be good.