Use full version for breakpad on iOS

Use full version for breakpad on iOS

The current version for breakpad in crash reports is CFBundleShortVersionString
It should be long version.

BUG=626490
Committed: https://crrev.com/b0959ee3fc34de2a87f0dce5fc29bddb314da576
Cr-Commit-Position: refs/heads/master@{#408953}

[Olivier, 2016-07-08 07:46:07]

olivierrobin@chromium.org changed reviewers:
+ rohitrao@chromium.org, sdefresne@chromium.org

[sdefresne, 2016-07-08 07:50:41]

https://codereview.chromium.org/2132913003/diff/1/build/mac/tweak_info_plist.py
File build/mac/tweak_info_plist.py (right):

https://codereview.chromium.org/2132913003/diff/1/build/mac/tweak_info_plist....
build/mac/tweak_info_plist.py:312: version_format_for_key['BreakpadVersion'] = \
Are you sure we also want to do this on mac? I think the use of
'CFBundleShortVersionString' was deliberate there, so maybe there should be a
test for options.platform here:

if options.use_breakpad:
  if option.platform == "mac":
    version_format_for_key['BreakpadVersion'] = \
        version_format_for_key['CFBundleShortVersionString']
  else:
    version_format_for_key['BreakpadVersion'] = \
        version_format_for_key['CFBundleVersion']

[Olivier, 2016-07-08 07:58:02]

https://codereview.chromium.org/2132913003/diff/1/build/mac/tweak_info_plist.py
File build/mac/tweak_info_plist.py (right):

https://codereview.chromium.org/2132913003/diff/1/build/mac/tweak_info_plist....
build/mac/tweak_info_plist.py:312: version_format_for_key['BreakpadVersion'] = \
On 2016/07/08 07:50:41, sdefresne wrote:
> Are you sure we also want to do this on mac? I think the use of
> 'CFBundleShortVersionString' was deliberate there, so maybe there should be a
> test for options.platform here:
> 
> if options.use_breakpad:
>   if option.platform == "mac":
>     version_format_for_key['BreakpadVersion'] = \
>         version_format_for_key['CFBundleShortVersionString']
>   else:
>     version_format_for_key['BreakpadVersion'] = \
>         version_format_for_key['CFBundleVersion']

On mac, ShortVersion is '@MAJOR@.@MINOR@.@BUILD@.@PATCH@'.
My understanding is that for breakpad, the version should be in this format.
At least, it is at the moment for Windows, Mac and Android.

[sdefresne, 2016-07-08 08:00:46]

lgtm

https://codereview.chromium.org/2132913003/diff/1/build/mac/tweak_info_plist.py
File build/mac/tweak_info_plist.py (right):

https://codereview.chromium.org/2132913003/diff/1/build/mac/tweak_info_plist....
build/mac/tweak_info_plist.py:312: version_format_for_key['BreakpadVersion'] = \
On 2016/07/08 07:58:02, Olivier Robin wrote:
> On 2016/07/08 07:50:41, sdefresne wrote:
> > Are you sure we also want to do this on mac? I think the use of
> > 'CFBundleShortVersionString' was deliberate there, so maybe there should be
a
> > test for options.platform here:
> > 
> > if options.use_breakpad:
> >   if option.platform == "mac":
> >     version_format_for_key['BreakpadVersion'] = \
> >         version_format_for_key['CFBundleShortVersionString']
> >   else:
> >     version_format_for_key['BreakpadVersion'] = \
> >         version_format_for_key['CFBundleVersion']
> 
> On mac, ShortVersion is mailto:.
> My understanding is that for breakpad, the version should be in this format.
> At least, it is at the moment for Windows, Mac and Android.

Ack.

[rohitrao (ping after 24h), 2016-07-08 10:41:05]

Do we understand why this regressed last month?

https://codereview.chromium.org/2132913003/diff/1/build/mac/tweak_info_plist.py
File build/mac/tweak_info_plist.py (right):

https://codereview.chromium.org/2132913003/diff/1/build/mac/tweak_info_plist....
build/mac/tweak_info_plist.py:312: version_format_for_key['BreakpadVersion'] = \
On 2016/07/08 08:00:46, sdefresne wrote:
> On 2016/07/08 07:58:02, Olivier Robin wrote:
> > On 2016/07/08 07:50:41, sdefresne wrote:
> > > Are you sure we also want to do this on mac? I think the use of
> > > 'CFBundleShortVersionString' was deliberate there, so maybe there should
be
> a
> > > test for options.platform here:
> > > 
> > > if options.use_breakpad:
> > >   if option.platform == "mac":
> > >     version_format_for_key['BreakpadVersion'] = \
> > >         version_format_for_key['CFBundleShortVersionString']
> > >   else:
> > >     version_format_for_key['BreakpadVersion'] = \
> > >         version_format_for_key['CFBundleVersion']
> > 
> > On mac, ShortVersion is mailto:.
> > My understanding is that for breakpad, the version should be in this format.
> > At least, it is at the moment for Windows, Mac and Android.
> 
> Ack.

Let's have Mark take a look to be sure.

[Olivier, 2016-07-08 10:54:53]

On 2016/07/08 10:41:05, rohitrao wrote:
> Do we understand why this regressed last month?
> 
>
https://codereview.chromium.org/2132913003/diff/1/build/mac/tweak_info_plist.py
> File build/mac/tweak_info_plist.py (right):
> 
>
https://codereview.chromium.org/2132913003/diff/1/build/mac/tweak_info_plist....
> build/mac/tweak_info_plist.py:312: version_format_for_key['BreakpadVersion'] =
\
> On 2016/07/08 08:00:46, sdefresne wrote:
> > On 2016/07/08 07:58:02, Olivier Robin wrote:
> > > On 2016/07/08 07:50:41, sdefresne wrote:
> > > > Are you sure we also want to do this on mac? I think the use of
> > > > 'CFBundleShortVersionString' was deliberate there, so maybe there should
> be
> > a
> > > > test for options.platform here:
> > > > 
> > > > if options.use_breakpad:
> > > >   if option.platform == "mac":
> > > >     version_format_for_key['BreakpadVersion'] = \
> > > >         version_format_for_key['CFBundleShortVersionString']
> > > >   else:
> > > >     version_format_for_key['BreakpadVersion'] = \
> > > >         version_format_for_key['CFBundleVersion']
> > > 
> > > On mac, ShortVersion is mailto:.
> > > My understanding is that for breakpad, the version should be in this
format.
> > > At least, it is at the moment for Windows, Mac and Android.
> > 
> > Ack.
> 
> Let's have Mark take a look to be sure.

Yes, This must be the CL that replaced tweak_info_plist.sh by
tweak_info_plist.py

[Olivier, 2016-07-29 08:43:10]

The CQ bit was checked by olivierrobin@chromium.org

[commit-bot: I haz the power, 2016-07-29 08:43:23]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Olivier, 2016-07-29 08:46:48]

olivierrobin@chromium.org changed reviewers:
+ mark@chromium.org

[Olivier, 2016-07-29 08:46:48]

+ mark for OWNER

[commit-bot: I haz the power, 2016-07-29 08:48:26]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-07-29 08:48:27]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[Olivier, 2016-08-01 09:12:02]

olivierrobin@chromium.org changed reviewers:
+ thomasvl@google.com

[Mark Mentovai, 2016-08-01 12:49:28]

LGTM

[Mark Mentovai, 2016-08-01 12:49:58]

Revise the CL description to say “on iOS”, since this isn’t a change on macOS.

[Olivier, 2016-08-01 12:52:59]

Description was changed from

==========
Use full version for breakpad

The current version for breakpad in crash reports is CFBundleShortVersionString
It should be long version.

BUG=626490
==========

to

==========
Use full version for breakpad on iOS

The current version for breakpad in crash reports is CFBundleShortVersionString
It should be long version.

BUG=626490
==========

[Olivier, 2016-08-01 12:53:11]

On 2016/08/01 12:49:58, Mark Mentovai wrote:
> Revise the CL description to say “on iOS”, since this isn’t a change on macOS.

Done. Thanks

[Olivier, 2016-08-01 12:53:17]

The CQ bit was checked by olivierrobin@chromium.org

[commit-bot: I haz the power, 2016-08-01 12:53:34]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-01 13:36:25]

Description was changed from

==========
Use full version for breakpad on iOS

The current version for breakpad in crash reports is CFBundleShortVersionString
It should be long version.

BUG=626490
==========

to

==========
Use full version for breakpad on iOS

The current version for breakpad in crash reports is CFBundleShortVersionString
It should be long version.

BUG=626490
==========

[commit-bot: I haz the power, 2016-08-01 13:36:26]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2016-08-01 13:38:03]

Description was changed from

==========
Use full version for breakpad on iOS

The current version for breakpad in crash reports is CFBundleShortVersionString
It should be long version.

BUG=626490
==========

to

==========
Use full version for breakpad on iOS

The current version for breakpad in crash reports is CFBundleShortVersionString
It should be long version.

BUG=626490

Committed: https://crrev.com/b0959ee3fc34de2a87f0dce5fc29bddb314da576
Cr-Commit-Position: refs/heads/master@{#408953}
==========

[commit-bot: I haz the power, 2016-08-01 13:38:05]

Patchset 1 (id:??) landed as
https://crrev.com/b0959ee3fc34de2a87f0dce5fc29bddb314da576
Cr-Commit-Position: refs/heads/master@{#408953}