Serious bug with pretty format strings & empty bodies?

I'm struggling to come up with a minimal test case that confirms this, but I seem to be hitting a bug in the pretty string formatter when trying to print bodies of commits that don't have bodies.

For example, on a private repository, I'm trying to print the subject & body of a commit. This particular commit doesn't actually have a body, so I'm expecting to see this output :

================= commit 18d2480ab689b483ef1ebbdb3f7420904049ba0b Try to flush log files before terminating the app

=================

However, when I actually run the command, I get gibberish in place of %b : ================= $ git rev-list -1- --pretty=format:"%s%n%b" 18d2480ab689b483ef1ebbdb3f7420904049ba0b commit 18d2480ab689b483ef1ebbdb3f7420904049ba0b Try to flush log files before terminating the app tree 57bc7cf30a10aee96251852125cf30fd2c81d7aa parent 04c833865828538315fcdf6e187da077869ce444 author Jonathan del Strother <jon.delStrother@bestbefore.tv> 1197901755 +0000 committer Jonathan del Strother <jon.delStrother@bestbefore.tv> 1197901755 +0000

Check that ThreadWorker's work method actually returns a value with method signatures =================

So here you can see that the full details of a different commit is appearing in place of the requested commit's (empty) body. Rerunning the rev-list produces the same output - the same string appears in place of %b every time, but it's different for each commit. Often you only get a small fragment of another commit's message, rather than the full commit as seen above.

When I use --pretty=full, it works fine.

This appears in quite a few places in my repository's history. Repacking the repo semi-fixes it - I still get junk bodies, but not quite so many.

It doesn't always happen when a commit message body is empty, but I've never seen it happen where the message body has something in.

This happens with both git 1.5.3.7 and 1.5.4.rc0.1162.g3bfea, though the output is slightly different - the former has a number of <unknown> markers in it. This is on OS X 10.5.1

Any suggestions? Cheers, Jon

Jonathan del Strother schrieb:

That's strange. Could you check if this happens with e52a5de, too? That's the commit that introduced --pretty=format. If it doesn't, could you then please try to bisect the bug?

How many '<unknown>' markers are there in the output of version 1.5.3.7 (and e52a5de)? One per %b? Are they the only output or are they combined with parts of unrelated commits, too?

Thanks, René

On Dec 19, 2007 6:28 PM, René Scharfe <rene.scharfe@lsrfire.ath.cx> wrote:

As far as I can tell, e52a... never compiled - it's missing prefixcmp from git-compat-util.h, which doesn't get merged in till 8ab3e18. And yes, I still get junk output from that build. (Shame, I was looking forward to trying out the bisect tool...)

Only one per %b, and where they appear, they're the only output. However, even with those builds, I get junk output in addition to the <unknown> markers. That is, with a format string of "**%b**", there are three possible outputs : The correct one - eg **Added test for NSDictionary generator** The unknown one - eg **<unknown>** The complete junk one - eg **n del Strothereb0d794bacc1761e8a0681e9b144c514702d9b7a**

The latter's 'body' is almost always terminated by an unprintable character, which always seems to be 03.

Hmmm. I wonder if it's coincidence that I can only seem to spot this problem in my repos that have been set up via submodule init...

Ignore that last bit, red herring.

Jonathan del Strother, Wed, Dec 19, 2007 15:32:20 +0100:

I tried

    $ git init
    $ git rev-list --pretty=format:"%s%n%b" \
	$(echo -n "Test" |git commit-tree $(git write-tree))

but it works. It is possible that the commit in question is created with an older commit-tree code. Could you try

    git cat-file 18d2480ab689b483ef1ebbdb3f7420904049ba0b

(or any other problematic commit) and post its output here? Or is the repo publised somewhere?

On Dec 19, 2007 6:44 PM, Alex Riesen <raa.lkml@gmail.com> wrote:

You mean git cat-file commit ... ? I get the normal output, but the problematic commits don't show a newline character at the end of the cat-file output.

Afraid not - it's private code.

I'll try bisecting e52a5de tomorrow, see if that gets me anywhere.

Jonathan del Strother schrieb:

Just a shot in the dark: does this patch on top of master make a difference?

On Dec 19, 2007 11:23 PM, René Scharfe <rene.scharfe@lsrfire.ath.cx> wrote:

No luck with that, I'm afraid.

Jonathan del Strother schrieb:

And how is this one?

The first chunk is the same as in the last try -- it guards against commit messages ending with a NUL without a prior \n _and_ being followed by a \n (in memory which shouldn't be accessed by us at all as it doesn't belong to the commit message). I guess that's quite rare.

The second chunk keeps the body offset from being incremented by the for loop if we've already found a terminating NUL.

Hi,

On Thu, 20 Dec 2007, Ren? Scharfe wrote:

Ah!  I missed that one *writethisdownintothebook*

Thanks, Dscho

On Dec 20, 2007 12:20 PM, René Scharfe <rene.scharfe@lsrfire.ath.cx> wrote:

Winnar!

Yep, that seems to have fixed things. Wonder how I managed to get the null into the commit message...

Has anyone actually managed to reproduce my problem? I've got multiple repos here that show the problem in several commits, made by different people. However, I can't actually come up with a way to reproduce it at will...

I'd really like to see René's patch accepted as it seems to magically fix all my problems, but it's a bit hard to justify if I'm the only one that's seeing these broken commit messages.

"Jonathan del Strother" <maillist@steelskies.com> writes:

Marking an issue as a "serious bug" without giving enough material for reproduction nor diagnosis tends to discourage people from looking into it seriously, as the issue cannot be even judged if it is really serious. For example:

  On Dec 19, 2007 6:44 PM, Alex Riesen <raa.lkml@gmail.com> wrote:
  > Could you try
  >
  >     git cat-file 18d2480ab689b483ef1ebbdb3f7420904049ba0b
  >
  > (or any other problematic commit) and post its output here?

  You mean git cat-file commit ... ?
  I get the normal output, but the problematic commits don't show a
  newline character at the end of the cat-file output.

"I get the normal output" is not what Alex asked you to supply, nor would be sufficient information. There may be some abnormality in the commit object that you probably did not spot, but Alex or other people may have been able to if you were actually posted its output here.

I have been suspecting that there is a NUL in the middle of the message somehow (I know the lowest-level plumbing commit-tree allows any byte sequence in the log message if you worked hard enough), which the parser is not prepared to cope with, but we haven't seen enough evidence to support nor refute that theory. We do not have much to work from.

One thing I noticed funny in your original message was "-1-". Is it essential that the number is spelled incorrectly to reproduce this problem?

  $ git rev-list -1- --pretty=format:"%s%n%b"
  18d2480ab689b483ef1ebbdb3f7420904049ba0b
  commit 18d2480ab689b483ef1ebbdb3f7420904049ba0b
  Try to flush log files before terminating the app
  tree 57bc7cf30a10aee96251852125cf30fd2c81d7aa
  parent 04c833865828538315fcdf6e187da077869ce444
  author Jonathan del Strother <jon.delStrother@bestbefore.tv> 1197901755 +0000
  committer Jonathan del Strother <jon.delStrother@bestbefore.tv> 1197901755 +0000

  Check that ThreadWorker's work method actually returns a value with
  method signatures

On Dec 22, 2007 8:39 AM, Junio C Hamano <gitster@pobox.com> wrote:

======== $ git cat-file commit 18d2480ab689b483ef1ebbdb3f7420904049ba0b tree 2416c620c6f7c1864065a6e778588b71b3e0bd5d parent 9fc80d0a05835c68885f253844ab586b38d09202 author Jonathan del Strother <jon.delStrother@bestbefore.tv> 1197907511 +0000 committer Jonathan del Strother <jon.delStrother@bestbefore.tv> 1197907511 +0000

Try to flush log files before terminating the app% ======== The '%' suffix is zsh showing that there's no newline there. Under bash I get no '%' and the next prompt starts immediately following 'app'.

No, sorry - I don't know how that got there. I get identical output whether it's "rev-list -1" or "rev-list -1-"

I've been trying to reproduce this on my home repo, and have only been partially successful. I haven't been able to make git rev-list -1 show a malformed message body. I can only get the problem to occur when printing multiple commits (eg git rev-list -20 ...) Bizarrely, the first 2 commits shown in rev-list never show a problem. After the first 2, commits with the broken message bodies will always appear broken, regardless of their position in the list. However, the content that appears instead of the body will change depending on the commits that appear prior to that one.

I realise this doesn't appear to mesh with my observations the other day, where I was seeing the broken bodies even when just using rev-list -1 - I'm at a loss to describe it. Both machines are intel macs (one a MacPro, one a MacBook Pro), running OS X 10.5.1, with the same git version (1.5.4.rc0.1162.g3bfea). Unfortunately I've switched my work machine off for christmas, or I'd ssh in and try to re-verify things.

I do appreciate people's help in trying to reproduce & fix this - I realise I've not given people much to go on.

I ended up trying to filter-branch my repository, see if I could come up with a version stripped of all our private code, suitable for making public. Disappointingly, filter-branch magically fixes all the commits, even when simply run as "git filter-branch HEAD".

However, looking at the rewritten repository vs the original, they share a lot of commits, then diverge halfway through their history. I can't see anything notable about the commit where they diverge, with the exception that the rewritten commit has a newline after the subject and the original doesn't. Neither has a commit message body.

Hi,

On Thu, 20 Dec 2007, Jonathan del Strother wrote:

I do not see how Rene's patch could _not_ have fixed that problem.

As it is, I cannot even reproduce without that patch. This is what I was trying:

-- snip --