Detect debug/release of out\Default from args.gn

Detect debug/release of out\Default from args.gn

The mini_installer build process was detecting debug versus release
based purely on the directory name, which fails on many gn setups
because there is no standard naming convention. This updates the
debug versus release detection by looking in args.gn if found.

BUG=596885
Committed: https://crrev.com/4d38569e5af8de3a252fcba0e465cfa7cbb40e2d
Cr-Commit-Position: refs/heads/master@{#385362}

[brucedawson, 2016-03-29 17:53:35]

Description was changed from

==========
Detect debug/release of out\Default from args.gn

The mini_installer build process was detecting debug versus release
based purely on the directory name, which fails on many gn setups
because there is no standard naming convention. This updates the
debug versus release detection by looking in args.gn if found.

BUG=596885
==========

to

==========
Detect debug/release of out\Default from args.gn

The mini_installer build process was detecting debug versus release
based purely on the directory name, which fails on many gn setups
because there is no standard naming convention. This updates the
debug versus release detection by looking in args.gn if found.

BUG=596885
==========

[brucedawson, 2016-03-29 17:53:35]

brucedawson@chromium.org changed reviewers:
+ dpranke@chromium.org

[brucedawson, 2016-03-29 17:54:51]

I added build type detection for out\Default. PTAL.

[Dirk Pranke, 2016-03-29 18:50:24]

lgtm

[brucedawson, 2016-03-29 18:51:49]

The CQ bit was checked by brucedawson@chromium.org

[commit-bot: I haz the power, 2016-03-29 18:52:06]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1837233003/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1837233003/1

[commit-bot: I haz the power, 2016-03-29 19:00:53]

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

[commit-bot: I haz the power, 2016-03-29 19:00:54]

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

[brucedawson, 2016-03-29 21:07:23]

brucedawson@chromium.org changed reviewers:
+ robertshield@chromium.org

[brucedawson, 2016-03-29 21:07:23]

Need owner approval. Maybe I should become an owner?

[brucedawson, 2016-03-29 21:08:10]

To clarify - robertshield@ - please approve as owner. I'll request becoming an
owner separately.

[robertshield, 2016-04-05 23:43:43]

LGTM w/nits

https://codereview.chromium.org/1837233003/diff/1/chrome/tools/build/win/crea...
File chrome/tools/build/win/create_installer_archive.py (right):

https://codereview.chromium.org/1837233003/diff/1/chrome/tools/build/win/crea...
chrome/tools/build/win/create_installer_archive.py:444: m =
re.match('^\s*is_debug\s*=\s*false(\s*$|\s*#.*$)', l)
Could you add a comment here that this regex is correct as per
https://chromium.googlesource.com/chromium/src/+/master/tools/gn/docs/referen...
?

https://codereview.chromium.org/1837233003/diff/1/chrome/tools/build/win/crea...
chrome/tools/build/win/create_installer_archive.py:466: "output directory or
args.gn, assuming Release build.")
What do you think about appending some explicit wording around: "If setup.exe
fails to launch, please check your build configuration is Release."  

This should be a failure mode, so rarely encountered, and when it is
encountered, setup.exe not launching will be expected in cases where we guessed
the build mode incorrectly.

[brucedawson, 2016-04-06 00:25:05]

https://codereview.chromium.org/1837233003/diff/1/chrome/tools/build/win/crea...
File chrome/tools/build/win/create_installer_archive.py (right):

https://codereview.chromium.org/1837233003/diff/1/chrome/tools/build/win/crea...
chrome/tools/build/win/create_installer_archive.py:444: m =
re.match('^\s*is_debug\s*=\s*false(\s*$|\s*#.*$)', l)
On 2016/04/05 23:43:43, robertshield wrote:
> Could you add a comment here that this regex is correct as per
>
https://chromium.googlesource.com/chromium/src/+/master/tools/gn/docs/referen...
> ?

Done.

https://codereview.chromium.org/1837233003/diff/1/chrome/tools/build/win/crea...
chrome/tools/build/win/create_installer_archive.py:466: "output directory or
args.gn, assuming Release build.")
On 2016/04/05 23:43:43, robertshield wrote:
> What do you think about appending some explicit wording around: "If setup.exe
> fails to launch, please check your build configuration is Release."  
> 
> This should be a failure mode, so rarely encountered, and when it is
> encountered, setup.exe not launching will be expected in cases where we
guessed
> the build mode incorrectly.

Done.

[Ilya Kulshin, 2016-04-06 00:34:30]

kulshin@chromium.org changed reviewers:
+ kulshin@chromium.org

[Ilya Kulshin, 2016-04-06 00:34:30]

lgtm

[brucedawson, 2016-04-06 00:40:18]

The CQ bit was checked by brucedawson@chromium.org

[brucedawson, 2016-04-06 00:40:18]

The patchset sent to the CQ was uploaded after l-g-t-m from
dpranke@chromium.org, robertshield@chromium.org
Link to the patchset: https://codereview.chromium.org/1837233003/#ps20001
(title: "Comment and printout tweaks per CR")

[commit-bot: I haz the power, 2016-04-06 00:40:49]

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

[commit-bot: I haz the power, 2016-04-06 01:25:46]

Description was changed from

==========
Detect debug/release of out\Default from args.gn

The mini_installer build process was detecting debug versus release
based purely on the directory name, which fails on many gn setups
because there is no standard naming convention. This updates the
debug versus release detection by looking in args.gn if found.

BUG=596885
==========

to

==========
Detect debug/release of out\Default from args.gn

The mini_installer build process was detecting debug versus release
based purely on the directory name, which fails on many gn setups
because there is no standard naming convention. This updates the
debug versus release detection by looking in args.gn if found.

BUG=596885
==========

[commit-bot: I haz the power, 2016-04-06 01:25:47]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-04-06 01:26:56]

Description was changed from

==========
Detect debug/release of out\Default from args.gn

The mini_installer build process was detecting debug versus release
based purely on the directory name, which fails on many gn setups
because there is no standard naming convention. This updates the
debug versus release detection by looking in args.gn if found.

BUG=596885
==========

to

==========
Detect debug/release of out\Default from args.gn

The mini_installer build process was detecting debug versus release
based purely on the directory name, which fails on many gn setups
because there is no standard naming convention. This updates the
debug versus release detection by looking in args.gn if found.

BUG=596885

Committed: https://crrev.com/4d38569e5af8de3a252fcba0e465cfa7cbb40e2d
Cr-Commit-Position: refs/heads/master@{#385362}
==========

[commit-bot: I haz the power, 2016-04-06 01:26:57]

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