Get lastchange.py to work correctly on Git repositories.

Get lastchange.py to work correctly on Git repositories.

This changes lastchange.py in two ways:
1) If the commit it finds is a Git hash, it outputs the whole hash, not just
   the first 7 characters.
2) It only looks at HEAD to see if there is a git-svn id. Previously, it used
   --grep=git-svn-id, which would find the most recent commit containing a
   git-svn id. This would be broken after the switch to git, as it would always
   find the last commit before the switch. Now, it only inspects the most recent
   commit, and falls through to pure-Git if that fails.

R=dilmah@chromium.org, stip@chromium.org
BUG=399113
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=291165

[agable, 2014-08-19 18:44:50]

PTAL. dilmah@ since the code I'm modifying is blamed to you, stip@ because
you're on bug 399113.

[iannucci, 2014-08-19 18:49:46]

https://codereview.chromium.org/488733002/diff/1/build/util/lastchange.py
File build/util/lastchange.py (right):

https://codereview.chromium.org/488733002/diff/1/build/util/lastchange.py#new...
build/util/lastchange.py:106: return VersionInfo('git', output)
should we include the commit position here, if it's available in the commit
message?

[iannucci, 2014-08-19 18:50:22]

On 2014/08/19 18:49:46, iannucci wrote:
> https://codereview.chromium.org/488733002/diff/1/build/util/lastchange.py
> File build/util/lastchange.py (right):
> 
>
https://codereview.chromium.org/488733002/diff/1/build/util/lastchange.py#new...
> build/util/lastchange.py:106: return VersionInfo('git', output)
> should we include the commit position here, if it's available in the commit
> message?

(maybe just all footers which start with `Cr-`?)

[agable, 2014-08-19 22:00:30]

https://codereview.chromium.org/488733002/diff/1/build/util/lastchange.py
File build/util/lastchange.py (right):

https://codereview.chromium.org/488733002/diff/1/build/util/lastchange.py#new...
build/util/lastchange.py:106: return VersionInfo('git', output)
On 2014/08/19 18:49:46, iannucci wrote:
> should we include the commit position here, if it's available in the commit
> message?

The output cannot be multiline. It's unclear to me what would happen if the
output had spaces in it, but given that it is going into a .h header file, I'm
not sure I want to risk it.

[iannucci, 2014-08-19 22:03:30]

On 2014/08/19 22:00:30, agable wrote:
> https://codereview.chromium.org/488733002/diff/1/build/util/lastchange.py
> File build/util/lastchange.py (right):
> 
>
https://codereview.chromium.org/488733002/diff/1/build/util/lastchange.py#new...
> build/util/lastchange.py:106: return VersionInfo('git', output)
> On 2014/08/19 18:49:46, iannucci wrote:
> > should we include the commit position here, if it's available in the commit
> > message?
> 
> The output cannot be multiline. It's unclear to me what would happen if the
> output had spaces in it, but given that it is going into a .h header file, I'm
> not sure I want to risk it.

It could be

deadbeef....

OR

beadfeeb...-refs/heads/master@{#12345}

No?

[agable, 2014-08-20 18:40:47]

On 2014/08/19 22:03:30, iannucci wrote:
> On 2014/08/19 22:00:30, agable wrote:
> > https://codereview.chromium.org/488733002/diff/1/build/util/lastchange.py
> > File build/util/lastchange.py (right):
> > 
> >
>
https://codereview.chromium.org/488733002/diff/1/build/util/lastchange.py#new...
> > build/util/lastchange.py:106: return VersionInfo('git', output)
> > On 2014/08/19 18:49:46, iannucci wrote:
> > > should we include the commit position here, if it's available in the
commit
> > > message?
> > 
> > The output cannot be multiline. It's unclear to me what would happen if the
> > output had spaces in it, but given that it is going into a .h header file,
I'm
> > not sure I want to risk it.
> 
> It could be
> 
> deadbeef....
> 
> OR
> 
> beadfeeb...-refs/heads/master@{#12345}
> 
> No?

+scottmg and +evanm, who have made the most recent and most changes to
lastchange.py, respectively. Do either of you have an opinion on the format? If
not, I'd like to land as-is and get feedback from release managers as to whether
they need more info than just the hash, and how to format that info.

[Evan Martin, 2014-08-20 18:49:01]

First seven chars of the hash is pretty standard git identifier info.
http://stackoverflow.com/questions/7128444/how-does-github-handle-possible-co...

However, from there,
"One of the largest Git projects, the Linux kernel, is beginning to need 12
characters out of the possible 40 to stay unique."

I don't know how the hash is used so I couldn't advise you as to which format is
necessary for your use case.

[scottmg, 2014-08-20 19:42:26]

On 2014/08/20 18:40:47, agable wrote:
> On 2014/08/19 22:03:30, iannucci wrote:
> > On 2014/08/19 22:00:30, agable wrote:
> > > https://codereview.chromium.org/488733002/diff/1/build/util/lastchange.py
> > > File build/util/lastchange.py (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/488733002/diff/1/build/util/lastchange.py#new...
> > > build/util/lastchange.py:106: return VersionInfo('git', output)
> > > On 2014/08/19 18:49:46, iannucci wrote:
> > > > should we include the commit position here, if it's available in the
> commit
> > > > message?
> > > 
> > > The output cannot be multiline. It's unclear to me what would happen if
the
> > > output had spaces in it, but given that it is going into a .h header file,
> I'm
> > > not sure I want to risk it.
> > 
> > It could be
> > 
> > deadbeef....
> > 
> > OR
> > 
> > beadfeeb...-refs/heads/master@{#12345}
> > 
> > No?
> 
> +scottmg and +evanm, who have made the most recent and most changes to
> lastchange.py, respectively. Do either of you have an opinion on the format?
If
> not, I'd like to land as-is and get feedback from release managers as to
whether
> they need more info than just the hash, and how to format that info.

A full hash seems good to me, it's mostly to tag builds so we can look up what a
binary corresponds to. We could always truncate in presentation later if it's
too long/ugly.

AFAIK it's only used in strings like this:
https://code.google.com/p/chromium/codesearch#search/&q=@LASTCHANGE@&sq=packa...
so I think adding the commit number should be fine. Do we have tooling to find
out what shipped/when/where already? If not, adding the commit number seems like
a good idea.

[iannucci, 2014-08-20 20:34:06]

On 2014/08/20 19:42:26, scottmg wrote:
> On 2014/08/20 18:40:47, agable wrote:
> > On 2014/08/19 22:03:30, iannucci wrote:
> > > On 2014/08/19 22:00:30, agable wrote:
> > > >
https://codereview.chromium.org/488733002/diff/1/build/util/lastchange.py
> > > > File build/util/lastchange.py (right):
> > > > 
> > > >
> > >
> >
>
https://codereview.chromium.org/488733002/diff/1/build/util/lastchange.py#new...
> > > > build/util/lastchange.py:106: return VersionInfo('git', output)
> > > > On 2014/08/19 18:49:46, iannucci wrote:
> > > > > should we include the commit position here, if it's available in the
> > commit
> > > > > message?
> > > > 
> > > > The output cannot be multiline. It's unclear to me what would happen if
> the
> > > > output had spaces in it, but given that it is going into a .h header
file,
> > I'm
> > > > not sure I want to risk it.
> > > 
> > > It could be
> > > 
> > > deadbeef....
> > > 
> > > OR
> > > 
> > > beadfeeb...-refs/heads/master@{#12345}
> > > 
> > > No?
> > 
> > +scottmg and +evanm, who have made the most recent and most changes to
> > lastchange.py, respectively. Do either of you have an opinion on the format?
> If
> > not, I'd like to land as-is and get feedback from release managers as to
> whether
> > they need more info than just the hash, and how to format that info.
> 
> A full hash seems good to me, it's mostly to tag builds so we can look up what
a
> binary corresponds to. We could always truncate in presentation later if it's
> too long/ugly.
> 
> AFAIK it's only used in strings like this:
>
https://code.google.com/p/chromium/codesearch#search/&q=@LASTCHANGE@&sq=packa...
> so I think adding the commit number should be fine. Do we have tooling to find
> out what shipped/when/where already? If not, adding the commit number seems
like
> a good idea.

crrev.com/12345 will (soon!) assume you're talking about refs/heads/master on
chromium/src and redirect you to the correctly numbered commit on
chromium.googlesource.com

We may want to add this later, but release branches will have footers that look
like:
  Cr-Commit-Position: refs/branch-heads/1700@{#7}
  Cr-Branched-From: deadbeef....-refs/heads/master@{#12345}

That is: 7th commit on 1700, and the 1700 branch diverged from master at 12345.

I think that this will be valuable information, but maybe I'm wrong. If we can
only have one of them, the Commit-Position is the better one.

[agable, 2014-08-20 21:47:39]

On 2014/08/20 20:34:06, iannucci wrote:
> On 2014/08/20 19:42:26, scottmg wrote:
> > On 2014/08/20 18:40:47, agable wrote:
> > > On 2014/08/19 22:03:30, iannucci wrote:
> > > > On 2014/08/19 22:00:30, agable wrote:
> > > > >
> https://codereview.chromium.org/488733002/diff/1/build/util/lastchange.py
> > > > > File build/util/lastchange.py (right):
> > > > > 
> > > > >
> > > >
> > >
> >
>
https://codereview.chromium.org/488733002/diff/1/build/util/lastchange.py#new...
> > > > > build/util/lastchange.py:106: return VersionInfo('git', output)
> > > > > On 2014/08/19 18:49:46, iannucci wrote:
> > > > > > should we include the commit position here, if it's available in the
> > > commit
> > > > > > message?
> > > > > 
> > > > > The output cannot be multiline. It's unclear to me what would happen
if
> > the
> > > > > output had spaces in it, but given that it is going into a .h header
> file,
> > > I'm
> > > > > not sure I want to risk it.
> > > > 
> > > > It could be
> > > > 
> > > > deadbeef....
> > > > 
> > > > OR
> > > > 
> > > > beadfeeb...-refs/heads/master@{#12345}
> > > > 
> > > > No?
> > > 
> > > +scottmg and +evanm, who have made the most recent and most changes to
> > > lastchange.py, respectively. Do either of you have an opinion on the
format?
> > If
> > > not, I'd like to land as-is and get feedback from release managers as to
> > whether
> > > they need more info than just the hash, and how to format that info.
> > 
> > A full hash seems good to me, it's mostly to tag builds so we can look up
what
> a
> > binary corresponds to. We could always truncate in presentation later if
it's
> > too long/ugly.
> > 
> > AFAIK it's only used in strings like this:
> >
>
https://code.google.com/p/chromium/codesearch#search/&q=@LASTCHANGE@&sq=packa...
> > so I think adding the commit number should be fine. Do we have tooling to
find
> > out what shipped/when/where already? If not, adding the commit number seems
> like
> > a good idea.
> 
> crrev.com/12345 will (soon!) assume you're talking about refs/heads/master on
> chromium/src and redirect you to the correctly numbered commit on
> http://chromium.googlesource.com
> 
> We may want to add this later, but release branches will have footers that
look
> like:
>   Cr-Commit-Position: refs/branch-heads/1700@{#7}
>   Cr-Branched-From: deadbeef....-refs/heads/master@{#12345}
> 
> That is: 7th commit on 1700, and the 1700 branch diverged from master at
12345.
> 
> I think that this will be valuable information, but maybe I'm wrong. If we can
> only have one of them, the Commit-Position is the better one.

PTAL. It now produces LASTCHANGE=<40 char hash>-<Cr-Commit-Position> if the
commit position exists, otherwise just LASTCHANGE=<40 char hash>.

[scottmg, 2014-08-20 21:56:20]

On 2014/08/20 21:47:39, agable wrote:
> On 2014/08/20 20:34:06, iannucci wrote:
> > On 2014/08/20 19:42:26, scottmg wrote:
> > > On 2014/08/20 18:40:47, agable wrote:
> > > > On 2014/08/19 22:03:30, iannucci wrote:
> > > > > On 2014/08/19 22:00:30, agable wrote:
> > > > > >
> > https://codereview.chromium.org/488733002/diff/1/build/util/lastchange.py
> > > > > > File build/util/lastchange.py (right):
> > > > > > 
> > > > > >
> > > > >
> > > >
> > >
> >
>
https://codereview.chromium.org/488733002/diff/1/build/util/lastchange.py#new...
> > > > > > build/util/lastchange.py:106: return VersionInfo('git', output)
> > > > > > On 2014/08/19 18:49:46, iannucci wrote:
> > > > > > > should we include the commit position here, if it's available in
the
> > > > commit
> > > > > > > message?
> > > > > > 
> > > > > > The output cannot be multiline. It's unclear to me what would happen
> if
> > > the
> > > > > > output had spaces in it, but given that it is going into a .h header
> > file,
> > > > I'm
> > > > > > not sure I want to risk it.
> > > > > 
> > > > > It could be
> > > > > 
> > > > > deadbeef....
> > > > > 
> > > > > OR
> > > > > 
> > > > > beadfeeb...-refs/heads/master@{#12345}
> > > > > 
> > > > > No?
> > > > 
> > > > +scottmg and +evanm, who have made the most recent and most changes to
> > > > lastchange.py, respectively. Do either of you have an opinion on the
> format?
> > > If
> > > > not, I'd like to land as-is and get feedback from release managers as to
> > > whether
> > > > they need more info than just the hash, and how to format that info.
> > > 
> > > A full hash seems good to me, it's mostly to tag builds so we can look up
> what
> > a
> > > binary corresponds to. We could always truncate in presentation later if
> it's
> > > too long/ugly.
> > > 
> > > AFAIK it's only used in strings like this:
> > >
> >
>
https://code.google.com/p/chromium/codesearch#search/&q=@LASTCHANGE@&sq=packa...
> > > so I think adding the commit number should be fine. Do we have tooling to
> find
> > > out what shipped/when/where already? If not, adding the commit number
seems
> > like
> > > a good idea.
> > 
> > crrev.com/12345 will (soon!) assume you're talking about refs/heads/master
on
> > chromium/src and redirect you to the correctly numbered commit on
> > http://chromium.googlesource.com
> > 
> > We may want to add this later, but release branches will have footers that
> look
> > like:
> >   Cr-Commit-Position: refs/branch-heads/1700@{#7}
> >   Cr-Branched-From: deadbeef....-refs/heads/master@{#12345}
> > 
> > That is: 7th commit on 1700, and the 1700 branch diverged from master at
> 12345.
> > 
> > I think that this will be valuable information, but maybe I'm wrong. If we
can
> > only have one of them, the Commit-Position is the better one.
> 
> PTAL. It now produces LASTCHANGE=<40 char hash>-<Cr-Commit-Position> if the
> commit position exists, otherwise just LASTCHANGE=<40 char hash>.

pos will be something like "refs/heads/master@{#290659}", right?

"57cce2b8bd8e4c3f4401ca22fc8bd3efcd51e6f0-refs/heads/master@{#290659}"

LGTM! (maybe we should add a QR code in there somewhere :)

[iannucci, 2014-08-20 22:15:41]

lgtm, fwiw :)

[iannucci, 2014-08-20 22:17:22]

lgtm

[iannucci, 2014-08-20 22:17:34]

oops >_<

[scottmg, 2014-08-20 22:19:43]

On 2014/08/20 22:17:34, iannucci wrote:
> oops >_<

lgtm

[agable, 2014-08-21 15:15:55]

> pos will be something like "refs/heads/master@{#290659}", right?
> 
> "57cce2b8bd8e4c3f4401ca22fc8bd3efcd51e6f0-refs/heads/master@{#290659}"
> 
> LGTM! (maybe we should add a QR code in there somewhere :)

Yep, that's exactly what it looks like. CQing.

[agable, 2014-08-21 15:16:03]

The CQ bit was checked by agable@chromium.org

[commit-bot: I haz the power, 2014-08-21 15:18:19]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/agable@chromium.org/488733002/20001

[commit-bot: I haz the power, 2014-08-21 20:27:54]

Committed patchset #2 (20001) as 291165