{"thread":{"id":"59806","subject":"notes handling broken on rebase","startedAt":"2023-05-30T09:23:58Z","lastAt":"2023-05-31T13:31:51Z","messageCount":2,"participants":["Uwe Kleine-König","Phillip Wood"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"477793","messageId":"20230530092155.3zbb5uxa7eisdzxb@pengutronix.de","threadId":"59806","inReplyTo":null,"subject":"notes handling broken on rebase","fromName":"Uwe Kleine-König","fromEmail":"u.kleine-koenig@pengutronix.de","sentAt":"2023-05-30T09:21:55Z","receivedAt":"2023-05-30T09:23:58Z","isPatch":false,"sender":{"key":"u.kleine-koenig@pengutronix.de","avatar":"https://gravatar.com/avatar/354b5e3ceb2806a2f1e1e382ac29ddbdad18288654da62b61eb13583a857eee7?d=mp&s=160"},"body":"Hello,\n\nI found a bug in git rebase and how notes are retained using\n\n\tuwe@taurus:~/tmp/gittest$ git version\n\tgit version 2.41.0.rc2.161.g9c6817b8e7dd\n\n, it also happens with 2.39.2.\n\nHere comes a reproducer:\n\n - Create some initial git history, just three commits A, B and C:\n\n\tuwe@taurus:~/tmp/gittest$ git init\n\tInitialized empty Git repository in /home/uwe/tmp/gittest/.git/\n\n\tuwe@taurus:~/tmp/gittest$ for i in A B C; do echo content $i >> file ; git add file ; git commit -m \"$i\" ; git notes add -m \"notes for $i\" ; git tag $i ; done\n\t[main (root-commit) db2548de5a3c] A\n\t 1 file changed, 1 insertion(+)\n\t create mode 100644 file\n\t[main 2b8fa82afb3c] B\n\t 1 file changed, 1 insertion(+)\n\t[main 3dd5e4a2037e] C\n\t 1 file changed, 1 insertion(+)\n\n - Pick C on top of A:\n\n\tuwe@taurus:~/tmp/gittest$ git checkout A\n\tHEAD is now at db2548de5a3c A\n\tuwe@taurus:~/tmp/gittest$ git cherry-pick C\n\tAuto-merging file\n\tCONFLICT (content): Merge conflict in file\n\terror: could not apply 3dd5e4a2037e... C\n\thint: After resolving the conflicts, mark them with\n\thint: \"git add/rm <pathspec>\", then run\n\thint: \"git cherry-pick --continue\".\n\thint: You can instead skip this commit with \"git cherry-pick --skip\".\n\thint: To abort and get back to the state before \"git cherry-pick\",\n\thint: run \"git cherry-pick --abort\".\n\tRecorded preimage for 'file'\n\tuwe@taurus:~/tmp/gittest$ { echo content A; echo content C; } > file\n\tuwe@taurus:~/tmp/gittest$ git add file\n\tuwe@taurus:~/tmp/gittest$ git cherry-pick --continue\n\tRecorded resolution for 'file'.\n\t[detached HEAD 6cfc9b7bd437] C\n\t Date: Tue May 30 11:02:13 2023 +0200\n\t 1 file changed, 1 insertion(+)\n\n - Rebase {B, C} on top of A + C' (which results in skipping C):\n\n\tuwe@taurus:~/tmp/gittest$ git rebase HEAD C\n\tAuto-merging file\n\tCONFLICT (content): Merge conflict in file\n\terror: could not apply 2b8fa82afb3c... B\n\thint: Resolve all conflicts manually, mark them as resolved with\n\thint: \"git add/rm <conflicted_files>\", then run \"git rebase --continue\".\n\thint: You can instead skip this commit: run \"git rebase --skip\".\n\thint: To abort and get back to the state before \"git rebase\", run \"git rebase --abort\".\n\tRecorded preimage for 'file'\n\tCould not apply 2b8fa82afb3c... B\n\tuwe@taurus:~/tmp/gittest$ { echo content A; echo content B; echo content C; } > file\n\tuwe@taurus:~/tmp/gittest$ git add file\n\tuwe@taurus:~/tmp/gittest$ git rebase --continue\n\tRecorded resolution for 'file'.\n\t[detached HEAD e5b3e28216f2] B\n\t 1 file changed, 1 insertion(+)\n\tdropping 3dd5e4a2037ecd31f5470561638cb065434eabbc C -- patch contents already upstream\n\tSuccessfully rebased and updated detached HEAD.\n\nNow I have:\n\n\tuwe@taurus:~/tmp/gittest$ git log --pretty=oneline --abbrev-commit --notes\n\te5b3e28216f2 (HEAD) B\n\tNotes:\n\t    notes for B\n\t    \n\t    notes for C\n\n\t6cfc9b7bd437 C\n\tdb2548de5a3c (tag: A) A\n\tNotes:\n\t    notes for A\n\nThe surprising part here is that (the new) B also has C's note. I would\nhave expected that it only has its own one. I quickly looked into the\nsource code, but didn't find an easy fix because the code that notices\nthat C should be dropped (in do_pick_commit()) isn't where the note is\ncopied (which I didn't find).\n\nBest regards\nUwe\n\n-- \nPengutronix e.K.                           | Uwe Kleine-König            |\nIndustrial Linux Solutions                 | https://www.pengutronix.de/ |\n"},{"id":"477832","messageId":"c180ce59-459b-ee99-e4c0-c2fa6c765d77@gmail.com","threadId":"59806","inReplyTo":"20230530092155.3zbb5uxa7eisdzxb@pengutronix.de","subject":"Re: notes handling broken on rebase","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2023-05-31T13:29:46Z","receivedAt":"2023-05-31T13:31:51Z","isPatch":false,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Uwe\n\nOn 30/05/2023 10:21, Uwe Kleine-König wrote:\n> Hello,\n> \n> I found a bug in git rebase and how notes are retained using\n> \n> \tuwe@taurus:~/tmp/gittest$ git version\n> \tgit version 2.41.0.rc2.161.g9c6817b8e7dd\n> \n> , it also happens with 2.39.2.\n> \n> Here comes a reproducer:\n\nThanks for reporting this and taking the time to provide an easy \nreproducer. I think the problem is that when git drops the commit in \ndo_pick_commit() it still gets recorded as rewritten and so the notes \nare copied. To fix it we need to improve do_pick_commit() to return a \nvalue indicating that the commit was dropped so that we don't call \nrecord_in_rewritten() in pick_commits().\n\nBest Wishes\n\nPhillip\n\n\n>   - Create some initial git history, just three commits A, B and C:\n> \n> \tuwe@taurus:~/tmp/gittest$ git init\n> \tInitialized empty Git repository in /home/uwe/tmp/gittest/.git/\n> \n> \tuwe@taurus:~/tmp/gittest$ for i in A B C; do echo content $i >> file ; git add file ; git commit -m \"$i\" ; git notes add -m \"notes for $i\" ; git tag $i ; done\n> \t[main (root-commit) db2548de5a3c] A\n> \t 1 file changed, 1 insertion(+)\n> \t create mode 100644 file\n> \t[main 2b8fa82afb3c] B\n> \t 1 file changed, 1 insertion(+)\n> \t[main 3dd5e4a2037e] C\n> \t 1 file changed, 1 insertion(+)\n> \n>   - Pick C on top of A:\n> \n> \tuwe@taurus:~/tmp/gittest$ git checkout A\n> \tHEAD is now at db2548de5a3c A\n> \tuwe@taurus:~/tmp/gittest$ git cherry-pick C\n> \tAuto-merging file\n> \tCONFLICT (content): Merge conflict in file\n> \terror: could not apply 3dd5e4a2037e... C\n> \thint: After resolving the conflicts, mark them with\n> \thint: \"git add/rm <pathspec>\", then run\n> \thint: \"git cherry-pick --continue\".\n> \thint: You can instead skip this commit with \"git cherry-pick --skip\".\n> \thint: To abort and get back to the state before \"git cherry-pick\",\n> \thint: run \"git cherry-pick --abort\".\n> \tRecorded preimage for 'file'\n> \tuwe@taurus:~/tmp/gittest$ { echo content A; echo content C; } > file\n> \tuwe@taurus:~/tmp/gittest$ git add file\n> \tuwe@taurus:~/tmp/gittest$ git cherry-pick --continue\n> \tRecorded resolution for 'file'.\n> \t[detached HEAD 6cfc9b7bd437] C\n> \t Date: Tue May 30 11:02:13 2023 +0200\n> \t 1 file changed, 1 insertion(+)\n> \n>   - Rebase {B, C} on top of A + C' (which results in skipping C):\n> \n> \tuwe@taurus:~/tmp/gittest$ git rebase HEAD C\n> \tAuto-merging file\n> \tCONFLICT (content): Merge conflict in file\n> \terror: could not apply 2b8fa82afb3c... B\n> \thint: Resolve all conflicts manually, mark them as resolved with\n> \thint: \"git add/rm <conflicted_files>\", then run \"git rebase --continue\".\n> \thint: You can instead skip this commit: run \"git rebase --skip\".\n> \thint: To abort and get back to the state before \"git rebase\", run \"git rebase --abort\".\n> \tRecorded preimage for 'file'\n> \tCould not apply 2b8fa82afb3c... B\n> \tuwe@taurus:~/tmp/gittest$ { echo content A; echo content B; echo content C; } > file\n> \tuwe@taurus:~/tmp/gittest$ git add file\n> \tuwe@taurus:~/tmp/gittest$ git rebase --continue\n> \tRecorded resolution for 'file'.\n> \t[detached HEAD e5b3e28216f2] B\n> \t 1 file changed, 1 insertion(+)\n> \tdropping 3dd5e4a2037ecd31f5470561638cb065434eabbc C -- patch contents already upstream\n> \tSuccessfully rebased and updated detached HEAD.\n> \n> Now I have:\n> \n> \tuwe@taurus:~/tmp/gittest$ git log --pretty=oneline --abbrev-commit --notes\n> \te5b3e28216f2 (HEAD) B\n> \tNotes:\n> \t    notes for B\n> \t\n> \t    notes for C\n> \n> \t6cfc9b7bd437 C\n> \tdb2548de5a3c (tag: A) A\n> \tNotes:\n> \t    notes for A\n> \n> The surprising part here is that (the new) B also has C's note. I would\n> have expected that it only has its own one. I quickly looked into the\n> source code, but didn't find an easy fix because the code that notices\n> that C should be dropped (in do_pick_commit()) isn't where the note is\n> copied (which I didn't find).\n> \n> Best regards\n> Uwe\n> \n\n"}]}