[PATCH] contrib/nmbug: use "resolve" merge strategy

[PATCH] contrib/nmbug: use "resolve" merge strategy

[david]

From: David Bremner <bremner@debian.org>

The recursive merge strategy does rename detection, which yields false
positives (and hence spurious merge conflicts) when merging trees of
empty files.
---
 An unresolved issue (ho ho) is the fact that failed merge operations
 are still not detected. This needs more thought, but I thought this 
 patch might save people some pain in the meantime. It isn't very heavily 
 tested, though.

 contrib/nmbug |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/contrib/nmbug b/contrib/nmbug
index bb0739f..0ed3c29 100755
--- a/contrib/nmbug
+++ b/contrib/nmbug
@@ -302,7 +302,7 @@ sub do_merge {
 
   git ( { GIT_WORK_TREE => $tempwork }, 'checkout', '-f', 'HEAD');
 
-  git ( { GIT_WORK_TREE => $tempwork }, 'merge', 'FETCH_HEAD');
+  git ( { GIT_WORK_TREE => $tempwork }, 'merge', '-s', 'resolve', 'FETCH_HEAD');
 
   do_checkout ();
 }
-- 
1.7.9.1

[PATCH] nmbug: check whether every forked process exit with (non)zero value

[Tomi Ollila]

If any of the forked process exits with nonzero value, terminate
current operation -- nonzero exit value indicates failure and
then there is no point continuing.
---

 I tested this by using most of the commands (pushed few tags in 
 addtion to pull, log, status & checkout). I haven't encountered
 any failures yet in real environment but mangling nmbug to fail
 does make it fail as expected (s/git/false/ or s/exec @_/exit 1/
 -- exit ! close $fh could not be tested as there is currently
 no code paths to arrive there).

 contrib/nmbug |   37 ++++++++++++++++++++++++++++++++-----
 1 files changed, 32 insertions(+), 5 deletions(-)

diff --git a/contrib/nmbug b/contrib/nmbug
index bb0739f..f003ef9 100755
--- a/contrib/nmbug
+++ b/contrib/nmbug
@@ -60,6 +60,9 @@ sub git_pipe {
 sub git {
   my $fh = git_pipe (@_);
   my $str = join ('', <$fh>);
+  unless (close $fh) {
+    die "'git @_' exited with nonzero value\n";
+  }
   chomp($str);
   return $str;
 }
@@ -84,7 +87,7 @@ sub spawn {
       foreach my $line (@{$ioref}) {
 	print $fh $line, "\n";
       }
-      exit 0;
+      exit ! close $fh;
     } else {
       if ($dir ne '|-') {
 	open STDIN, '<', '/dev/null' or die "reopening stdin: $!"
@@ -106,6 +109,9 @@ sub get_tags {
     chomp ();
     push @tags, $_ if (m/^$prefix/);
   }
+  unless (close $fh) {
+    die "'notmuch search --output=tags *' exited with nonzero value\n";
+  }
   return @tags;
 }
 
@@ -173,6 +179,10 @@ sub update_index {
   foreach my $pair (@{$status->{added}}) {
     index_tags_for_msg ($git, $pair->{id}, 'A', $pair->{tag});
   }
+  unless (close $git) {
+    die "'git update-index --index-info' exited with nonzero value\n";
+  }
+
 }
 
 
@@ -211,8 +221,12 @@ sub index_tags {
     my @tags = grep { s/^$TAGPREFIX//; } split (' ', $rest);
     index_tags_for_msg ($git,$id, 'A', @tags);
   }
-
-  close $git;
+  unless (close $git) {
+    die "'git update-index --index-info' exited with nonzero value\n";
+  }
+  unless (close $fh) {
+    die "'notmuch dump -- $query' exited with nonzero value\n";
+  }
   return $index;
 }
 
@@ -395,6 +409,9 @@ sub compute_status {
     } else {
       push @deleted, $pair;
     }
+    unless (close $fh) {
+      die "'notmuch search --output=files id:$id' exited with nonzero value\n";
+    }
   }
 
 
@@ -414,7 +431,12 @@ sub diff_index {
 		  qw/diff-index --cached/,
 		 "--diff-filter=$filter", qw/--name-only HEAD/ );
 
-  return unpack_diff_lines ($fh);
+  my @lines = unpack_diff_lines ($fh);
+  unless (close $fh) {
+    die "'git diff-index --cached --diff-filter=$filter --name-only HEAD' ",
+	"exited with nonzero value\n";
+  }
+  return @lines;
 }
 
 
@@ -426,7 +448,12 @@ sub diff_refs {
   my $fh= git_pipe ( 'diff', "--diff-filter=$filter", '--name-only',
 		 $ref1, $ref2);
 
-  return unpack_diff_lines ($fh);
+  my @lines = unpack_diff_lines ($fh);
+  unless (close $fh) {
+    die "'git diff --diff-filter=$filter --name-only $ref1 $ref2' ",
+	"exited with nonzero value\n";
+  }
+  return @lines;
 }
 
 
-- 
1.7.8.2

Re: [PATCH] contrib/nmbug: use "resolve" merge strategy

[Tomi Ollila]

On Sat, Mar 31 2012, david@tethera.net wrote:

> From: David Bremner <bremner@debian.org>
>
> The recursive merge strategy does rename detection, which yields false
> positives (and hence spurious merge conflicts) when merging trees of
> empty files.

I attempted to search more info about 'recursive' vs 'resolve' merge
but without much luck. Nothing better comes out than MERGE STRATEGIES
section in git-merge namual page.

IMHO text favoring resolve: "It tries to carefully detect criss-cross
merge ambiguities and is considered generally safe and fast."

& text unfavoring recursive (in this particular case): "Additionally
this can detect and handle merges involving renames."

so LGTM.

Tomi

> ---
>  An unresolved issue (ho ho) is the fact that failed merge operations
>  are still not detected. This needs more thought, but I thought this 
>  patch might save people some pain in the meantime. It isn't very heavily 
>  tested, though.
>
>  contrib/nmbug |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/contrib/nmbug b/contrib/nmbug
> index bb0739f..0ed3c29 100755
> --- a/contrib/nmbug
> +++ b/contrib/nmbug
> @@ -302,7 +302,7 @@ sub do_merge {
>  
>    git ( { GIT_WORK_TREE => $tempwork }, 'checkout', '-f', 'HEAD');
>  
> -  git ( { GIT_WORK_TREE => $tempwork }, 'merge', 'FETCH_HEAD');
> +  git ( { GIT_WORK_TREE => $tempwork }, 'merge', '-s', 'resolve', 'FETCH_HEAD');
>  
>    do_checkout ();
>  }
> -- 
> 1.7.9.1
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH] nmbug: check whether every forked process exit with (non)zero value

[David Bremner]

Tomi Ollila <tomi.ollila@iki.fi> writes:

> If any of the forked process exits with nonzero value, terminate
> current operation -- nonzero exit value indicates failure and
> then there is no point continuing.

Pushed.

Ironically I'm still not too sure about the merge strategy thing, I hope
I have some time to test it more, but at the moment I forget how to
reliable get a conflict.

d