[NaCl SDK] Fix the SDK builders to use Cr-Commit-Position.

Committed: https://crrev.com/8594182e1a9fb4bb10a586d4b7d5210682038611
Cr-Commit-Position: refs/heads/master@{#292179}

[bradn, 2014-08-26 23:51:40]

bradnelson@google.com changed reviewers:
+ bradnelson@google.com

[bradn, 2014-08-26 23:51:40]

https://codereview.chromium.org/507883003/diff/1/native_client_sdk/src/build_...
File native_client_sdk/src/build_tools/build_version.py (right):

https://codereview.chromium.org/507883003/diff/1/native_client_sdk/src/build_...
native_client_sdk/src/build_tools/build_version.py:98: 
two lines

[binji, 2014-08-26 23:53:42]

https://codereview.chromium.org/507883003/diff/1/native_client_sdk/src/build_...
File native_client_sdk/src/build_tools/build_version.py (right):

https://codereview.chromium.org/507883003/diff/1/native_client_sdk/src/build_...
native_client_sdk/src/build_tools/build_version.py:98: 
On 2014/08/26 23:51:40, bradn wrote:
> two lines

Done.

[bradn, 2014-08-26 23:56:12]

lgtm

[Matt Giuca, 2014-08-27 01:20:18]

FYI:

1. If you're planning to later switch over to using lastchange.py, note that I
believe it will not simply return the commit position, but rather a string of
the form "GITHASH-COMMITPOSITION". See
https://codereview.chromium.org/488733002. You might want to do the same here.

2. You can probably revert the following CLs (in this order) as part of this CL:
https://codereview.chromium.org/507673003
https://codereview.chromium.org/502883002

since the revision will once again be an integer, and we can meaningfully
compare revisions. (Or I am happy to revert those in a follow-up CL.)

[binji, 2014-08-27 15:24:23]

On 2014/08/27 01:20:18, Matt Giuca wrote:
> FYI:
> 
> 1. If you're planning to later switch over to using lastchange.py, note that I
> believe it will not simply return the commit position, but rather a string of
> the form "GITHASH-COMMITPOSITION". See
> https://codereview.chromium.org/488733002. You might want to do the same here.
> 
> 2. You can probably revert the following CLs (in this order) as part of this
CL:
> https://codereview.chromium.org/507673003
> https://codereview.chromium.org/502883002
> 
> since the revision will once again be an integer, and we can meaningfully
> compare revisions. (Or I am happy to revert those in a follow-up CL.)

I thought about this some more. This simple solution of using the trunk commit 
position as just a number will not work for branch builds, which have commit 
positions that look like "refs/branch-heads/2125@{#116}". A particular major 
version may use different branch numbers over the course of its history, so its
branch value must be part of the "revision" (not just 116 as this CL would do).

Not to mention the fact that this value is also displayed to the user in the
README file included with the SDK as a method to identify when/how the SDK was 
built. This value therefore needs to uniquely identify the revision that would
recreate this SDK.

I think the only viable solution is to use the full commit position (with ref),
and fix all the places in the SDK where it is assumed to be an integer:

$ pwdf
.../native_client_sdk/src/build_tools
$ git grep -i revision | wc
    238    1271   17815

[binji, 2014-08-27 17:45:27]

On 2014/08/27 15:24:23, binji wrote:
> On 2014/08/27 01:20:18, Matt Giuca wrote:
> > FYI:
> > 
> > 1. If you're planning to later switch over to using lastchange.py, note that
I
> > believe it will not simply return the commit position, but rather a string
of
> > the form "GITHASH-COMMITPOSITION". See
> > https://codereview.chromium.org/488733002. You might want to do the same
here.
> > 
> > 2. You can probably revert the following CLs (in this order) as part of this
> CL:
> > https://codereview.chromium.org/507673003
> > https://codereview.chromium.org/502883002
> > 
> > since the revision will once again be an integer, and we can meaningfully
> > compare revisions. (Or I am happy to revert those in a follow-up CL.)
> 
> I thought about this some more. This simple solution of using the trunk commit

> position as just a number will not work for branch builds, which have commit 
> positions that look like "refs/branch-heads/2125@{#116}". A particular major 
> version may use different branch numbers over the course of its history, so
its
> branch value must be part of the "revision" (not just 116 as this CL would
do).
> 
> Not to mention the fact that this value is also displayed to the user in the
> README file included with the SDK as a method to identify when/how the SDK was

> built. This value therefore needs to uniquely identify the revision that would
> recreate this SDK.
> 
> I think the only viable solution is to use the full commit position (with
ref),
> and fix all the places in the SDK where it is assumed to be an integer:
> 
> $ pwdf
> .../native_client_sdk/src/build_tools
> $ git grep -i revision | wc
>     238    1271   17815

OK, I talked through this with bradnelson@ and we think that this can land
as-is,
and we can incrementally resolve other issues (with SDK branches,
update_nacl_manifest.py,
the SDK updater, naclports and getos.py)

[binji, 2014-08-27 17:59:47]

Committed patchset #2 (id:20001) to pending queue manually as 2206bcb (presubmit
successful).

[commit-bot: I haz the power, 2014-09-10 02:52:36]

Patchset 2 (id:??) landed as
https://crrev.com/8594182e1a9fb4bb10a586d4b7d5210682038611
Cr-Commit-Position: refs/heads/master@{#292179}