{"thread":{"id":"45370","subject":"Possible git blame bug?","startedAt":"2017-03-13T20:11:13Z","lastAt":"2017-03-13T23:19:45Z","messageCount":10,"participants":["Domagoj Stolfa","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"313947","messageId":"20170313201104.GA32821@workstation","threadId":"45370","inReplyTo":null,"subject":"Possible git blame bug?","fromName":"Domagoj Stolfa","fromEmail":"domagoj.stolfa@gmail.com","sentAt":"2017-03-13T20:11:04Z","receivedAt":"2017-03-13T20:11:13Z","isPatch":false,"sender":{"key":"domagoj.stolfa@gmail.com","avatar":null},"body":"Hello,\n\nyesterday I came across sort of a weird behaviour with git-blame. It would\nappear when one queries the git blame on a specific file, such as:\n\n$ git blame --since=\n\nit will blame the entire file on some commit of that year, regardless of the\nfact whether the commit has actually touched that file or not.\n\nThis seems consistent across all the tested systems: FreeBSD, macOS, Ubuntu and\nFedora.\n\nAn example of the blame can be seen:\n\n$ git blame tcp_output.c\ncd0bc69e2fdd (wollman 1995-09-22 20:05:58 +0000 29) * @(#)tcp_output.c 8.4 (Berkeley) 5/24/95\n\ncompared to:\n\n$ git blame tcp_output.c --since=2017\n^e19f2a27ed8 (Domagoj Stolfa 2017-03-12 20:43:01 +0100 29) * @(#)tcp_output.c 8.4 (Berkeley) 5/24/95\n\n$ git blame tcp_output.c --since=2016\n^e4bdb83e76c (jceel 2016-03-13 19:50:17 +0000 29) * @(#)tcp_output.c 8.4 (Berkeley) 5/24/95\n\n$ git blame tcp_output.c --since=2015\n^d749a6e6c70 (pfg 2015-03-13 18:42:43 +0000 29) * @(#)tcp_output.c 8.4 (Berkeley) 5/24/95\n\nOf course, specifiying\n\n$ git blame --since=1.1.2017\n\ngives correct results, as expected.\n\nThe question is whether this is a bug or not, as --since= might not be a\nvalid filter. However, this might surprise a lot of users and mislead\ndevelopment. I would personally like to see this behaviour changed to either a\nblank report, a report of that year(making it a valid filter), but certainly not\nblaming it on commits that have never touched that file.\n\n-- \nBest regards,\nDomagoj Stolfa\n"},{"id":"313960","messageId":"xmqqmvcorjyo.fsf@gitster.mtv.corp.google.com","threadId":"45370","inReplyTo":"20170313201104.GA32821@workstation","subject":"Re: Possible git blame bug?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-03-13T20:38:07Z","receivedAt":"2017-03-13T20:38:14Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Domagoj Stolfa writes:\n\n> The question is whether this is a bug or not, as --since= might not be a\n> valid filter.\n\nI do not think blame ever was designed to work with --since, so that\nis indeed the case. \n\nMaking it work with --since= might be a worthy addition.\nPatches welcome.\n\n"},{"id":"313964","messageId":"20170313204614.GA66050@workstation","threadId":"45370","inReplyTo":"20170313204401.GB80633@workstation","subject":"Re: Possible git blame bug?","fromName":"Domagoj Stolfa","fromEmail":"domagoj.stolfa@gmail.com","sentAt":"2017-03-13T20:46:14Z","receivedAt":"2017-03-13T20:46:28Z","isPatch":false,"sender":{"key":"domagoj.stolfa@gmail.com","avatar":null},"body":"Hello,\n \n> > The question is whether this is a bug or not, as --since= might not be a\n> > valid filter.\n> \n> I do not think blame ever was designed to work with --since, so that\n> is indeed the case. \n> \n> Making it work with --since= might be a worthy addition.\n> Patches welcome.\n \nThanks for the information. I wasn't aware that it wasn't designed with that in\nmind, though it would indicate to me that --since seems to work with git-blame\naccording to [1], but expects a date, as I have usually been using it. It also\nseems to produce correct results on FreeBSD 12.0-CURRENT with git 2.11.1, except\nwhen given a filter such as --since=, in which case perhaps nothing should\nbe displayed?\n \nCould you please clarify which bits wouldn't work with --since in git-blame?\n \n[1] https://www.git-scm.com/docs/git-blame\n \n-- \nBest regards,\nDomagoj Stolfa\n"},{"id":"313972","messageId":"CAPc5daVRX_-bJ_2reDLKJin9PNfy6EjbD14T=Mpx=9P8xzdmuw@mail.gmail.com","threadId":"45370","inReplyTo":"xmqqmvcorjyo.fsf@gitster.mtv.corp.google.com","subject":"Re: Possible git blame bug?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-03-13T21:29:28Z","receivedAt":"2017-03-13T21:29:56Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"On Mon, Mar 13, 2017 at 1:38 PM, Junio C Hamano wrote:\n> Domagoj Stolfa writes:\n>\n>> The question is whether this is a bug or not, as --since= might not be a\n>> valid filter.\n>\n> I do not think blame ever was designed to work with --since, so that\n> is indeed the case.\n\nActually, I do see that we had a cut-off based on rev->max_age since we\nintroduced it at cee7f245 (\"git-pickaxe: blame rewritten.\", 2006-10-19).\n\nI do not know offhand if --since=2000 _means_ --since=2000-01-01 or something\ncompletely different, though.\n"},{"id":"313987","messageId":"20170313214400.GA98717@workstation","threadId":"45370","inReplyTo":"CAPc5daVRX_-bJ_2reDLKJin9PNfy6EjbD14T=Mpx=9P8xzdmuw@mail.gmail.com","subject":"Re: Possible git blame bug?","fromName":"Domagoj Stolfa","fromEmail":"domagoj.stolfa@gmail.com","sentAt":"2017-03-13T21:44:00Z","receivedAt":"2017-03-13T21:44:27Z","isPatch":false,"sender":{"key":"domagoj.stolfa@gmail.com","avatar":null},"body":"Hello,\n\n> >> The question is whether this is a bug or not, as --since= might not be a\n> >> valid filter.\n> >\n> > I do not think blame ever was designed to work with --since, so that\n> > is indeed the case.\n> \n> Actually, I do see that we had a cut-off based on rev->max_age since we\n> introduced it at cee7f245 (\"git-pickaxe: blame rewritten.\", 2006-10-19).\n> \n> I do not know offhand if --since=2000 _means_ --since=2000-01-01 or something\n> completely different, though.\n\nIt seems to indicate something entirely different. The problem with it is that\nit mentions commits that haven't even touched the file though. Output with\ncommit hashes that have touched that file would be sensible, albeit wrong in the\nsense that the user did not want to see that behaviour.\n\nFor example, saying:\n\n$ git blame time.h --since=2017\n^e19f2a27ed8 (Domagoj Stolfa 2017-03-12 20:43:01 +0100 33) #ifndef _SYS_TIME_H_\n\n$ git blame time.h --since=2016\n^21613a57af9 (bz 2016-03-13 21:26:18 +0000 33) #ifndef _SYS_TIME_H_\n\n$ git blame time.h --since=2015\n^48507f436f0 (mav 2015-03-13 21:01:25 +0000 33) #ifndef _SYS_TIME_H_\n\nand so on, with different hashes.\n\nLooking at these commits:\n\n(1) https://github.com/dstolfa/freebsd/commit/e19f2a27ed82f616d47dd4e0dc75722139e72957\n(2) https://github.com/dstolfa/freebsd/commit/21613a57af9500acca9b3528958312dd3ae12bb4\n(3) https://github.com/dstolfa/freebsd/commit/48507f436f07a9515c6d7108509a50dd4fd403b2\n\nneither of them seems to touch time.h, yet git blame reports them to do so.\nInterestingly enough, it seems to be taking the commit that is the closest to\nthe current date in that year, and blaming it on that one, regardless of what it\nactually did and the file specified.\n\n-- \nBest regards,\nDomagoj Stolfa\n"},{"id":"313994","messageId":"xmqqfuign7jw.fsf@gitster.mtv.corp.google.com","threadId":"45370","inReplyTo":"20170313214400.GA98717@workstation","subject":"Re: Possible git blame bug?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-03-13T22:19:47Z","receivedAt":"2017-03-13T22:19:56Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Domagoj Stolfa writes:\n\n> For example, saying:\n>\n> $ git blame time.h --since=2017\n> ^e19f2a27ed8 (Domagoj Stolfa 2017-03-12 20:43:01 +0100 33) #ifndef _SYS_TIME_H_\n>\n> $ git blame time.h --since=2016\n> ^21613a57af9 (bz 2016-03-13 21:26:18 +0000 33) #ifndef _SYS_TIME_H_\n>\n> $ git blame time.h --since=2015\n> ^48507f436f0 (mav 2015-03-13 21:01:25 +0000 33) #ifndef _SYS_TIME_H_\n>\n> and so on, with different hashes.\n\nThe output lines \"^deadbeef\" does *NOT* mean that commit deadbeef\nchanged the revision. It just is telling you that the hisory was\ndug down to that revision and it was found that since that revision\nthere is no change (and you told the command not to bother looking\nbeyond that time range, so we do not know what happened before that\ntime).\n\nIt is understandable, when your history has a lot of merges, the\nhistory traversal may stop at commits on different branches.\n\nImagine a case where the line in question never changed throughout\nthe history:\n\n o---o---B\n / \\\n O---o---o---A---C---o---o\n\nImagine A is from 2015, B is from 2016 and C is from 2017. C's\nfirst parent, i.e. C^1, is A and C^2 is B.\n\nIf you ask the command to stop digging when you hit a commit on or\nbefore 2017-03-13 (03-13 is because today's date is appended to your\n2017), your traversal will stop at C and you get a line that begins\nwith ^C.\n\nIf you ask it to stop at 2016, A won't be even looked at because it\nis older. The command will keep digging from C to find B. If B's\nparent is also newer than the cutoff, but its parent is older, then\nthe line will be shown with ^ and commit object name of B's parent.\n\nIf you ask it to stop at 2015, the command will first consider A\n(C's earlier parent) and pass blame to the lines common between\nthese two commits. In this illustration, we are pretending that the\nfile did not change throughout the hsitory, so blame for all lines\nare passed to A and we don't even look at B. Then we keep digging\nthrough A to find the culprit, or hit a commit older than the\nspecified cut-off time. The line will be shown with ^A or perhaps\nits ancestor.\n\nSo it is entirely sane if you saw three boundary commits named with\nthree different time ranges.\n\n"},{"id":"314001","messageId":"xmqqy3w8lrr2.fsf@gitster.mtv.corp.google.com","threadId":"45370","inReplyTo":"xmqqfuign7jw.fsf@gitster.mtv.corp.google.com","subject":"Re: Possible git blame bug?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-03-13T22:46:25Z","receivedAt":"2017-03-13T22:46:52Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Domagoj Stolfa writes:\n>\n>> For example, saying:\n>>\n>> $ git blame time.h --since=2017\n>> ^e19f2a27ed8 (Domagoj Stolfa 2017-03-12 20:43:01 +0100 33) #ifndef _SYS_TIME_H_\n>>\n>> $ git blame time.h --since=2016\n>> ^21613a57af9 (bz 2016-03-13 21:26:18 +0000 33) #ifndef _SYS_TIME_H_\n>>\n>> $ git blame time.h --since=2015\n>> ^48507f436f0 (mav 2015-03-13 21:01:25 +0000 33) #ifndef _SYS_TIME_H_\n>>\n>> and so on, with different hashes.\n>\n> The output lines \"^deadbeef\" does *NOT* mean that commit deadbeef\n> changed the revision. It just is telling you that the hisory was\n> dug down to that revision and it was found that since that revision\n> there is no change (and you told the command not to bother looking\n> beyond that time range, so we do not know what happened before that\n> time).\n> ...\n> So it is entirely sane if you saw three boundary commits named with\n> three different time ranges.\n\nSide note. If the displaying of the boundary commit object names\nare distracting, the user can always use the -b option to blank them\nout.\n"},{"id":"314006","messageId":"20170313230810.GA80865@workstation","threadId":"45370","inReplyTo":"xmqqfuign7jw.fsf@gitster.mtv.corp.google.com","subject":"Re: Possible git blame bug?","fromName":"Domagoj Stolfa","fromEmail":"domagoj.stolfa@gmail.com","sentAt":"2017-03-13T23:08:10Z","receivedAt":"2017-03-13T23:08:19Z","isPatch":false,"sender":{"key":"domagoj.stolfa@gmail.com","avatar":null},"body":"Hello,\n\n> > For example, saying:\n> >\n> > $ git blame time.h --since=2017\n> > ^e19f2a27ed8 (Domagoj Stolfa 2017-03-12 20:43:01 +0100 33) #ifndef _SYS_TIME_H_\n> >\n> > $ git blame time.h --since=2016\n> > ^21613a57af9 (bz 2016-03-13 21:26:18 +0000 33) #ifndef _SYS_TIME_H_\n> >\n> > $ git blame time.h --since=2015\n> > ^48507f436f0 (mav 2015-03-13 21:01:25 +0000 33) #ifndef _SYS_TIME_H_\n> >\n> > and so on, with different hashes.\n> \n> The output lines \"^deadbeef\" does *NOT* mean that commit deadbeef\n> changed the revision. It just is telling you that the hisory was\n> dug down to that revision and it was found that since that revision\n> there is no change (and you told the command not to bother looking\n> beyond that time range, so we do not know what happened before that\n> time).\n> \n> It is understandable, when your history has a lot of merges, the\n> history traversal may stop at commits on different branches.\n> \n> Imagine a case where the line in question never changed throughout\n> the history:\n> \n> o---o---B\n> / \\\n> O---o---o---A---C---o---o\n> \n> Imagine A is from 2015, B is from 2016 and C is from 2017. C's\n> first parent, i.e. C^1, is A and C^2 is B.\n> \n> If you ask the command to stop digging when you hit a commit on or\n> before 2017-03-13 (03-13 is because today's date is appended to your\n> 2017), your traversal will stop at C and you get a line that begins\n> with ^C.\n> \n> If you ask it to stop at 2016, A won't be even looked at because it\n> is older. The command will keep digging from C to find B. If B's\n> parent is also newer than the cutoff, but its parent is older, then\n> the line will be shown with ^ and commit object name of B's parent.\n> \n> If you ask it to stop at 2015, the command will first consider A\n> (C's earlier parent) and pass blame to the lines common between\n> these two commits. In this illustration, we are pretending that the\n> file did not change throughout the hsitory, so blame for all lines\n> are passed to A and we don't even look at B. Then we keep digging\n> through A to find the culprit, or hit a commit older than the\n> specified cut-off time. The line will be shown with ^A or perhaps\n> its ancestor.\n> \n> So it is entirely sane if you saw three boundary commits named with\n> three different time ranges.\n\nThanks for clearing this up. Is this documented somewhere, so that if it happens\nagain I can point people to the docs that explain this behaviour?\n\n-- \nBest regards,\nDomagoj Stolfa\n"},{"id":"314007","messageId":"xmqqtw6wlqf8.fsf@gitster.mtv.corp.google.com","threadId":"45370","inReplyTo":"20170313230810.GA80865@workstation","subject":"Re: Possible git blame bug?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-03-13T23:15:07Z","receivedAt":"2017-03-13T23:15:26Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Domagoj Stolfa writes:\n\n> Thanks for clearing this up. Is this documented somewhere, so that if it happens\n> again I can point people to the docs that explain this behaviour?\n\nIs this from \"git blame --help\" sufficient?\n\n When you are not interested in changes older than version\n v2.6.18, or changes older than 3 weeks, you can use revision\n range specifiers similar to 'git rev-list':\n\n git blame v2.6.18.. -- foo\n git blame --since=3.weeks -- foo\n\n When revision range specifiers are used to limit the annotation,\n lines that have not changed since the range boundary (either the\n commit v2.6.18 or the most recent commit that is more than 3\n weeks old in the above example) are blamed for that range\n boundary commit.\n\n"},{"id":"314009","messageId":"20170313231935.GA51164@workstation","threadId":"45370","inReplyTo":"xmqqtw6wlqf8.fsf@gitster.mtv.corp.google.com","subject":"Re: Possible git blame bug?","fromName":"Domagoj Stolfa","fromEmail":"domagoj.stolfa@gmail.com","sentAt":"2017-03-13T23:19:35Z","receivedAt":"2017-03-13T23:19:45Z","isPatch":false,"sender":{"key":"domagoj.stolfa@gmail.com","avatar":null},"body":"Hello,\n\n> > Thanks for clearing this up. Is this documented somewhere, so that if it happens\n> > again I can point people to the docs that explain this behaviour?\n> \n> Is this from \"git blame --help\" sufficient?\n> \n> When you are not interested in changes older than version\n> v2.6.18, or changes older than 3 weeks, you can use revision\n> range specifiers similar to 'git rev-list':\n> \n> git blame v2.6.18.. -- foo\n> git blame --since=3.weeks -- foo\n> \n> When revision range specifiers are used to limit the annotation,\n> lines that have not changed since the range boundary (either the\n> commit v2.6.18 or the most recent commit that is more than 3\n> weeks old in the above example) are blamed for that range\n> boundary commit.\n\nre-reading it post your explanation, it seems to make perfect sense. Thanks\nagain for the detailed explanation of the behaviour!\n\n-- \nBest regards,\nDomagoj Stolfa\n"}]}