Annotate not working in linux kernel

[htot]

Since Ubuntu 22.10 (kinetic) qgit will not annotate (ctrl-a) files from a linux kernel repo. It does annotate other (smaller?) repo's. The status line says "Sorry, annotation not available for this file".

Kinetic has qgit 2.10 same as Jammy. But with Jammy I never had this issue. Does it depend on underlying git (Kinetic 2.37, Jammy 2.34)?

[htot]

Additionally, on the terminal I get

ASSERT: merging annotations of different length
 merging 1805b2f04855f07afe3a71d620a68f483b0ed74f in 8a9ea3237e7eb5c25f09e429ad242ae5a3d5ea22

(those are 2011 commits)

[htot]

@tibirna ?

[tibirna]

I don't have ubuntu around. The time needed for me to install it, you could try to see if git itself (at the command line) is able to do the same annotate you try in qgit. Please report back with the finding.

Also, please specify here where in the kernel git history you are positioned and which file do you try to annotate.

[htot]

I makes no difference which file or which location in the history. Also I found in other repo's (but not all) the same problem.

Which git command would you like me to try exactly?

[tibirna]

Identify the sha of the currently selected commit in your qgit that fails to annotate. Identify the file on which you try to do Ctrl-A and it fails. Then do, at a console:

git blame :<path/filename>

And let me know if you get the annotations back or some error.

For the record, I can't reproduce the issue you have. I tried about 30 files at random at the top of the current kernel git and all works as expected.

A wild guess I have is that you might have corrupted qgit cache. I never encountered this before, but just for testing try to quit your qgit and then remove the file <kernel_repo>/.git/qgit_cache.dat, then open your qgit and try again.

[htot]

Again: any file and any sha I tried have the same result.

But as an example: drivers/iio/light/tsl2563.c

On the command line where I started qgit:
ASSERT: merging annotations of different length merging ceb675a9e25c0c11f76f8e72a862caf08d3934d3 in 74e1a2a39355b2d3ae8c60c78d8add162c6d7183

And

git blame tsl2563.c > tsl2563.ann

2b27bdcc20958 drivers/iio/light/tsl2563.c         (Thomas Gleixner          2019-05-29 16:57:50 -0700   1) // SPDX-License-Identifier: GPL-2.0-only
ee1f1fa407141 drivers/staging/iio/light/tsl2563.c (Amit Kucheria            2009-11-09 15:14:28 +0200   2) /*
9c2251dd4b7fe drivers/iio/light/tsl2563.c         (Jonathan Cameron         2013-01-12 10:35:00 +0000   3)  * drivers/iio/light/tsl2563.c
ee1f1fa407141 drivers/staging/iio/light/tsl2563.c (Amit Kucheria            2009-11-09 15:14:28 +0200   4)  *
ee1f1fa407141 drivers/staging/iio/light/tsl2563.c (Amit Kucheria            2009-11-09 15:14:28 +0200   5)  * Copyright (C) 2008 Nokia Corporation
ee1f1fa407141 drivers/staging/iio/light/tsl2563.c (Amit Kucheria            2009-11-09 15:14:28 +0200   6)  *
ee1f1fa407141 drivers/staging/iio/light/tsl2563.c (Amit Kucheria            2009-11-09 15:14:28 +0200   7)  * Written by Timo O. Karjalainen <[email protected]>
ee1f1fa407141 drivers/staging/iio/light/tsl2563.c (Amit Kucheria            2009-11-09 15:14:28 +0200   8)  * Contact: Amit Kucheria <[email protected]>
ee1f1fa407141 drivers/staging/iio/light/tsl2563.c (Amit Kucheria            2009-11-09 15:14:28 +0200   9)  *
ee1f1fa407141 drivers/staging/iio/light/tsl2563.c (Amit Kucheria            2009-11-09 15:14:28 +0200  10)  * Converted to IIO driver
ee1f1fa407141 drivers/staging/iio/light/tsl2563.c (Amit Kucheria            2009-11-09 15:14:28 +0200  11)  * Amit Kucheria <[email protected]>
ee1f1fa407141 drivers/staging/iio/light/tsl2563.c (Amit Kucheria            2009-11-09 15:14:28 +0200  12)  */
ee1f1fa407141 drivers/staging/iio/light/tsl2563.c (Amit Kucheria            2009-11-09 15:14:28 +0200  13) 
63a7c95af2d65 drivers/iio/light/tsl2563.c         (Andy Shevchenko          2022-11-30 01:00:17 +0200  14) #include <linux/bits.h>
43f50c6d80677 drivers/iio/light/tsl2563.c         (Andy Shevchenko          2022-11-30 01:00:23 +0200  15) #include <linux/delay.h>
43f50c6d80677 drivers/iio/light/tsl2563.c         (Andy Shevchenko          2022-11-30 01:00:23 +0200  16) #include <linux/err.h>
ee1f1fa407141 drivers/staging/iio/light/tsl2563.c (Amit Kucheria            2009-11-09 15:14:28 +0200  17) #include <linux/i2c.h>
ee1f1fa407141 drivers/staging/iio/light/tsl2563.c (Amit Kucheria            2009-11-09 15:14:28 +0200  18) #include <linux/interrupt.h>
388be48839528 drivers/staging/iio/light/tsl2563.c (Jonathan Cameron         2010-06-26 12:54:21 +0100  19) #include <linux/irq.h>
3f718aaeaaef7 drivers/iio/light/tsl2563.c         (Andy Shevchenko          2022-11-30 01:00:18 +0200  20) #include <linux/math.h>
43f50c6d80677 drivers/iio/light/tsl2563.c         (Andy Shevchenko          2022-11-30 01:00:23 +0200  21) #include <linux/mod_devicetable.h>
43f50c6d80677 drivers/iio/light/tsl2563.c         (Andy Shevchenko          2022-11-30 01:00:23 +0200  22) #include <linux/module.h>
ee1f1fa407141 drivers/staging/iio/light/tsl2563.c (Amit Kucheria            2009-11-09 15:14:28 +0200  23) #include <linux/mutex.h>

I had already tried deleting the cache, no effect.

Also: Ubuntu 22.04 has the same version of qgit as 22.10. However, git was updated from 2.34.1 to 2.37.2.

[tibirna]

Thanks for the investigation. I have git 2.35 here. So there might be something into the update to git 2.37. I'll check it when I'll find some time (need find that version for my distro).

[tibirna]

OK, I can reproduce the issue with git 2.38. I'll try to debug this. Thanks.

[htot]

Good you can reproduce, if there is anything I can do to help let me know.

BTW thanks for QGit! It's the only git viewer that really helps to make sense of complicated repo's like linux. Especially because of the annotation of files.

[htot]

Have you been able to debug this?

[tibirna]

@htot Sorry for the late answer. No, I didn't advance on that. I need contiguous time to set up a VM with the proper dev tools and the deviating git binary. I plan to look into it in the next 2 weeks. Thanks for your understanding.

[htot]

No need to apologize. Just curious.

[htot]

I uninstalled git, git-e-mail and git-man and then manually downloaded and installed these packages Ubuntu Jammy as a stopgap. Then I pinned these packages to prevent updating. However, lunar is releasing soon, with git 2.39.

[millnet-maho]

I have encountered the same issue with git 2.39 (Debian 12 "bookworm"). git 2.30 works.
Obviously the bug isn't in Annotate::doAnnotate(); rather, it gets bad data in. Something is wrong with the file history, causing it to include every merge commit in the history of every file, as seen in @htot's QGit screenshot.

I haven't figured out how everything works yet, but if it's of any help, when I debug, I can see that the parents is wrong. Instead of the actual parents, r->parents() lists the closest ancestor that is a merge commit. But those are simply the commits that are shown as parents in the file history view, and maybe this doesn't explain why the annotations to be merged have different lengths. The "different lengths" assertion error also only happens sometimes.

The output from git log --topo-order --no-color --parents --boundary -z '--pretty=format:%m%HX%PX%n%cn<%ce>%n%an<%ae>%n%at%n%s%n' is the same from both git 2.30 and git 2.39. Is that what's used to build the file history?

[millnet-maho]

OK, that wasn't the full command. Something has changed with respect to the -m flag. Previously, it made diffs be included for merge commits. Now it seems to make every merge commit be included in the log, even those without diffs. Might be a bug in git?

[millnet-maho]

Yeah, git --diff-merges=separate doesn't seem to be working properly. Not only does it include irrelevant merge commits; it doesn't even generate the full diff with respect to each parent it's supposed to. The other formats seem to work as documented, however.

Also note that -m (or --diff-merges=on) now uses the default format, which can be changed using the log.diffMerges configuration parameter.

[millnet-maho]

The commit that introduces the changed behaviour is git/git@0dec322. I'm not sure that I understand or agree with the explanation in the commit message, but it can be counteracted with --simplify-merges (which is ancient), if I'm not mistaken.

Git 2.32 is the version that changed the meaning of -m and introduced the various style options to --diff-merges (see https://github.com/git/git/blob/master/Documentation/RelNotes/2.32.0.txt), but rather than using different parameters, the default style can be forced with git -c log.diffMerges=separate.

[tibirna]

Thank you very much, Magnus, for all the hard work!

[millnet-maho]

Upon further testing it appears that while --simplify-merges often makes stuff work, the result is not the same as when running Git 2.35 without it. I think that any time there's a merge commit that only modifies the file in question from one side, you still get the "merging annotations of different length". I assume it's because the log output only includes one log record in that situation, not one for each parent. Previously those commits were omitted (which was actually a bit confusing, because it looked like some changes hadn't been merged even though they had). This situation for example arises when two lineages introduce the same change and one of them also introduces other changes (or not?).

Can we check for missing diffs and insert empty ones, or whatever is necessary to complete the data structure?

[tibirna]

Thanks for the further investigation. Yes, I had started looking into the assumption taken in the original analysis of logs in qgit and then I lacked time. Unfortunately, this will be the case until at least December. For who wants to look into the matter, qgit's log parsing is not super complicated. It all goes on inside git.cpp and it doesn't do any fancy stuff, just basic string chunking/parsing. It just needs a careful, rested and patient mind. Thanks Cristian

…

On Fri, Sep 8, 2023 at 11:14 AM Magnus Holmgren ***@***.***> wrote: Upon further testing it appears that while --simplify-merges often makes stuff work, the result is not the same as when running Git 2.35 without it. I think that any time there's a merge commit that only modifies the file in question from one side, you still get the "merging annotations of different length". I assume it's because the log output only includes one log record in that situation, not one for each parent. Previously those commits were omitted (which was actually a bit confusing, because it looked like some changes hadn't been merged even though they had). This situation for example arises when two lineages introduce the same change and one of them also introduces other changes (or not?). Can we check for missing diffs and insert empty ones, or whatever is necessary to complete the data structure? — Reply to this email directly, view it on GitHub <#129 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAJOWDWULY26X7HMMXHXZ63XZMY4LANCNFSM6AAAAAASDYO4DI> . You are receiving this because you modified the open/close state.Message ID: ***@***.***>

[millnet-maho]

I think the whole problem is that in Git::addChunk(), mergeSha is constructed by taking the next free number instead of the correct index from the list of parents, but the custom pretty format doesn't include the information of which parent the diff is relative to. The default format includes that information in parentheses ("commit 5d42f7cae786035659034e44ad6677f28ece980a (from d1a69b4b50dc2ab9408adb66c08d88a1e635c0a1)"), but I can't find any format code for that in the manpage...

[millnet-maho]

This means that as long as long as all merge commits affecting a specific file merge some change into the main branch (i.e. there is a difference relative to the first parent, or in the case of a multi-way merge, relative to all parents except the last), you'll be fine.
I had an idea that we could use the object IDs (fileSha) of the parents to distinguish between the parents, since we're looking at a single file, but I haven't got it to work yet.

[wRAR]

I'm not sure what is the actual status of the fix but should this be reopened if this is not fixed?

I have a bug report in Debian about this problem and I'd like to know if it's fine to apply the linked PR or if I should wait for a more comprehensive fix.

[tibirna]

You're right. I reopen it, lest I forget about it.

[htot]

I'm not sure what is the actual status of the fix but should this be reopened if this is not fixed?

I have a bug report in Debian about this problem and I'd like to know if it's fine to apply the linked PR or if I should wait for a more comprehensive fix.

I just built master df6326c tonight and used it to check the history of a file in linux. To me it appears as if the bug is fixed - or at least good enough to make qgit useful again. Like I mentioned before my main use of qgit is to quickly find patches that changed a particular file.

EDIT I am seeing now it is better but not completely right. When I select a file with ctrl-A I get the list of patches that also modified this file, but the list is polluted with merge commits that did not touch this file.

So, thanks guys, I hope to see this fix landing in Debian and Ubuntu!

BTW while building with g++ 12.3.0 there are quite a bit of warning and notes. Harmless?

[htot]

See my edit above

[tibirna]

Hello @htot

Thanks for the testing. As @millnet-maho says, his fix indeed works some of the time, but not all. The log outputs in git proper changed in a complex way that requires more complex adaptations in the qgit parser. It was a design choice long ago to parse the git output. I didn't check, but I would believe the libgit2 was not yet available at that time. The proper work to do is to switch to using libgit2, this is something in my todo, but is a rather consequent investment of time and effort.

I never consider warnings harmless. I still use g++ 7 here. I'll get v12 and I'll check, when time allows. Thanks for the heads-up.

[ReformCopyright]

EDIT I am seeing now it is better but not completely right. When I select a file with ctrl-A I get the list of patches that also modified this file, but the list is polluted with merge commits that did not touch this file.

Hmm, getting rid of the merge commits that don't touch the file is exactly what the change should do (by passing --simplify-merges to git). What doesn't work is annotations when there's a merge commit that doesn't merge in any changes.

[htot]

Yeah you're right. The merge commits are out of the way. But for some files it says

while on the CLI:

ASSERT: merging annotations of different length
 merging cab18d19bb05452fc7f5c45f7964643bdae80c92 in d027db132b395dabfac208e52a7e510e441bb9d2

for others it works fine and nothing on the CLI.

[htot]

Is there anybody working on this issue?

[tibirna]

As I lacked the time, I take it there is nobody for now. Thanks for your understanding.

[htot]

That is a pity, I couldn't find any other git viewer with such useful presentation of the history of changes per file. I hope you will be able to find time for this in the near future. Thanks!