Possible bug in Git

3 messages, 3 authors, 2016-06-15 · open the first message on its own page

Possible bug in Git

From: David Martínez Martí <hidden>
Date: 2016-06-15 22:48:37

(Please add deavidsedice@gmail.com and desarrollo@gestiweb.com to CC 
because I'm not suscripted to this list)

Here I've a strange behaviour in GIT:

We noticed a file in our repo is empty: 
facturacion/facturacion/scripts/lineasfacturascli.qs

We didn't found the reason using gitk --all -- 
facturacion/facturacion/scripts/lineasfacturascli.qs

With git bisect we found the problematic commit: 
dac6a95f559604978ff9e1fac24ad752c54382ae

With git show, it doesn't show anything about the modification:

$ git show dac6a95f5 -p --pretty=raw -- 
facturacion/facturacion/scripts/lineasfacturascli.qs
commit dac6a95f559604978ff9e1fac24ad752c54382ae
tree f1472f1d289f9813df5fd691c9cea6ccb04313a7
parent 5dfdd78f24f4578cdfeed876b8bacb45790af9ef
parent 6ed54c8c6508b6d5551290b45785eaf44dd05d75
author David Martinez Marti [off-list ref] 1260959309 +0100
committer David Martinez Marti [off-list ref] 1260959309 +0100

     Merge remote branch 'origin/infosial'

     Conflicts:
         contabilidad/informes/scripts/flcontinfo.qs
         facturacion/almacen/forms/articulos.ui
         facturacion/almacen/scripts/articulos.qs
         facturacion/almacen/tables/factalma_general.mtd
         facturacion/facturacion/forms/albaranescli.ui
         facturacion/facturacion/forms/albaranesprov.ui
         facturacion/facturacion/forms/facturasprov.ui
         facturacion/facturacion/forms/pedidoscli.ui
         facturacion/facturacion/forms/pedidosprov.ui
         facturacion/facturacion/forms/presupuestoscli.ui
         facturacion/facturacion/scripts/facturascli.qs
         facturacion/facturacion/scripts/facturasprov.qs
         facturacion/facturacion/scripts/flfacturac.qs
         facturacion/facturacion/scripts/lineasalbaranescli.qs
         facturacion/facturacion/scripts/lineasfacturascli.qs
         facturacion/facturacion/scripts/lineaspresupuestoscli.qs
         facturacion/informes/flfactinfo.xml
         facturacion/informes/forms/i_reciboscli.ui
         facturacion/informes/forms/i_recibosprov.ui
         facturacion/informes/queries/i_reciboscli.qry
         facturacion/informes/queries/i_recibosprov.qry
         facturacion/informes/queries/i_resreciboscli.qry
         facturacion/informes/queries/i_resrecibosprov.qry
         facturacion/informes/scripts/flfactinfo.qs
         facturacion/informes/scripts/i_masterinventario.qs
         facturacion/principal/flfactppal.xml
         facturacion/principal/forms/agentes.ui
         facturacion/principal/forms/clientes.ui
         facturacion/principal/forms/empresa.ui
         facturacion/principal/forms/flfactppal.ui
         facturacion/principal/forms/proveedores.ui
         facturacion/principal/scripts/clientes.qs
         facturacion/principal/scripts/proveedores.qs
         facturacion/principal/tables/clientes.mtd
         facturacion/principal/tables/proveedores.mtd
         facturacion/tesoreria/tables/reciboscli.mtd




There was a conflict on that file but it doesn't show anything changed 
about it. Gitk does the same.

git show commit^1:path and git show commit^2:path shows the files:

$ git show 
dac6a95f5^1:facturacion/facturacion/scripts/lineasfacturascli.qs | head
/***************************************************************************
                  lineasfacturascli.qs  -  description
                              -------------------
     begin                : lun abr 26 2004
     copyright            : (C) 2004 by InfoSiAL S.L.
     email                : mail@infosial.com
  ***************************************************************************/
/***************************************************************************
  *                                                                         *
  *   This program is free software; you can redistribute it and/or 
modify  *


$ git show 
dac6a95f5^2:facturacion/facturacion/scripts/lineasfacturascli.qs | head
/***************************************************************************
                  lineasfacturascli.qs  -  description
                              -------------------
     begin                : lun abr 26 2004
     copyright            : (C) 2004 by InfoSiAL S.L.
     email                : mail@infosial.com
  ***************************************************************************/
/***************************************************************************
  *                                                                         *
  *   This program is free software; you can redistribute it and/or 
modify  *


git show commit:path shows empty file:
$ git show 
dac6a95f5:facturacion/facturacion/scripts/lineasfacturascli.qs | wc
       0       0       0



I believe that changes *should* appear on git log -p and on gitk, and 
they doesn't, so I think it may be a bug in some git tools.

Tell me if I can do anything to fdetermine if it's really a bug and 
where it is to help GIT devs resovling it.

PD: I know my merge failed somehow, and I corrected it in a newer commit.

[PATCH] combined diff: correctly handle truncated file

From: Thomas Rast <hidden>
Date: 2016-06-15 22:48:37

Consider an evil merge of two commits A and B, both of which have a
file 'foo', but the merge result does not have that file.

The combined-diff code learned in 4462731 (combine-diff: do not punt
on removed or added files., 2006-02-06) to concisely show only the
removal, since that is the evil part and the previous contents are
presumably uninteresting.

However, to diagnose an empty merge result, it overloaded the variable
that holds the file's length.  This means that the check also triggers
for truncated files.  Consequently, such files were not shown in the
diff at all despite the merge being clearly evil.

Fix this by adding a new variable that distinguishes whether the file
was deleted (which is the case 4462731 handled) or truncated.  In the
truncated case, we show the full combined diff again, which is rather
spammy but at least does not hide the evilness.

Reported-by: David Martínez Martí <redacted>
Signed-off-by: Thomas Rast <redacted>
---
 combine-diff.c           |   14 ++++++++------
 t/t4038-diff-combined.sh |    8 ++++++++
 2 files changed, 16 insertions(+), 6 deletions(-)
diff --git a/combine-diff.c b/combine-diff.c
index 6162691..3480dae 100644
--- a/combine-diff.c
+++ b/combine-diff.c
@@ -204,7 +204,7 @@ static void consume_line(void *state_, char *line, unsigned long len)
 static void combine_diff(const unsigned char *parent, unsigned int mode,
 			 mmfile_t *result_file,
 			 struct sline *sline, unsigned int cnt, int n,
-			 int num_parent)
+			 int num_parent, int result_deleted)
 {
 	unsigned int p_lno, lno;
 	unsigned long nmask = (1UL << n);
@@ -215,7 +215,7 @@ static void combine_diff(const unsigned char *parent, unsigned int mode,
 	struct combine_diff_state state;
 	unsigned long sz;
 
-	if (!cnt)
+	if (result_deleted)
 		return; /* result deleted */
 
 	parent_file.ptr = grab_blob(parent, mode, &sz);
@@ -517,7 +517,7 @@ static void show_line_to_eol(const char *line, int len, const char *reset)
 }
 
 static void dump_sline(struct sline *sline, unsigned long cnt, int num_parent,
-		       int use_color)
+		       int use_color, int result_deleted)
 {
 	unsigned long mark = (1UL<<num_parent);
 	unsigned long no_pre_delete = (2UL<<num_parent);
@@ -530,7 +530,7 @@ static void dump_sline(struct sline *sline, unsigned long cnt, int num_parent,
 	const char *c_plain = diff_get_color(use_color, DIFF_PLAIN);
 	const char *c_reset = diff_get_color(use_color, DIFF_RESET);
 
-	if (!cnt)
+	if (result_deleted)
 		return; /* result deleted */
 
 	while (1) {
@@ -687,6 +687,7 @@ static void show_patch_diff(struct combine_diff_path *elem, int num_parent,
 {
 	struct diff_options *opt = &rev->diffopt;
 	unsigned long result_size, cnt, lno;
+	int result_deleted = 0;
 	char *result, *cp;
 	struct sline *sline; /* survived lines */
 	int mode_differs = 0;
@@ -767,6 +768,7 @@ static void show_patch_diff(struct combine_diff_path *elem, int num_parent,
 		}
 		else {
 		deleted_file:
+			result_deleted = 1;
 			result_size = 0;
 			elem->mode = 0;
 			result = xcalloc(1, 1);
@@ -823,7 +825,7 @@ static void show_patch_diff(struct combine_diff_path *elem, int num_parent,
 			combine_diff(elem->parent[i].sha1,
 				     elem->parent[i].mode,
 				     &result_file, sline,
-				     cnt, i, num_parent);
+				     cnt, i, num_parent, result_deleted);
 		if (elem->parent[i].mode != elem->mode)
 			mode_differs = 1;
 	}
@@ -889,7 +891,7 @@ static void show_patch_diff(struct combine_diff_path *elem, int num_parent,
 			dump_quoted_path("+++ ", b_prefix, elem->path,
 					 c_meta, c_reset);
 		dump_sline(sline, cnt, num_parent,
-			   DIFF_OPT_TST(opt, COLOR_DIFF));
+			   DIFF_OPT_TST(opt, COLOR_DIFF), result_deleted);
 	}
 	free(result);
 
diff --git a/t/t4038-diff-combined.sh b/t/t4038-diff-combined.sh
index 7584efa..09e8c8a 100755
--- a/t/t4038-diff-combined.sh
+++ b/t/t4038-diff-combined.sh
@@ -81,4 +81,12 @@ test_expect_success 'check combined output (2)' '
 	verify_helper sidesansone
 '
 
+test_expect_success 'diagnose truncated file' '
+	: > file &&
+	git add file &&
+	git commit --amend -C HEAD &&
+	git show > out &&
+	grep "diff --cc file" out
+'
+
 test_done
-- 
1.7.1.rc1.265.g77471

Re: Possible bug in Git

From: Avery Pennarun <hidden>
Date: 2016-06-15 22:48:39

On Thu, Apr 15, 2010 at 6:47 AM, David Martínez Martí
[off-list ref] wrote:
$ git show dac6a95f5 -p --pretty=raw --
facturacion/facturacion/scripts/lineasfacturascli.qs
commit dac6a95f559604978ff9e1fac24ad752c54382ae
tree f1472f1d289f9813df5fd691c9cea6ccb04313a7
parent 5dfdd78f24f4578cdfeed876b8bacb45790af9ef
parent 6ed54c8c6508b6d5551290b45785eaf44dd05d75
author David Martinez Marti [off-list ref] 1260959309 +0100
committer David Martinez Marti [off-list ref] 1260959309 +0100

   Merge remote branch 'origin/infosial'

   Conflicts:
      [...]
   [empty diff]

There was a conflict on that file but it doesn't show anything changed about
it. Gitk does the same.
You're trying to show the log of a merge commit.  git tries to be
clever about showing diffs on such commits, but it screws up rather
too often to be reliable, and this bites a lot of people (including
myself).

The core of the problem is this: if you're looking at 'git log -p',
you'll see all the individual commits.  But a merge commit is
essentially a combination of all the commits on branch #2 as you add
them to branch #1.  (Or alternatively: all the commits on branch #1 as
you add them to branch #2.)  So if git did the obvious thing and just
showed the *entire* diff that the merge applies to branch #1, it would
be a big long patch, but worse, it would be *equivalent* to the
patches you've already seen from other individual commits.

So what git tries to do is reduce the diff output of a merge commit
down to just the "unexpected" parts: the parts that are different from
what you'd get if you just merged in the individual changes in the
obvious way.  Unfortunately, actually calculating that would be really
complicated; it would require redoing the whole merge just to find out
what happened, and that turns out to be too slow.  So git uses
heuristics to try to guess what happened, and those heuristics don't
always work out.  In your case, it seems to have decided that "nothing
special happened" during the merge even though apparently the file was
entirely blanked out, which seems obviously wrong to me, but I don't
actually know how the heuristics work.

The "git log" options -c and --cc are documented (in 'man git-log') as
supposedly affecting this behaviour.  However, it seems that "--cc"
became the default somehow, and there's no obvious way to turn it back
to the "supposedly default" plain or -c behaviour.  Maybe someone else
on the list can comment more.

Have fun,

Avery
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help