[Mac/GN] Add targets to run dump_syms and create the dSYM archive.

[Mac/GN] Add targets to run dump_syms and create the dSYM archive.

These steps are required for the official build.

BUG=330301, 431177
R=mark@chromium.org

Committed: https://crrev.com/a3be980be2732db92353823012b8ee93ffedc7cd
Cr-Commit-Position: refs/heads/master@{#399045}

[Mark Mentovai, 2016-06-09 18:48:04]

LGTM

https://codereview.chromium.org/2054893002/diff/1/build/redirect_stdout.py
File build/redirect_stdout.py (right):

https://codereview.chromium.org/2054893002/diff/1/build/redirect_stdout.py#ne...
build/redirect_stdout.py:18: with open(sys.argv[1], 'w+') as fp:
The + is fine, but I’m curious why it’s there.

https://codereview.chromium.org/2054893002/diff/1/chrome/BUILD.gn
File chrome/BUILD.gn (right):

https://codereview.chromium.org/2054893002/diff/1/chrome/BUILD.gn#newcode439
chrome/BUILD.gn:439: if (is_chrome_branded && is_official_build) {
We don’t have a better variable for “want dumped symbols” than this as we do for
“want dSYMs”?

https://codereview.chromium.org/2054893002/diff/1/chrome/BUILD.gn#newcode1036
chrome/BUILD.gn:1036: # Prefix dump_syms with a ./ in case a user has it in
their path.
Does this mean that gn or something puts an implicit dot in its path? Hmm…

Do this or use an absolute path, good call. Absolute paths are nice when you
want to copy and paste commands from verbose output and not have to worry about
your working directory.

https://codereview.chromium.org/2054893002/diff/1/chrome/tools/build/mac/arch...
File chrome/tools/build/mac/archive_symbols.py (right):

https://codereview.chromium.org/2054893002/diff/1/chrome/tools/build/mac/arch...
chrome/tools/build/mac/archive_symbols.py:26: if os.path.exists(path):
try-catch swallowing OSError ENOENT might be more Pythony, but OK.

https://codereview.chromium.org/2054893002/diff/1/chrome/tools/build/mac/arch...
chrome/tools/build/mac/archive_symbols.py:31: sys.exit(Main(sys.argv))
I like to call main with sys.argv[1:] (especially where it’s called something
other than argv in main, such as args as it is here), so that you have more
natural-looking ['tar', '-cjf'] + args instead of having to 1-index everywhere.

UNLESS you want to use argv[0] in the usage message.

[Robert Sesek, 2016-06-09 20:53:46]

https://codereview.chromium.org/2054893002/diff/1/build/redirect_stdout.py
File build/redirect_stdout.py (right):

https://codereview.chromium.org/2054893002/diff/1/build/redirect_stdout.py#ne...
build/redirect_stdout.py:18: with open(sys.argv[1], 'w+') as fp:
On 2016/06/09 18:48:04, Mark Mentovai wrote:
> The + is fine, but I’m curious why it’s there.

Because I'm used to C... fixed.

https://codereview.chromium.org/2054893002/diff/1/chrome/BUILD.gn
File chrome/BUILD.gn (right):

https://codereview.chromium.org/2054893002/diff/1/chrome/BUILD.gn#newcode439
chrome/BUILD.gn:439: if (is_chrome_branded && is_official_build) {
On 2016/06/09 18:48:04, Mark Mentovai wrote:
> We don’t have a better variable for “want dumped symbols” than this as we do
for
> “want dSYMs”?

This is making the top-level "chrome" target depend on the dSYM archive and
dump_syms targets if you're building an official, branded build. But these
targets only do something if enable_dsyms=true.

https://codereview.chromium.org/2054893002/diff/1/chrome/BUILD.gn#newcode1036
chrome/BUILD.gn:1036: # Prefix dump_syms with a ./ in case a user has it in
their path.
On 2016/06/09 18:48:04, Mark Mentovai wrote:
> Does this mean that gn or something puts an implicit dot in its path? Hmm…

No, the way I had rebase_path'd it, there was no path prefix at all. Switched to
an absolute path.

https://codereview.chromium.org/2054893002/diff/1/chrome/tools/build/mac/arch...
File chrome/tools/build/mac/archive_symbols.py (right):

https://codereview.chromium.org/2054893002/diff/1/chrome/tools/build/mac/arch...
chrome/tools/build/mac/archive_symbols.py:26: if os.path.exists(path):
On 2016/06/09 18:48:04, Mark Mentovai wrote:
> try-catch swallowing OSError ENOENT might be more Pythony, but OK.

Ack, but will keep since this pattern is in a few other build py files.

https://codereview.chromium.org/2054893002/diff/1/chrome/tools/build/mac/arch...
chrome/tools/build/mac/archive_symbols.py:31: sys.exit(Main(sys.argv))
On 2016/06/09 18:48:04, Mark Mentovai wrote:
> I like to call main with sys.argv[1:] (especially where it’s called something
> other than argv in main, such as args as it is here), so that you have more
> natural-looking ['tar', '-cjf'] + args instead of having to 1-index
everywhere.
> 
> UNLESS you want to use argv[0] in the usage message.

Done.

[Mark Mentovai, 2016-06-09 21:50:11]

LGTM

[Robert Sesek, 2016-06-09 21:52:52]

rsesek@chromium.org changed reviewers:
+ thakis@chromium.org

[Robert Sesek, 2016-06-09 21:52:52]

+thakis for OWNERS

[Nico, 2016-06-09 21:57:00]

rs-lgtm

[Robert Sesek, 2016-06-09 22:00:59]

The CQ bit was checked by rsesek@chromium.org

[commit-bot: I haz the power, 2016-06-09 22:02:24]

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

[commit-bot: I haz the power, 2016-06-09 22:55:43]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-06-09 22:56:00]

CQ bit was unchecked

[commit-bot: I haz the power, 2016-06-09 22:59:17]

Description was changed from

==========
[Mac/GN] Add targets to run dump_syms and create the dSYM archive.

These steps are required for the official build.

BUG=330301,431177
R=mark@chromium.org
==========

to

==========
[Mac/GN] Add targets to run dump_syms and create the dSYM archive.

These steps are required for the official build.

BUG=330301,431177
R=mark@chromium.org

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

[commit-bot: I haz the power, 2016-06-09 22:59:18]

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