Re: Possible git blame bug?

2017-03-13 Thread Domagoj Stolfa
Hello,

> > Thanks for clearing this up. Is this documented somewhere, so that if it 
> > happens
> > again I can point people to the docs that explain this behaviour?
> 
> Is this from "git blame --help" sufficient?
> 
> When you are not interested in changes older than version
> v2.6.18, or changes older than 3 weeks, you can use revision
> range specifiers  similar to 'git rev-list':
> 
> git blame v2.6.18.. -- foo
> git blame --since=3.weeks -- foo
> 
> When revision range specifiers are used to limit the annotation,
> lines that have not changed since the range boundary (either the
> commit v2.6.18 or the most recent commit that is more than 3
> weeks old in the above example) are blamed for that range
> boundary commit.

re-reading it post your explanation, it seems to make perfect sense. Thanks
again for the detailed explanation of the behaviour!

-- 
Best regards,
Domagoj Stolfa


signature.asc
Description: PGP signature


Re: Possible git blame bug?

2017-03-13 Thread Junio C Hamano
Domagoj Stolfa  writes:

> Thanks for clearing this up. Is this documented somewhere, so that if it 
> happens
> again I can point people to the docs that explain this behaviour?

Is this from "git blame --help" sufficient?

When you are not interested in changes older than version
v2.6.18, or changes older than 3 weeks, you can use revision
range specifiers  similar to 'git rev-list':

git blame v2.6.18.. -- foo
git blame --since=3.weeks -- foo

When revision range specifiers are used to limit the annotation,
lines that have not changed since the range boundary (either the
commit v2.6.18 or the most recent commit that is more than 3
weeks old in the above example) are blamed for that range
boundary commit.



Re: Possible git blame bug?

2017-03-13 Thread Domagoj Stolfa
Hello,

> > For example, saying:
> >
> > $ git blame time.h --since=2017
> > ^e19f2a27ed8 (Domagoj Stolfa 2017-03-12 20:43:01 +0100  33) #ifndef 
> > _SYS_TIME_H_
> >
> > $ git blame time.h --since=2016
> > ^21613a57af9 (bz  2016-03-13 21:26:18 +  33) #ifndef _SYS_TIME_H_
> >
> > $ git blame time.h --since=2015
> > ^48507f436f0 (mav 2015-03-13 21:01:25 +  33) #ifndef _SYS_TIME_H_
> >
> > and so on, with different hashes.
> 
> The output lines "^deadbeef" does *NOT* mean that commit deadbeef
> changed the revision.  It just is telling you that the hisory was
> dug down to that revision and it was found that since that revision
> there is no change (and you told the command not to bother looking
> beyond that time range, so we do not know what happened before that
> time).
> 
> It is understandable, when your history has a lot of merges, the
> history traversal may stop at commits on different branches.
> 
> Imagine a case where the line in question never changed throughout
> the history:
> 
>   o---o---B
>  / \
> O---o---o---A---C---o---o
> 
> Imagine A is from 2015, B is from 2016 and C is from 2017.  C's
> first parent, i.e. C^1, is A and C^2 is B.
> 
> If you ask the command to stop digging when you hit a commit on or
> before 2017-03-13 (03-13 is because today's date is appended to your
> 2017), your traversal will stop at C and you get a line that begins
> with ^C.
> 
> If you ask it to stop at 2016, A won't be even looked at because it
> is older.  The command will keep digging from C to find B.  If B's
> parent is also newer than the cutoff, but its parent is older, then
> the line will be shown with ^ and commit object name of B's parent.
> 
> If you ask it to stop at 2015, the command will first consider A
> (C's earlier parent) and pass blame to the lines common between
> these two commits.  In this illustration, we are pretending that the
> file did not change throughout the hsitory, so blame for all lines
> are passed to A and we don't even look at B.  Then we keep digging
> through A to find the culprit, or hit a commit older than the
> specified cut-off time.  The line will be shown with ^A or perhaps
> its ancestor.
> 
> So it is entirely sane if you saw three boundary commits named with
> three different time ranges.

Thanks for clearing this up. Is this documented somewhere, so that if it happens
again I can point people to the docs that explain this behaviour?

-- 
Best regards,
Domagoj Stolfa


signature.asc
Description: PGP signature


Re: Possible git blame bug?

2017-03-13 Thread Junio C Hamano
Junio C Hamano  writes:

> Domagoj Stolfa  writes:
>
>> For example, saying:
>>
>> $ git blame time.h --since=2017
>> ^e19f2a27ed8 (Domagoj Stolfa 2017-03-12 20:43:01 +0100  33) #ifndef 
>> _SYS_TIME_H_
>>
>> $ git blame time.h --since=2016
>> ^21613a57af9 (bz  2016-03-13 21:26:18 +  33) #ifndef _SYS_TIME_H_
>>
>> $ git blame time.h --since=2015
>> ^48507f436f0 (mav 2015-03-13 21:01:25 +  33) #ifndef _SYS_TIME_H_
>>
>> and so on, with different hashes.
>
> The output lines "^deadbeef" does *NOT* mean that commit deadbeef
> changed the revision.  It just is telling you that the hisory was
> dug down to that revision and it was found that since that revision
> there is no change (and you told the command not to bother looking
> beyond that time range, so we do not know what happened before that
> time).
> ...
> So it is entirely sane if you saw three boundary commits named with
> three different time ranges.

Side note.  If the displaying of the boundary commit object names
are distracting, the user can always use the -b option to blank them
out.


Re: Possible git blame bug?

2017-03-13 Thread Junio C Hamano
Domagoj Stolfa  writes:

> For example, saying:
>
> $ git blame time.h --since=2017
> ^e19f2a27ed8 (Domagoj Stolfa 2017-03-12 20:43:01 +0100  33) #ifndef 
> _SYS_TIME_H_
>
> $ git blame time.h --since=2016
> ^21613a57af9 (bz  2016-03-13 21:26:18 +  33) #ifndef _SYS_TIME_H_
>
> $ git blame time.h --since=2015
> ^48507f436f0 (mav 2015-03-13 21:01:25 +  33) #ifndef _SYS_TIME_H_
>
> and so on, with different hashes.

The output lines "^deadbeef" does *NOT* mean that commit deadbeef
changed the revision.  It just is telling you that the hisory was
dug down to that revision and it was found that since that revision
there is no change (and you told the command not to bother looking
beyond that time range, so we do not know what happened before that
time).

It is understandable, when your history has a lot of merges, the
history traversal may stop at commits on different branches.

Imagine a case where the line in question never changed throughout
the history:

  o---o---B
 / \
O---o---o---A---C---o---o

Imagine A is from 2015, B is from 2016 and C is from 2017.  C's
first parent, i.e. C^1, is A and C^2 is B.

If you ask the command to stop digging when you hit a commit on or
before 2017-03-13 (03-13 is because today's date is appended to your
2017), your traversal will stop at C and you get a line that begins
with ^C.

If you ask it to stop at 2016, A won't be even looked at because it
is older.  The command will keep digging from C to find B.  If B's
parent is also newer than the cutoff, but its parent is older, then
the line will be shown with ^ and commit object name of B's parent.

If you ask it to stop at 2015, the command will first consider A
(C's earlier parent) and pass blame to the lines common between
these two commits.  In this illustration, we are pretending that the
file did not change throughout the hsitory, so blame for all lines
are passed to A and we don't even look at B.  Then we keep digging
through A to find the culprit, or hit a commit older than the
specified cut-off time.  The line will be shown with ^A or perhaps
its ancestor.

So it is entirely sane if you saw three boundary commits named with
three different time ranges.



Re: Possible git blame bug?

2017-03-13 Thread Domagoj Stolfa
Hello,

> >> The question is whether this is a bug or not, as --since= might not 
> >> be a
> >> valid filter.
> >
> > I do not think blame ever was designed to work with --since, so that
> > is indeed the case.
> 
> Actually, I do see that we had a cut-off based on rev->max_age since we
> introduced it at cee7f245 ("git-pickaxe: blame rewritten.", 2006-10-19).
> 
> I do not know offhand if --since=2000 _means_ --since=2000-01-01 or something
> completely different, though.

It seems to indicate something entirely different. The problem with it is that
it mentions commits that haven't even touched the file though. Output with
commit hashes that have touched that file would be sensible, albeit wrong in the
sense that the user did not want to see that behaviour.

For example, saying:

$ git blame time.h --since=2017
^e19f2a27ed8 (Domagoj Stolfa 2017-03-12 20:43:01 +0100  33) #ifndef _SYS_TIME_H_

$ git blame time.h --since=2016
^21613a57af9 (bz  2016-03-13 21:26:18 +  33) #ifndef _SYS_TIME_H_

$ git blame time.h --since=2015
^48507f436f0 (mav 2015-03-13 21:01:25 +  33) #ifndef _SYS_TIME_H_

and so on, with different hashes.

Looking at these commits:

(1) 
https://github.com/dstolfa/freebsd/commit/e19f2a27ed82f616d47dd4e0dc75722139e72957
(2) 
https://github.com/dstolfa/freebsd/commit/21613a57af9500acca9b3528958312dd3ae12bb4
(3) 
https://github.com/dstolfa/freebsd/commit/48507f436f07a9515c6d7108509a50dd4fd403b2

neither of them seems to touch time.h, yet git blame reports them to do so.
Interestingly enough, it seems to be taking the commit that is the closest to
the current date in that year, and blaming it on that one, regardless of what it
actually did and the file specified.

-- 
Best regards,
Domagoj Stolfa


signature.asc
Description: PGP signature


Re: Possible git blame bug?

2017-03-13 Thread Junio C Hamano
On Mon, Mar 13, 2017 at 1:38 PM, Junio C Hamano  wrote:
> Domagoj Stolfa  writes:
>
>> The question is whether this is a bug or not, as --since= might not be 
>> a
>> valid filter.
>
> I do not think blame ever was designed to work with --since, so that
> is indeed the case.

Actually, I do see that we had a cut-off based on rev->max_age since we
introduced it at cee7f245 ("git-pickaxe: blame rewritten.", 2006-10-19).

I do not know offhand if --since=2000 _means_ --since=2000-01-01 or something
completely different, though.


Re: Possible git blame bug?

2017-03-13 Thread Domagoj Stolfa
Hello,
 
> > The question is whether this is a bug or not, as --since= might not 
> > be a
> > valid filter.
> 
> I do not think blame ever was designed to work with --since, so that
> is indeed the case.  
> 
> Making it work with --since= might be a worthy addition.
> Patches welcome.
 
Thanks for the information. I wasn't aware that it wasn't designed with that in
mind, though it would indicate to me that --since seems to work with git-blame
according to [1], but expects a date, as I have usually been using it. It also
seems to produce correct results on FreeBSD 12.0-CURRENT with git 2.11.1, except
when given a filter such as --since=, in which case perhaps nothing should
be displayed?
 
Could you please clarify which bits wouldn't work with --since in git-blame?
 
[1] https://www.git-scm.com/docs/git-blame
 
-- 
Best regards,
Domagoj Stolfa


signature.asc
Description: PGP signature


Re: Possible git blame bug?

2017-03-13 Thread Junio C Hamano
Domagoj Stolfa  writes:

> The question is whether this is a bug or not, as --since= might not be a
> valid filter.

I do not think blame ever was designed to work with --since, so that
is indeed the case.  

Making it work with --since= might be a worthy addition.
Patches welcome.