New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 634184 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Sep 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Bug



Sign in to add a comment

clean up configs in chromium's mb_config.pyl now that GYP is gone

Project Member Reported by dpranke@chromium.org, Aug 3 2016

Issue description

Now that we don't have to support GYP any more, we can significantly clean up and
rationalize the args passed to builders and some of the defaults in the GN build 
(e.g., making debug builds component builds by default), and clean up a lot of
config entries for the builders in the MB config pyl.

Eventually we want to get rid of MB altogether (or at least make it a much thinner wrapper), but that's out of scope for this bug (we will have to work out how we want
to use args files, handle isolates and command lines, make analyze work properly, 
etc.).



 
Labels: -Pri-3 Pri-2
Status: Available (was: Started)
Status: Assigned (was: Available)
Project Member

Comment 3 by bugdroid1@chromium.org, Aug 30 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/64fea5e29655036e2c5410b903cd43f832d04e93

commit 64fea5e29655036e2c5410b903cd43f832d04e93
Author: dpranke <dpranke@chromium.org>
Date: Tue Aug 30 16:55:01 2016

Rework default GN symbol handling for win goma builds.

Win goma builds do not support PDB files, so trying to build with
full symbols is very painful. Previously we would default to "full
symbols" but silently turn off symbols if using goma; this CL
restructures things so that we default to "minimal symbols" if
using goma (and visual studio's compiler).

R=brucedawson@chromium.org, brettw@chromium.org, thakis@chromium.org
BUG= 634184 

Review-Url: https://codereview.chromium.org/2215433002
Cr-Commit-Position: refs/heads/master@{#415325}

[modify] https://crrev.com/64fea5e29655036e2c5410b903cd43f832d04e93/build/config/compiler/BUILD.gn
[modify] https://crrev.com/64fea5e29655036e2c5410b903cd43f832d04e93/build/config/compiler/compiler.gni

Cc: samu...@chromium.org
Looks like the above CL crrev.com/415325 affected Chromedriver Windows bot.

https://build.chromium.org/p/chromium.chromedriver/builders/Win7/builds/18177/steps/meta%20build/logs/stdio


Writing """\
goma_dir = "E:\\b\\build\\slave\\cache\\cipd\\goma"
is_component_build = false
is_debug = false
target_cpu = "x86"
use_goma = true
""" to E:\b\build\slave\chromedriver_win7\build\src\out\Default\args.gn.

E:\b\build\slave\chromedriver_win7\build\src\buildtools\win\gn.exe gen //out/Default --check
  -> returned 1
ERROR at //build/config/compiler/compiler.gni:85:24: Undefined identifier
  } else if (is_win && use_goma && !is_clang) {
                       ^-------
See //BUILD.gn:11:1: whence it was imported.
import("//build/config/compiler/compiler.gni")
^--------------------------------------------
GN gen failed: 1
step returned non-zero exit code: 1

Can you please check if it's related?
Yes, it's related. I will revert the change; thanks!
Project Member

Comment 6 by bugdroid1@chromium.org, Aug 30 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/67a9b0799014469e2819b237c398f7e0ab0425f6

commit 67a9b0799014469e2819b237c398f7e0ab0425f6
Author: mmenke <mmenke@chromium.org>
Date: Tue Aug 30 18:24:40 2016

Revert "Rework default GN symbol handling for win goma builds."

This reverts commit 64fea5e29655036e2c5410b903cd43f832d04e93.

The CL broke the Windows build for some users.

NOPRESUBMIT=true
NOTRY=true
TBR=dpranke@chromium.org
BUG= 634184 

Review-Url: https://codereview.chromium.org/2294903002
Cr-Commit-Position: refs/heads/master@{#415361}

[modify] https://crrev.com/67a9b0799014469e2819b237c398f7e0ab0425f6/build/config/compiler/BUILD.gn
[modify] https://crrev.com/67a9b0799014469e2819b237c398f7e0ab0425f6/build/config/compiler/compiler.gni

Project Member

Comment 7 by bugdroid1@chromium.org, Aug 30 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/169ed77764c622e520118a43940ad06877234cbf

commit 169ed77764c622e520118a43940ad06877234cbf
Author: dpranke <dpranke@chromium.org>
Date: Tue Aug 30 21:38:48 2016

Re-land "Rework default GN symbol handling for win goma builds."

This relands r415325 with the neede fix.

TBR=thakis@chromium.org
BUG= 634184 

Review-Url: https://codereview.chromium.org/2296953002
Cr-Commit-Position: refs/heads/master@{#415435}

[modify] https://crrev.com/169ed77764c622e520118a43940ad06877234cbf/build/config/compiler/BUILD.gn
[modify] https://crrev.com/169ed77764c622e520118a43940ad06877234cbf/build/config/compiler/compiler.gni

Status: Started (was: Assigned)
Project Member

Comment 9 by bugdroid1@chromium.org, Sep 23 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f37aebb917de5c93bf44f93370694b813e2f8ecc

commit f37aebb917de5c93bf44f93370694b813e2f8ecc
Author: dpranke <dpranke@chromium.org>
Date: Fri Sep 23 01:14:49 2016

Clean up mb_config.pyl now that we're off GYP.

This change cleans up a bunch of things now that we don't need to
support GYP:

'gn' becomes the default build type, so we no longer need 'gn_' in
the config name or need to specify the 'gn' mixin.

'swarming' and 'noswarming' are no longer needed in the config
names since the swarming flag was only needed for GYP.
'archive_gpu_tests' is also gone, for the same reason.

The 'none' builder type is gone; builders that don't build simply
aren't listed.

The 'configs' and 'mixins' sections are sorted and de-duped.

Any non-clang win builder that was specifying both goma and
minimal_symbols is now just goma, since goma + win + !clang
implies minimal symbols. A follow-on CL should make goma
imply minimal symbols across the board ...

'x64' no longer needs to be specified explicitly on windows
since it is the default.

The 80-col line limit in mb_config.pyl was dropped so that
configs can be listed on one line.

The 'symbolized' config is being removed; it was never
implemented in GN since we dropped those builders;
the one remaining v8 builder has been reporting
warnings since :).

R=thakis@chromium.org, machenbach@chromium.org
BUG= 634184 

Review-Url: https://codereview.chromium.org/2357483002
Cr-Commit-Position: refs/heads/master@{#420529}

[modify] https://crrev.com/f37aebb917de5c93bf44f93370694b813e2f8ecc/tools/mb/PRESUBMIT.py
[modify] https://crrev.com/f37aebb917de5c93bf44f93370694b813e2f8ecc/tools/mb/mb.py
[modify] https://crrev.com/f37aebb917de5c93bf44f93370694b813e2f8ecc/tools/mb/mb_config.pyl

Status: Fixed (was: Started)
Project Member

Comment 11 by bugdroid1@chromium.org, Dec 6 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3a2f4935b92906c059733e32fa0443dda98ab858

commit 3a2f4935b92906c059733e32fa0443dda98ab858
Author: thakis <thakis@chromium.org>
Date: Tue Dec 06 20:39:21 2016

gn: Remove an import() that's unused after https://codereview.chromium.org/2296953002

BUG= 634184 

Review-Url: https://codereview.chromium.org/2556843003
Cr-Commit-Position: refs/heads/master@{#436702}

[modify] https://crrev.com/3a2f4935b92906c059733e32fa0443dda98ab858/build/config/compiler/BUILD.gn

Sign in to add a comment