{"thread":{"id":"55831","subject":"git feature request: git blame --ignore-cleanup/--ignore-trivial","startedAt":"2021-06-02T15:20:50Z","lastAt":"2021-06-03T15:33:59Z","messageCount":8,"participants":["Mathieu Desnoyers","Al Viro","Jeff King","Taylor Blau","Felipe Contreras","David Sterba"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"426238","messageId":"30399052.5964.1622647235870.JavaMail.zimbra@efficios.com","threadId":"55831","inReplyTo":null,"subject":"git feature request: git blame --ignore-cleanup/--ignore-trivial","fromName":"Mathieu Desnoyers","fromEmail":"mathieu.desnoyers@efficios.com","sentAt":"2021-06-02T15:20:35Z","receivedAt":"2021-06-02T15:20:50Z","isPatch":false,"sender":{"key":"mathieu.desnoyers@efficios.com","avatar":"https://gravatar.com/avatar/c3872f90e251e72cba6a65443eb6a4144e73ab1b2e08149cf78eb5d4779a88dd?d=mp&s=160"},"body":"Hi,\n\nFollowing a discussion with Peter Zijlstra about whether code cleanup\nand functional changes done to the Linux kernel scheduler belong to separate\npatches or should be folded together, the argument for folding cleanup\nand function changes came to be mainly motivated by the current behavior\nof git blame: code cleanup patches end up burying the important changes so\nit becomes cumbersome to find them using git blame.\n\nConsidering the added value brought by splitting cleanups from functional changes\nfrom a maintainer perspective (easier reverts) and from a reviewer perspective\n(easier to focus on the functional changes), I think it would be good to improve\nthe git tooling to allow easily filtering out the noise from git blame.\n\nPerhaps a new git blame \"--ignore-trivial\" and/or \"--ignore-cleanup\" could solve\nthis by filtering out \"trivial\" and \"cleanup\" patches from the history it considers.\n\nTagging patches as trivial and cleanup should be done in the patch commit message\n(possibly in the title), and enforcing proper tagging of commits is already the\nresponsibility of the maintainer merging those cleanup/trivial commits into the\nLinux kernel anyway.\n\nUnder the hood, I suspect it could use something similar to git log --grep=<pattern>\n--invert-grep.\n\nThis should allow git blame users to easily filter out the noise and focus on the relevant\nfunctional changes.\n\nAny maybe the patterns associated to \"cleanup\" and \"trivial\" commits should be something\nthat can be configured through a git config file.\n\nThoughts ?\n\nThanks,\n\nMathieu\n\n-- \nMathieu Desnoyers\nEfficiOS Inc.\nhttp://www.efficios.com\n"},{"id":"426239","messageId":"YLej6F24Emm7SX35@zeniv-ca.linux.org.uk","threadId":"55831","inReplyTo":"30399052.5964.1622647235870.JavaMail.zimbra@efficios.com","subject":"Re: git feature request: git blame --ignore-cleanup/--ignore-trivial","fromName":"Al Viro","fromEmail":"viro@zeniv.linux.org.uk","sentAt":"2021-06-02T15:29:44Z","receivedAt":"2021-06-02T15:29:52Z","isPatch":false,"sender":{"key":"viro@zeniv.linux.org.uk","avatar":null},"body":"On Wed, Jun 02, 2021 at 11:20:35AM -0400, Mathieu Desnoyers wrote:\n> Hi,\n> \n> Following a discussion with Peter Zijlstra about whether code cleanup\n> and functional changes done to the Linux kernel scheduler belong to separate\n> patches or should be folded together, the argument for folding cleanup\n> and function changes came to be mainly motivated by the current behavior\n> of git blame: code cleanup patches end up burying the important changes so\n> it becomes cumbersome to find them using git blame.\n> \n> Considering the added value brought by splitting cleanups from functional changes\n> from a maintainer perspective (easier reverts) and from a reviewer perspective\n> (easier to focus on the functional changes), I think it would be good to improve\n> the git tooling to allow easily filtering out the noise from git blame.\n> \n> Perhaps a new git blame \"--ignore-trivial\" and/or \"--ignore-cleanup\" could solve\n> this by filtering out \"trivial\" and \"cleanup\" patches from the history it considers.\n> \n> Tagging patches as trivial and cleanup should be done in the patch commit message\n> (possibly in the title), and enforcing proper tagging of commits is already the\n> responsibility of the maintainer merging those cleanup/trivial commits into the\n> Linux kernel anyway.\n> \n> Under the hood, I suspect it could use something similar to git log --grep=<pattern>\n> --invert-grep.\n> \n> This should allow git blame users to easily filter out the noise and focus on the relevant\n> functional changes.\n> \n> Any maybe the patterns associated to \"cleanup\" and \"trivial\" commits should be something\n> that can be configured through a git config file.\n> \n> Thoughts ?\n\nJust an observation: quite a few subtle bugs arise from mistakes in\nwhat should've been a trivial cleanup.  Hell, I've seen bugs coming\nfrom rebase of provably no-op patches - with commit message unchanged.\nSo IME this is counterproductive...\n"},{"id":"426249","messageId":"YLfd5pWtRqde1++Y@coredump.intra.peff.net","threadId":"55831","inReplyTo":"30399052.5964.1622647235870.JavaMail.zimbra@efficios.com","subject":"Re: git feature request: git blame --ignore-cleanup/--ignore-trivial","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-06-02T19:37:10Z","receivedAt":"2021-06-02T19:37:13Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Jun 02, 2021 at 11:20:35AM -0400, Mathieu Desnoyers wrote:\n\n> Considering the added value brought by splitting cleanups from functional changes\n> from a maintainer perspective (easier reverts) and from a reviewer perspective\n> (easier to focus on the functional changes), I think it would be good to improve\n> the git tooling to allow easily filtering out the noise from git blame.\n> \n> Perhaps a new git blame \"--ignore-trivial\" and/or \"--ignore-cleanup\" could solve\n> this by filtering out \"trivial\" and \"cleanup\" patches from the history it considers.\n\nThere's \"-w\" to ignore whitespace-only changes. Since Git v2.23.0,\nthere's also \"--ignore-rev-file\", which lets you ignore arbitrary\ncommits. Since you have to generate a separate list of commits to feed\nit, it's a little more involved than your \"--invert-grep\" example. But\nit's also much more flexible (you can generate the file however you\nlike, and even tweak it by hand).\n\nI do tend to agree with Al's notion that commits which are _supposed_ to\nbe trivial sometimes end up not being so. Or another way of thinking\nabout it is: relevance is a property of the query you're making, not the\noriginal change.\n\nSo IMHO the best tool for this kind of thing is the \"re-blame from\nparent\" feature that many interactive blame viewers have (I use tig, but\nI'm sure other tools like magic have a similar feature). There when you\nland on a boring commit, it's a single key to skip past it and see how\nthe earlier code came about.\n\n-Peff\n"},{"id":"426250","messageId":"YLfe+HXl4hkzs44b@nand.local","threadId":"55831","inReplyTo":"YLej6F24Emm7SX35@zeniv-ca.linux.org.uk","subject":"Re: git feature request: git blame --ignore-cleanup/--ignore-trivial","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2021-06-02T19:41:44Z","receivedAt":"2021-06-02T19:42:04Z","isPatch":false,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Wed, Jun 02, 2021 at 03:29:44PM +0000, Al Viro wrote:\n> > Any maybe the patterns associated to \"cleanup\" and \"trivial\" commits\n> > should be something that can be configured through a git config\n> > file.\n>\n> Just an observation: quite a few subtle bugs arise from mistakes in\n> what should've been a trivial cleanup.  Hell, I've seen bugs coming\n> from rebase of provably no-op patches - with commit message unchanged.\n> So IME this is counterproductive...\n\nYes, I find excluding revisions from 'git blame' to be rarely useful,\nexactly for this reason.\n\nYou could probably use the '--ignore-revs-file' option of 'git blame' to\nexclude commits you consider trivial ahead of time. If you had an\n'Is-trivial' trailer, I would probably do something like:\n\n  $ git log --format='%H %(trailers:key=Is-trivial)' |\n      grep \"Is-trivial: true\" | cut -d\" \" -f1 >exclude\n  $ git blame --ignore-revs-file exclude ...\n\nThanks,\nTaylor\n"},{"id":"426266","messageId":"60b7f54697fda_16925f2083d@natae.notmuch","threadId":"55831","inReplyTo":"30399052.5964.1622647235870.JavaMail.zimbra@efficios.com","subject":"RE: git feature request: git blame --ignore-cleanup/--ignore-trivial","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-06-02T21:16:54Z","receivedAt":"2021-06-02T21:16:59Z","isPatch":false,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Mathieu Desnoyers wrote:\n> Perhaps a new git blame \"--ignore-trivial\" and/or \"--ignore-cleanup\" could solve\n> this by filtering out \"trivial\" and \"cleanup\" patches from the history it considers.\n\nWhile this feature would be a good addition, more often than not I need\nto look behind the latest commit regardless of whether or not it's a\ntrivial one.\n\nSo what I often end up doing is a `git blame --incremental`:\n\n  git blame --incremental -L100,+6 file.c | grep -o -e '^[0-9a-f]\\{40\\}'\n\nThis can be fed to `git log --stdin --oneline --no-walk` for more\nuser-friendliness.\n\nOf course you could just do:\n\n  git log -L100,+6:file.c --oneline --no-patch\n\nBut for some reason that's much slower on my system.\n\nCheers.\n\n-- \nFelipe Contreras\n"},{"id":"426316","messageId":"20210603101350.GW31483@twin.jikos.cz","threadId":"55831","inReplyTo":"YLej6F24Emm7SX35@zeniv-ca.linux.org.uk","subject":"Re: git feature request: git blame --ignore-cleanup/--ignore-trivial","fromName":"David Sterba","fromEmail":"dsterba@suse.cz","sentAt":"2021-06-03T10:13:50Z","receivedAt":"2021-06-03T10:16:35Z","isPatch":false,"sender":{"key":"dsterba@suse.cz","avatar":null},"body":"On Wed, Jun 02, 2021 at 03:29:44PM +0000, Al Viro wrote:\n> On Wed, Jun 02, 2021 at 11:20:35AM -0400, Mathieu Desnoyers wrote:\n> > Hi,\n> > \n> > Following a discussion with Peter Zijlstra about whether code cleanup\n> > and functional changes done to the Linux kernel scheduler belong to separate\n> > patches or should be folded together, the argument for folding cleanup\n> > and function changes came to be mainly motivated by the current behavior\n> > of git blame: code cleanup patches end up burying the important changes so\n> > it becomes cumbersome to find them using git blame.\n> > \n> > Considering the added value brought by splitting cleanups from functional changes\n> > from a maintainer perspective (easier reverts) and from a reviewer perspective\n> > (easier to focus on the functional changes), I think it would be good to improve\n> > the git tooling to allow easily filtering out the noise from git blame.\n> > \n> > Perhaps a new git blame \"--ignore-trivial\" and/or \"--ignore-cleanup\" could solve\n> > this by filtering out \"trivial\" and \"cleanup\" patches from the history it considers.\n> > \n> > Tagging patches as trivial and cleanup should be done in the patch commit message\n> > (possibly in the title), and enforcing proper tagging of commits is already the\n> > responsibility of the maintainer merging those cleanup/trivial commits into the\n> > Linux kernel anyway.\n> > \n> > Under the hood, I suspect it could use something similar to git log --grep=<pattern>\n> > --invert-grep.\n> > \n> > This should allow git blame users to easily filter out the noise and focus on the relevant\n> > functional changes.\n> > \n> > Any maybe the patterns associated to \"cleanup\" and \"trivial\" commits should be something\n> > that can be configured through a git config file.\n> > \n> > Thoughts ?\n> \n> Just an observation: quite a few subtle bugs arise from mistakes in\n> what should've been a trivial cleanup.\n\nI was about to write such comment. Cleanups that are eg. mechanically\nswitching names/types/variables/... and need some manual fixup are hot\ncandidates for buggy patches.\n"},{"id":"426317","messageId":"20210603102303.GX31483@twin.jikos.cz","threadId":"55831","inReplyTo":"30399052.5964.1622647235870.JavaMail.zimbra@efficios.com","subject":"Re: git feature request: git blame --ignore-cleanup/--ignore-trivial","fromName":"David Sterba","fromEmail":"dsterba@suse.cz","sentAt":"2021-06-03T10:23:03Z","receivedAt":"2021-06-03T10:25:46Z","isPatch":false,"sender":{"key":"dsterba@suse.cz","avatar":null},"body":"On Wed, Jun 02, 2021 at 11:20:35AM -0400, Mathieu Desnoyers wrote:\n> Following a discussion with Peter Zijlstra about whether code cleanup\n> and functional changes done to the Linux kernel scheduler belong to separate\n> patches or should be folded together, the argument for folding cleanup\n> and function changes came to be mainly motivated by the current behavior\n> of git blame: code cleanup patches end up burying the important changes so\n> it becomes cumbersome to find them using git blame.\n> \n> Considering the added value brought by splitting cleanups from functional changes\n> from a maintainer perspective (easier reverts) and from a reviewer perspective\n> (easier to focus on the functional changes), I think it would be good to improve\n> the git tooling to allow easily filtering out the noise from git blame.\n> \n> Perhaps a new git blame \"--ignore-trivial\" and/or \"--ignore-cleanup\" could solve\n> this by filtering out \"trivial\" and \"cleanup\" patches from the history it considers.\n> \n> Tagging patches as trivial and cleanup should be done in the patch commit message\n> (possibly in the title), and enforcing proper tagging of commits is already the\n> responsibility of the maintainer merging those cleanup/trivial commits into the\n> Linux kernel anyway.\n> \n> Under the hood, I suspect it could use something similar to git log --grep=<pattern>\n> --invert-grep.\n> \n> This should allow git blame users to easily filter out the noise and focus on the relevant\n> functional changes.\n> \n> Any maybe the patterns associated to \"cleanup\" and \"trivial\" commits should be something\n> that can be configured through a git config file.\n\nAs long as the subsystem policy is consistent, eg. always split cleanups\nfrom functional changes, and make the trivial cleanups really obvious\neither from code or mentioned in the changelog, I don't see a need for\na tooling.\n\nGoing through unrelated cleanups when digging in the git history is\nannoying and I think it's up to maintainers' and developers' decision\nwhat kind of cleanups are desired (because they make the code better,\nnot just because they're trivial).  Mandating some sort of tagging is\njust another burden, if it's not applied consistently it won't be\nreliable so it won't help much.\n"},{"id":"426329","messageId":"654904857.6915.1622734437354.JavaMail.zimbra@efficios.com","threadId":"55831","inReplyTo":"YLfe+HXl4hkzs44b@nand.local","subject":"Re: git feature request: git blame --ignore-cleanup/--ignore-trivial","fromName":"Mathieu Desnoyers","fromEmail":"mathieu.desnoyers@efficios.com","sentAt":"2021-06-03T15:33:57Z","receivedAt":"2021-06-03T15:33:59Z","isPatch":false,"sender":{"key":"mathieu.desnoyers@efficios.com","avatar":"https://gravatar.com/avatar/c3872f90e251e72cba6a65443eb6a4144e73ab1b2e08149cf78eb5d4779a88dd?d=mp&s=160"},"body":"----- On Jun 2, 2021, at 3:41 PM, Taylor Blau me@ttaylorr.com wrote:\n\n> On Wed, Jun 02, 2021 at 03:29:44PM +0000, Al Viro wrote:\n>> > Any maybe the patterns associated to \"cleanup\" and \"trivial\" commits\n>> > should be something that can be configured through a git config\n>> > file.\n>>\n>> Just an observation: quite a few subtle bugs arise from mistakes in\n>> what should've been a trivial cleanup.  Hell, I've seen bugs coming\n>> from rebase of provably no-op patches - with commit message unchanged.\n>> So IME this is counterproductive...\n> \n> Yes, I find excluding revisions from 'git blame' to be rarely useful,\n> exactly for this reason.\n> \n> You could probably use the '--ignore-revs-file' option of 'git blame' to\n> exclude commits you consider trivial ahead of time. If you had an\n> 'Is-trivial' trailer, I would probably do something like:\n> \n>  $ git log --format='%H %(trailers:key=Is-trivial)' |\n>      grep \"Is-trivial: true\" | cut -d\" \" -f1 >exclude\n>  $ git blame --ignore-revs-file exclude ...\n\nNice trick! So within a project which standardize on a \"Cleanup: \" prefix\nat the beginning of the patch subject, this would look like:\n\ngit log --format='%H Subject=(\"%s\")' file.c | grep 'Subject=(\\\"Cleanup: ' | cut -d\" \" -f1 > exclude.txt\ngit blame --ignore-revs-file exclude.txt file.c\n\nI fully understand that in many cases having the entire set of revisions is\nneeded, because even a cleanup patch could be buggy, but IMHO it's nice to\nhave a way to achieve this in situations where the cleanup patches get in the\nway of figuring out the most recent behavior changes in a given area of the\ncode.\n\nThanks,\n\nMathieu\n\n-- \nMathieu Desnoyers\nEfficiOS Inc.\nhttp://www.efficios.com\n"}]}