{"thread":{"id":"16689","subject":"git log --numstat disagrees with git apply --numstat","startedAt":"2008-12-11T23:53:37Z","lastAt":"2008-12-15T20:32:26Z","messageCount":6,"participants":["Shawn O. Pearce","Jeff King","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"97643","messageId":"20081211235337.GK32487@spearce.org","threadId":"16689","inReplyTo":null,"subject":"git log --numstat disagrees with git apply --numstat","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2008-12-11T23:53:37Z","receivedAt":"2008-12-11T23:53:37Z","isPatch":false,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"I've found a case where git apply --numstat and git log --numstat\nproduce different results for the same commit.\n\nIn egit (git://repo.or.cz/egit.git/):\n\n  $ git log --numstat -1 --pretty=o 9bda5ece6806cd797416eaa47c\n  9bda5ece6806cd797416eaa47c7b927cc6e9c3b2 Teach RevWalk about ...\n  8       0       org.spearce.jgit/src/org/spearce/jgit/revwalk/DateRevQueue.java\n  69      13      org.spearce.jgit/src/org/spearce/jgit/revwalk/RevWalk.java\n\n  $ git log -p -1 --pretty=o 9bda5ece6806cd797416eaa47c | git apply --numstat\n  8       0       org.spearce.jgit/src/org/spearce/jgit/revwalk/DateRevQueue.java\n  68      12      org.spearce.jgit/src/org/spearce/jgit/revwalk/RevWalk.java\n\nI found this because I was writing a unit test for JGit that ran\nthrough the JGit project history and compared the output of git\nlog --numstat against the output of JGit's \"git apply --numstat\"\nimplementation, after scraping the \"git log -p\" output.\n\nI can't quite figure out why log --numstat is coming up with a +1\ndifference here for both added and removed, but it is.  I haven't\ndug into the Git code yet to figure out why.  FWIW, JGit produces\nthe same result as \"git apply --numstat\" (the 68/12).\n\nAt this point JGit was able to successfully read and match 715 of\n1211 commits before it found this difference, so its also somewhat\nrare to occur I think...\n\n-- \nShawn.\n"},{"id":"97654","messageId":"20081212015254.GA23128@sigill.intra.peff.net","threadId":"16689","inReplyTo":"20081211235337.GK32487@spearce.org","subject":"Re: git log --numstat disagrees with git apply --numstat","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-12-12T01:52:55Z","receivedAt":"2008-12-12T01:52:55Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Dec 11, 2008 at 03:53:37PM -0800, Shawn O. Pearce wrote:\n\n> I've found a case where git apply --numstat and git log --numstat\n> produce different results for the same commit.\n\n\"git apply\" doesn't actually do the diff; it just calculates based on\nwhat it sees in the patch. So the real issue is \"git log -p\" and \"git\nlog --numstat\" produce different patches. And you can see it by\ninstrumenting like this:\n\ndiff --git a/diff.c b/diff.c\nindex af822c1..fce93db 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -726,6 +726,8 @@ static void diffstat_consume(void *priv, char *line, unsigned long len)\n \tstruct diffstat_t *diffstat = priv;\n \tstruct diffstat_file *x = diffstat->files[diffstat->nr - 1];\n \n+\tfwrite(line, 1, len, stderr);\n+\n \tif (line[0] == '+')\n \t\tx->added++;\n \telse if (line[0] == '-')\n\nand then comparing what diffstat_consume gets versus the patch,\nsomething like:\n\n    what=\"9bda5ece org.spearce.jgit/src/org/spearce/jgit/revwalk/RevWalk.java\"\n    git log -p -1 --pretty=format: $what | grep '^[-+@]' >a\n    git log --numstat -1 --pretty=format: $what 2>b >/dev/null\n    diff -u a b\n\nIt looks like it is just a place where two different valid diffs can be\nconstructed:\n\n    +-\n    +-              for (final RevCommit p : c.parents) {\n    +-                      if ((p.flags & SEEN) != 0)\n     +              for (;;) {\n     +                      final RevCommit c = pending.pop();\n     +                      if (c == null)\n    @@ -67,19 +68,20 @@\n     +                              p.flags |= SEEN;\n     +                              pending.add(p);\n     +                      }\n    --              for (final RevCommit p : c.parents) {\n    --                      if ((p.flags & SEEN) != 0)\n    ++\n\nwhich is probably just due to different xdi settings being used between\nthe two codepaths. I haven't looked closely to see which different\noptions we are feeding to xdiff.\n\n-Peff\n"},{"id":"97656","messageId":"20081212020857.GB23128@sigill.intra.peff.net","threadId":"16689","inReplyTo":"20081212015254.GA23128@sigill.intra.peff.net","subject":"Re: git log --numstat disagrees with git apply --numstat","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-12-12T02:08:58Z","receivedAt":"2008-12-12T02:08:58Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Dec 11, 2008 at 08:52:55PM -0500, Jeff King wrote:\n\n> It looks like it is just a place where two different valid diffs can be\n> constructed:\n> [...]\n> which is probably just due to different xdi settings being used between\n> the two codepaths. I haven't looked closely to see which different\n> options we are feeding to xdiff.\n\nAh. Doing this gives me the 68/12 answer for \"git log --numstat\":\n\ndiff --git a/diff.c b/diff.c\nindex af822c1..5f314ce 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -1539,6 +1539,7 @@ static void builtin_diffstat(const char *name_a, const char *name_b,\n \n \t\tmemset(&xpp, 0, sizeof(xpp));\n \t\tmemset(&xecfg, 0, sizeof(xecfg));\n+\t\txecfg.ctxlen = o->context;\n \t\txpp.flags = XDF_NEED_MINIMAL | o->xdl_opts;\n \t\txdi_diff_outf(&mf1, &mf2, diffstat_consume, diffstat,\n \t\t\t      &xpp, &xecfg, &ecb);\n\nI guess it is slightly less efficient (since we just throw away the\ncontext lines anyway), but it is nice to count the exact same patch\nthat \"git log -p\" would produce.\n\n-Peff\n"},{"id":"97657","messageId":"20081212022156.GC23128@sigill.intra.peff.net","threadId":"16689","inReplyTo":"20081212020857.GB23128@sigill.intra.peff.net","subject":"Re: git log --numstat disagrees with git apply --numstat","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-12-12T02:21:56Z","receivedAt":"2008-12-12T02:21:56Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Dec 11, 2008 at 09:08:58PM -0500, Jeff King wrote:\n\n> > which is probably just due to different xdi settings being used between\n> > the two codepaths. I haven't looked closely to see which different\n> > options we are feeding to xdiff.\n> \n> Ah. Doing this gives me the 68/12 answer for \"git log --numstat\":\n\nBTW, I got a little confused looking at the parameters to xdi_diff_outf,\nsince ecb gets passed in full of random garbage. I don't know if this\ncleanup is worth applying:\n\n-- >8 --\nremove xecb parameter to xdi_diff_outf\n\nIt is pointless to pass this parameter in instead of just\ndeclaring it locally inside the function because:\n\n  1. We overwrite every member of the struct inside the\n     function anyway, so we ignore anything passed in.\n\n  2. The contents after we return point to a local variable\n     that has gone out of scope, so it is wrong to look at\n     them.\n\nSo it is just making the interface more complex to have it\nthere, and it looks like a potential error in the callers to\nbe passing a completely uninitialized variable.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n diff.c            |   15 +++++----------\n xdiff-interface.c |    9 +++++----\n xdiff-interface.h |    2 +-\n 3 files changed, 11 insertions(+), 15 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex af822c1..1801aef 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -403,7 +403,6 @@ static void diff_words_show(struct diff_words_data *diff_words)\n {\n \txpparam_t xpp;\n \txdemitconf_t xecfg;\n-\txdemitcb_t ecb;\n \tmmfile_t minus, plus;\n \tint i;\n \n@@ -428,7 +427,7 @@ static void diff_words_show(struct diff_words_data *diff_words)\n \txpp.flags = XDF_NEED_MINIMAL;\n \txecfg.ctxlen = diff_words->minus.alloc + diff_words->plus.alloc;\n \txdi_diff_outf(&minus, &plus, fn_out_diff_words_aux, diff_words,\n-\t\t      &xpp, &xecfg, &ecb);\n+\t\t      &xpp, &xecfg);\n \tfree(minus.ptr);\n \tfree(plus.ptr);\n \tdiff_words->minus.text.size = diff_words->plus.text.size = 0;\n@@ -1436,7 +1435,6 @@ static void builtin_diff(const char *name_a,\n \t\tconst char *diffopts = getenv(\"GIT_DIFF_OPTS\");\n \t\txpparam_t xpp;\n \t\txdemitconf_t xecfg;\n-\t\txdemitcb_t ecb;\n \t\tstruct emit_callback ecbdata;\n \t\tconst struct userdiff_funcname *pe;\n \n@@ -1484,7 +1482,7 @@ static void builtin_diff(const char *name_a,\n \t\t\tecbdata.diff_words->file = o->file;\n \t\t}\n \t\txdi_diff_outf(&mf1, &mf2, fn_out_consume, &ecbdata,\n-\t\t\t      &xpp, &xecfg, &ecb);\n+\t\t\t      &xpp, &xecfg);\n \t\tif (DIFF_OPT_TST(o, COLOR_DIFF_WORDS))\n \t\t\tfree_diff_words_data(&ecbdata);\n \t\tif (textconv_one)\n@@ -1535,13 +1533,12 @@ static void builtin_diffstat(const char *name_a, const char *name_b,\n \t\t/* Crazy xdl interfaces.. */\n \t\txpparam_t xpp;\n \t\txdemitconf_t xecfg;\n-\t\txdemitcb_t ecb;\n \n \t\tmemset(&xpp, 0, sizeof(xpp));\n \t\tmemset(&xecfg, 0, sizeof(xecfg));\n \t\txpp.flags = XDF_NEED_MINIMAL | o->xdl_opts;\n \t\txdi_diff_outf(&mf1, &mf2, diffstat_consume, diffstat,\n-\t\t\t      &xpp, &xecfg, &ecb);\n+\t\t\t      &xpp, &xecfg);\n \t}\n \n  free_and_return:\n@@ -1582,14 +1579,13 @@ static void builtin_checkdiff(const char *name_a, const char *name_b,\n \t\t/* Crazy xdl interfaces.. */\n \t\txpparam_t xpp;\n \t\txdemitconf_t xecfg;\n-\t\txdemitcb_t ecb;\n \n \t\tmemset(&xpp, 0, sizeof(xpp));\n \t\tmemset(&xecfg, 0, sizeof(xecfg));\n \t\txecfg.ctxlen = 1; /* at least one context line */\n \t\txpp.flags = XDF_NEED_MINIMAL;\n \t\txdi_diff_outf(&mf1, &mf2, checkdiff_consume, &data,\n-\t\t\t      &xpp, &xecfg, &ecb);\n+\t\t\t      &xpp, &xecfg);\n \n \t\tif ((data.ws_rule & WS_TRAILING_SPACE) &&\n \t\t    data.trailing_blanks_start) {\n@@ -3009,7 +3005,6 @@ static int diff_get_patch_id(struct diff_options *options, unsigned char *sha1)\n \tfor (i = 0; i < q->nr; i++) {\n \t\txpparam_t xpp;\n \t\txdemitconf_t xecfg;\n-\t\txdemitcb_t ecb;\n \t\tmmfile_t mf1, mf2;\n \t\tstruct diff_filepair *p = q->queue[i];\n \t\tint len1, len2;\n@@ -3071,7 +3066,7 @@ static int diff_get_patch_id(struct diff_options *options, unsigned char *sha1)\n \t\txecfg.ctxlen = 3;\n \t\txecfg.flags = XDL_EMIT_FUNCNAMES;\n \t\txdi_diff_outf(&mf1, &mf2, patch_id_consume, &data,\n-\t\t\t      &xpp, &xecfg, &ecb);\n+\t\t\t      &xpp, &xecfg);\n \t}\n \n \tgit_SHA1_Final(sha1, &ctx);\ndiff --git a/xdiff-interface.c b/xdiff-interface.c\nindex d782f06..d0d60fa 100644\n--- a/xdiff-interface.c\n+++ b/xdiff-interface.c\n@@ -140,18 +140,19 @@ int xdi_diff(mmfile_t *mf1, mmfile_t *mf2, xpparam_t const *xpp, xdemitconf_t co\n int xdi_diff_outf(mmfile_t *mf1, mmfile_t *mf2,\n \t\t  xdiff_emit_consume_fn fn, void *consume_callback_data,\n \t\t  xpparam_t const *xpp,\n-\t\t  xdemitconf_t const *xecfg, xdemitcb_t *xecb)\n+\t\t  xdemitconf_t const *xecfg)\n {\n \tint ret;\n \tstruct xdiff_emit_state state;\n+\txdemitcb_t xecb;\n \n \tmemset(&state, 0, sizeof(state));\n \tstate.consume = fn;\n \tstate.consume_callback_data = consume_callback_data;\n-\txecb->outf = xdiff_outf;\n-\txecb->priv = &state;\n+\txecb.outf = xdiff_outf;\n+\txecb.priv = &state;\n \tstrbuf_init(&state.remainder, 0);\n-\tret = xdi_diff(mf1, mf2, xpp, xecfg, xecb);\n+\tret = xdi_diff(mf1, mf2, xpp, xecfg, &xecb);\n \tstrbuf_release(&state.remainder);\n \treturn ret;\n }\ndiff --git a/xdiff-interface.h b/xdiff-interface.h\nindex 7352b9a..491037d 100644\n--- a/xdiff-interface.h\n+++ b/xdiff-interface.h\n@@ -10,7 +10,7 @@ int xdi_diff(mmfile_t *mf1, mmfile_t *mf2, xpparam_t const *xpp, xdemitconf_t co\n int xdi_diff_outf(mmfile_t *mf1, mmfile_t *mf2,\n \t\t  xdiff_emit_consume_fn fn, void *consume_callback_data,\n \t\t  xpparam_t const *xpp,\n-\t\t  xdemitconf_t const *xecfg, xdemitcb_t *xecb);\n+\t\t  xdemitconf_t const *xecfg);\n int xdi_diff_hunks(mmfile_t *mf1, mmfile_t *mf2,\n \t\t   xdiff_emit_hunk_consume_fn fn, void *consume_callback_data,\n \t\t   xpparam_t const *xpp, xdemitconf_t *xecfg);\n-- \n1.6.1.rc2.307.gb39e0.dirty\n"},{"id":"97939","messageId":"7vhc55yd58.fsf@gitster.siamese.dyndns.org","threadId":"16689","inReplyTo":"20081212022156.GC23128@sigill.intra.peff.net","subject":"Re: git log --numstat disagrees with git apply --numstat","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-12-15T09:57:07Z","receivedAt":"2008-12-15T09:57:07Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> BTW, I got a little confused looking at the parameters to xdi_diff_outf,\n> since ecb gets passed in full of random garbage. I don't know if this\n> cleanup is worth applying:\n\nI think this makes sense, but let's do this after 1.6.1 final.  It does\nnot fix anything, and I'd rather avoid distraction.\n"},{"id":"98000","messageId":"20081215203225.GA19964@sigill.intra.peff.net","threadId":"16689","inReplyTo":"7vhc55yd58.fsf@gitster.siamese.dyndns.org","subject":"Re: git log --numstat disagrees with git apply --numstat","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-12-15T20:32:26Z","receivedAt":"2008-12-15T20:32:26Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Dec 15, 2008 at 01:57:07AM -0800, Junio C Hamano wrote:\n\n> Jeff King <peff@peff.net> writes:\n> \n> > BTW, I got a little confused looking at the parameters to xdi_diff_outf,\n> > since ecb gets passed in full of random garbage. I don't know if this\n> > cleanup is worth applying:\n> \n> I think this makes sense, but let's do this after 1.6.1 final.  It does\n> not fix anything, and I'd rather avoid distraction.\n\nAgreed. I will re-post both patches after 1.6.1 is released.\n\n-Peff\n"}]}