Replace CAST_BUILD_REVISION with <major>.<minor>.<incremental>.

Replace CAST_BUILD_REVISION with <major>.<minor>.<incremental>.

Old <incremental>-only number is preserved in CAST_BUILD_INCREMENTAL.

BUG=

Committed: https://crrev.com/439edb66143eaaeae6ea0201d7e493ef41000377
Cr-Commit-Position: refs/heads/master@{#312887}

[byungchul, 2015-01-22 17:37:01]

byungchul@chromium.org changed reviewers:
+ gunsch@chromium.org, lcwu@chromium.org

[gunsch, 2015-01-22 17:42:05]

https://codereview.chromium.org/866843004/diff/1/chromecast/chromecast.gyp
File chromecast/chromecast.gyp (right):

https://codereview.chromium.org/866843004/diff/1/chromecast/chromecast.gyp#ne...
chromecast/chromecast.gyp:266: '-e', 'CAST_BUILD_RELEASE="<!(echo
${CAST_BUILD_RELEASE:="eng.${USER}"})"',
where does CAST_BUILD_RELEASE come from? I don't see this on the builders.

https://codereview.chromium.org/866843004/diff/1/chromecast/common/version.h.in
File chromecast/common/version.h.in (right):

https://codereview.chromium.org/866843004/diff/1/chromecast/common/version.h....
chromecast/common/version.h.in:12: #define CAST_BUILD_RELEASE
"@CAST_BUILD_RELEASE@"
do we need CAST_BUILD_RELEASE anywhere? If not, I'd rather not propagate the
C-define.

[byungchul, 2015-01-22 17:47:54]

https://codereview.chromium.org/866843004/diff/1/chromecast/chromecast.gyp
File chromecast/chromecast.gyp (right):

https://codereview.chromium.org/866843004/diff/1/chromecast/chromecast.gyp#ne...
chromecast/chromecast.gyp:266: '-e', 'CAST_BUILD_RELEASE="<!(echo
${CAST_BUILD_RELEASE:="eng.${USER}"})"',
On 2015/01/22 17:42:05, gunsch wrote:
> where does CAST_BUILD_RELEASE come from? I don't see this on the builders.

This will be set by chromecast.mk based on PLATFORM_VERSION in
build/core/version_defaults.mk. I didn't want to describe it here because
version_defaults.mk are not available.

https://codereview.chromium.org/866843004/diff/1/chromecast/common/version.h.in
File chromecast/common/version.h.in (right):

https://codereview.chromium.org/866843004/diff/1/chromecast/common/version.h....
chromecast/common/version.h.in:12: #define CAST_BUILD_RELEASE
"@CAST_BUILD_RELEASE@"
On 2015/01/22 17:42:05, gunsch wrote:
> do we need CAST_BUILD_RELEASE anywhere? If not, I'd rather not propagate the
> C-define.

Removed.

[gunsch, 2015-01-22 19:46:20]

https://codereview.chromium.org/866843004/diff/1/chromecast/chromecast.gyp
File chromecast/chromecast.gyp (right):

https://codereview.chromium.org/866843004/diff/1/chromecast/chromecast.gyp#ne...
chromecast/chromecast.gyp:266: '-e', 'CAST_BUILD_RELEASE="<!(echo
${CAST_BUILD_RELEASE:="eng.${USER}"})"',
On 2015/01/22 17:47:54, byungchul wrote:
> On 2015/01/22 17:42:05, gunsch wrote:
> > where does CAST_BUILD_RELEASE come from? I don't see this on the builders.
> 
> This will be set by chromecast.mk based on PLATFORM_VERSION in
> build/core/version_defaults.mk. I didn't want to describe it here because
> version_defaults.mk are not available.

Discussed offline about the issues with this approach.

Let's figure out how to set CAST_BUILD_RELEASE consistently in all of our
downstream builds first.

[byungchul, 2015-01-22 19:46:33]

https://codereview.chromium.org/866843004/diff/1/chromecast/chromecast.gyp
File chromecast/chromecast.gyp (right):

https://codereview.chromium.org/866843004/diff/1/chromecast/chromecast.gyp#ne...
chromecast/chromecast.gyp:266: '-e', 'CAST_BUILD_RELEASE="<!(echo
${CAST_BUILD_RELEASE:="eng.${USER}"})"',
On 2015/01/22 17:47:54, byungchul wrote:
> On 2015/01/22 17:42:05, gunsch wrote:
> > where does CAST_BUILD_RELEASE come from? I don't see this on the builders.
> 
> This will be set by chromecast.mk based on PLATFORM_VERSION in
> build/core/version_defaults.mk. I didn't want to describe it here because
> version_defaults.mk are not available.

Changed to read from cast_build_release file which will be set by build env
script.

[byungchul, 2015-01-22 20:52:08]

On 2015/01/22 19:46:20, gunsch wrote:
> https://codereview.chromium.org/866843004/diff/1/chromecast/chromecast.gyp
> File chromecast/chromecast.gyp (right):
> 
>
https://codereview.chromium.org/866843004/diff/1/chromecast/chromecast.gyp#ne...
> chromecast/chromecast.gyp:266: '-e', 'CAST_BUILD_RELEASE="<!(echo
> ${CAST_BUILD_RELEASE:="eng.${USER}"})"',
> On 2015/01/22 17:47:54, byungchul wrote:
> > On 2015/01/22 17:42:05, gunsch wrote:
> > > where does CAST_BUILD_RELEASE come from? I don't see this on the builders.
> > 
> > This will be set by chromecast.mk based on PLATFORM_VERSION in
> > build/core/version_defaults.mk. I didn't want to describe it here because
> > version_defaults.mk are not available.
> 
> Discussed offline about the issues with this approach.
> 
> Let's figure out how to set CAST_BUILD_RELEASE consistently in all of our
> downstream builds first.

I would take a different approach which compares CAST_BUILD_RELEASE in
chromecast/internal and PLATFORM_VERSION, and stops building with error if they
don't match to each other.
PLATFORM_VERSION in version_deafults.mk is set by ":=" which means it doesn't
wait for all makefiles to be parsed completely. So, even if we can reset
PLATFORM_VERSION at some point with chromecast.mk, some rules still see the old
value.

[byungchul, 2015-01-23 04:05:21]

PTAL

[gunsch, 2015-01-23 16:50:33]

https://codereview.chromium.org/866843004/diff/80001/chromecast/chromecast.gyp
File chromecast/chromecast.gyp (right):

https://codereview.chromium.org/866843004/diff/80001/chromecast/chromecast.gy...
chromecast/chromecast.gyp:267: '-e', 'CAST_BUILD_RELEASE="<!(cat
<(cast_build_release))"',
How does this fallback if "internal" directory is not present?  (e.g. a public
checkout)

[byungchul, 2015-01-23 17:12:52]

https://codereview.chromium.org/866843004/diff/80001/chromecast/chromecast.gyp
File chromecast/chromecast.gyp (right):

https://codereview.chromium.org/866843004/diff/80001/chromecast/chromecast.gy...
chromecast/chromecast.gyp:267: '-e', 'CAST_BUILD_RELEASE="<!(cat
<(cast_build_release))"',
On 2015/01/23 16:50:33, gunsch wrote:
> How does this fallback if "internal" directory is not present?  (e.g. a public
> checkout)

Good catch. Done.

[lcwu1, 2015-01-23 17:30:59]

lgtm

[gunsch, 2015-01-23 17:33:30]

https://codereview.chromium.org/866843004/diff/100001/chromecast/chromecast.gyp
File chromecast/chromecast.gyp (right):

https://codereview.chromium.org/866843004/diff/100001/chromecast/chromecast.g...
chromecast/chromecast.gyp:268: # otherwize, a dev string is used.
nit: otherwize --> otherwise

https://codereview.chromium.org/866843004/diff/100001/chromecast/chromecast.g...
chromecast/chromecast.gyp:269: '-e', 'CAST_BUILD_RELEASE="<!(if [[ -f
<(cast_build_release) ]]; then cat <(cast_build_release); else echo eng.${USER};
fi)"',
Do we know for sure that gyp uses bash shells to run this test? double-bracket
is bash-specific. I'd prefer an explicit call to "test" if we're not sure.

[byungchul, 2015-01-23 17:51:27]

https://codereview.chromium.org/866843004/diff/100001/chromecast/chromecast.gyp
File chromecast/chromecast.gyp (right):

https://codereview.chromium.org/866843004/diff/100001/chromecast/chromecast.g...
chromecast/chromecast.gyp:268: # otherwize, a dev string is used.
On 2015/01/23 17:33:30, gunsch wrote:
> nit: otherwize --> otherwise

Done.

https://codereview.chromium.org/866843004/diff/100001/chromecast/chromecast.g...
chromecast/chromecast.gyp:269: '-e', 'CAST_BUILD_RELEASE="<!(if [[ -f
<(cast_build_release) ]]; then cat <(cast_build_release); else echo eng.${USER};
fi)"',
On 2015/01/23 17:33:30, gunsch wrote:
> Do we know for sure that gyp uses bash shells to run this test? double-bracket
> is bash-specific. I'd prefer an explicit call to "test" if we're not sure.

Done.

[gunsch, 2015-01-23 18:02:50]

lgtm

[byungchul, 2015-01-23 18:05:22]

The CQ bit was checked by byungchul@chromium.org

[commit-bot: I haz the power, 2015-01-23 18:05:31]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/866843004/120001

[commit-bot: I haz the power, 2015-01-23 18:59:27]

Committed patchset #7 (id:120001)

[commit-bot: I haz the power, 2015-01-23 19:00:36]

Patchset 7 (id:??) landed as
https://crrev.com/439edb66143eaaeae6ea0201d7e493ef41000377
Cr-Commit-Position: refs/heads/master@{#312887}