New issue
Advanced search Search tips

Issue 622889 link

Starred by 4 users

Issue metadata

Status: Fixed
Owner:
Closed: Jul 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 1
Type: Bug


Sign in to add a comment

Build flag differences between Mac GN and GYP

Project Member Reported by rsesek@chromium.org, Jun 23 2016

Issue description

I investigated the difference in //chrome targets using a highly modified version of //tools/gin/bin/gyp_flag_compare.py. Results of my investigation:

Flags in GYP that are missing in GN:

  -gdwarf-2 globally (may be cause of  issue 622406 )

  -fstrict-aliasing in //v8

  -Wglobal-constructors in //third_party/WebKit
  -Wall in //third_party/icu
  -Wextra in //third_party/{libyuv,sfntly,webrtc}
  -Wno-char-subscripts globally
  -Wobjc-missing-property-synthesis globally
  -Wpartial-availability globally

  -D__ASSERT_MACROS_DEFINE_VERSIONS_WITHOUT_UNDERSCORE=0 globally


Flags in GN that are not present in GYP:

  Globally:
    -fdata-sections is ELF-only, remove
    -ffunction-sections is ELF-only, remove
    -fno-ident
    -fstack-protector is intentional (?) https://codereview.chromium.org/2029633002/

  -frtti in //third_party/icu

  -Wno-deprecated-declarations in most of //chrome (see  https://crbug.com/622481#c10 )
  -Wno-implicit-function-declaration in //third_party/libsrtp
  -Wno-incompatible-pointer-types in //third_party/{libwebp,usrsctp}
  -Wno-logical-op-parentheses in //third_party/ffmpeg
  -Wno-newline-eof in //third_party/libaddressinput
  -Wno-switch in //third_party/icu
  -Wno-uninitialized in //third_party/webrtc
  -Wno-unused-const-variable in //third_party/icu
  -Wno-unused-function in //third_party/icu
  -Wno-unused-variable in //third_party/{libyuv,sfntly,webrtc}


The following targets are strange:

  //base
  //gpu/command_buffer
  //ipc
  //mojo
  //ppapi
  //skia
  //third_party/icu
  //third_party/skia
  //ui/gfx/geometry
  //ui/gfx/range
  //url

  Missing flags from GYP:
    -std=c++11
    -Wheader-hygiene
    -Wstring-conversion

  New flags from GN:
    -fomit-frame-pointer
    -fasynchronous-unwind-tables
    -Os
    -m64
 

Comment 1 by rsesek@chromium.org, Jun 23 2016

Components: Build
Labels: Proj-GN-Migration

Comment 2 by rsesek@chromium.org, Jun 23 2016

The "strange" targets are the NaCl IRT build.

Comment 3 by rsesek@chromium.org, Jun 23 2016

-Wpartial-availability globally is an artifact of NaCl IRT build as well.
Project Member

Comment 4 by bugdroid1@chromium.org, Jun 24 2016

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

commit adfae52965b56ca7a17fbd60149f78be801691f1
Author: rsesek <rsesek@chromium.org>
Date: Fri Jun 24 01:56:39 2016

[Mac/GN] Use -gdwarf-2 instead of -g2 in //build/config/compiler:symbols.

This matches GYP.

BUG= 622889 , 622406 
R=dpranke@chromium.org

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

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

Project Member

Comment 5 by bugdroid1@chromium.org, Jun 24 2016

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

commit 054b2e4f537d040e12f874c633be75462d93b406
Author: rsesek <rsesek@chromium.org>
Date: Fri Jun 24 02:11:38 2016

[Mac/GN] Do not use ELF-only cflags on Mac.

This removes -fdata-sections, -ffunction-sections, and -fno-ident.

BUG= 622889 
R=dpranke@chromium.org

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

[modify] https://crrev.com/054b2e4f537d040e12f874c633be75462d93b406/build/config/compiler/BUILD.gn

Project Member

Comment 6 by bugdroid1@chromium.org, Jun 24 2016

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

commit 5d8c668952c6972eb1f833fddccef7d2fd66048b
Author: rsesek <rsesek@chromium.org>
Date: Fri Jun 24 03:24:58 2016

[Mac/iOS/GN] Remove bad -Wno-deprecated-declarations from //third_party/google_toolbox_for_mac public_config.

BUG= 622889 , 622481 
R=thakis@chromium.org

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

[modify] https://crrev.com/5d8c668952c6972eb1f833fddccef7d2fd66048b/chrome/browser/ui/BUILD.gn
[modify] https://crrev.com/5d8c668952c6972eb1f833fddccef7d2fd66048b/third_party/google_toolbox_for_mac/BUILD.gn

Comment 7 by rsesek@chromium.org, Jun 24 2016

Flag differences in these components are explicit in the GN files:

  //third_party/ffmpeg
  //third_party/icu
  //third_party/libaddressinput
  //third_party/libyuv
  //third_party/sfntly
  //third_party/usrsctp
  //third_party/webrtc

Not having -Wno-char-subscripts globally seems like a positive (one fewer disabled warnings). 

https://codereview.chromium.org/2092133002/ will fix -Wobjc-missing-property-synthesis and the __ASSERT define.

The one mystery is -Wglobal-constructors, which WebKit seems to enable. Will investigate that further.
Project Member

Comment 8 by bugdroid1@chromium.org, Jun 24 2016

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

commit 227e59731da9ca04c4ceb9c6cb4725a07e15c199
Author: rsesek <rsesek@chromium.org>
Date: Fri Jun 24 21:50:40 2016

[Mac/GN] Add missing -Wobjc-missing-property-synthesis.

This also adds a missing __ASSERT_MACROS_DEFINE_VERSIONS_WITHOUT_UNDERSCORE define.

BUG= 622889 
R=dpranke@chromium.org
TBR=rohitrao@chromium.org

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

[modify] https://crrev.com/227e59731da9ca04c4ceb9c6cb4725a07e15c199/build/config/compiler/BUILD.gn
[modify] https://crrev.com/227e59731da9ca04c4ceb9c6cb4725a07e15c199/build/config/mac/BUILD.gn
[modify] https://crrev.com/227e59731da9ca04c4ceb9c6cb4725a07e15c199/third_party/class-dump/BUILD.gn

Project Member

Comment 10 by bugdroid1@chromium.org, Jun 28 2016

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

commit 9f1375ea1bfb9f35e48b69f4c10c8ffa3233ca2a
Author: Robert Sesek <rsesek@chromium.org>
Date: Tue Jun 28 19:23:29 2016

Refactor //tools/gn/bin/gyp_flag_compare.py to be usable in interactive Python.

When comparing large targets like //chrome, it is nearly impossible to deal with
the mountainous 46,000 lines of output the executable script produces. With an
interactive environment, it is much easier to compare differences and drill-down
into specific issues.

BUG= 622889 
R=scottmg@chromium.org

Review URL: https://codereview.chromium.org/2108683002 .

Cr-Commit-Position: refs/heads/master@{#402512}

[modify] https://crrev.com/9f1375ea1bfb9f35e48b69f4c10c8ffa3233ca2a/tools/gn/bin/gyp_flag_compare.py

Project Member

Comment 11 by bugdroid1@chromium.org, Jun 28 2016

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

commit 74af4d66235496370bd173c6139314e8adfecb2e
Author: khushalsagar <khushalsagar@chromium.org>
Date: Tue Jun 28 19:55:28 2016

Revert of Refactor //tools/gn/bin/gyp_flag_compare.py to be usable in interactive Python. (patchset #1 id:1 of https://codereview.chromium.org/2108683002/ )

Reason for revert:
A significant number of bots are broken after this landed. This is a speculative revert. Please re-land in case this was not the error.

Original issue's description:
> Refactor //tools/gn/bin/gyp_flag_compare.py to be usable in interactive Python.
>
> When comparing large targets like //chrome, it is nearly impossible to deal with
> the mountainous 46,000 lines of output the executable script produces. With an
> interactive environment, it is much easier to compare differences and drill-down
> into specific issues.
>
> BUG= 622889 
> R=scottmg@chromium.org
>
> Committed: https://chromium.googlesource.com/chromium/src/+/9f1375ea1bfb9f35e48b69f4c10c8ffa3233ca2a

TBR=scottmg@chromium.org,rsesek@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 622889 

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

[modify] https://crrev.com/74af4d66235496370bd173c6139314e8adfecb2e/tools/gn/bin/gyp_flag_compare.py

Project Member

Comment 12 by bugdroid1@chromium.org, Jun 28 2016

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

commit db530b158baf864d61e74ef6d74a8938038ac7b0
Author: rsesek <rsesek@chromium.org>
Date: Tue Jun 28 20:30:30 2016

Reland of actor //tools/gn/bin/gyp_flag_compare.py to be usable in interactive Python. (patchset #1 id:1 of https://codereview.chromium.org/2104083002/ )

Reason for revert:
Revert unwarranted.

Original issue's description:
> Revert of Refactor //tools/gn/bin/gyp_flag_compare.py to be usable in interactive Python. (patchset #1 id:1 of https://codereview.chromium.org/2108683002/ )
>
> Reason for revert:
> A significant number of bots are broken after this landed. This is a speculative revert. Please re-land in case this was not the error.
>
> Original issue's description:
> > Refactor //tools/gn/bin/gyp_flag_compare.py to be usable in interactive Python.
> >
> > When comparing large targets like //chrome, it is nearly impossible to deal with
> > the mountainous 46,000 lines of output the executable script produces. With an
> > interactive environment, it is much easier to compare differences and drill-down
> > into specific issues.
> >
> > BUG= 622889 
> > R=scottmg@chromium.org
> >
> > Committed: https://chromium.googlesource.com/chromium/src/+/9f1375ea1bfb9f35e48b69f4c10c8ffa3233ca2a
>
> TBR=scottmg@chromium.org,rsesek@chromium.org
> # Skipping CQ checks because original CL landed less than 1 days ago.
> NOPRESUBMIT=true
> NOTREECHECKS=true
> NOTRY=true
> BUG= 622889 
>
> Committed: https://crrev.com/74af4d66235496370bd173c6139314e8adfecb2e
> Cr-Commit-Position: refs/heads/master@{#402514}

TBR=scottmg@chromium.org,khushalsagar@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 622889 

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

[modify] https://crrev.com/db530b158baf864d61e74ef6d74a8938038ac7b0/tools/gn/bin/gyp_flag_compare.py

Project Member

Comment 13 by bugdroid1@chromium.org, Jun 28 2016

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

commit 99679aa7b04b20488f5e6669a74ddcc4ddbd8690
Author: rsesek <rsesek@chromium.org>
Date: Tue Jun 28 21:24:17 2016

[GN] Add //build/config/compiler:wexit_time_destructors everywhere it is in GYP.

BUG= 622889 
R=brettw@chromium.org

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

[modify] https://crrev.com/99679aa7b04b20488f5e6669a74ddcc4ddbd8690/base/BUILD.gn
[modify] https://crrev.com/99679aa7b04b20488f5e6669a74ddcc4ddbd8690/chrome/BUILD.gn
[modify] https://crrev.com/99679aa7b04b20488f5e6669a74ddcc4ddbd8690/chrome/browser/extensions/BUILD.gn
[modify] https://crrev.com/99679aa7b04b20488f5e6669a74ddcc4ddbd8690/chrome/common/BUILD.gn
[modify] https://crrev.com/99679aa7b04b20488f5e6669a74ddcc4ddbd8690/chrome/renderer/BUILD.gn
[modify] https://crrev.com/99679aa7b04b20488f5e6669a74ddcc4ddbd8690/chrome/service/BUILD.gn
[modify] https://crrev.com/99679aa7b04b20488f5e6669a74ddcc4ddbd8690/chrome/utility/BUILD.gn
[modify] https://crrev.com/99679aa7b04b20488f5e6669a74ddcc4ddbd8690/components/content_settings/core/browser/BUILD.gn
[modify] https://crrev.com/99679aa7b04b20488f5e6669a74ddcc4ddbd8690/components/content_settings/core/common/BUILD.gn
[modify] https://crrev.com/99679aa7b04b20488f5e6669a74ddcc4ddbd8690/content/BUILD.gn
[modify] https://crrev.com/99679aa7b04b20488f5e6669a74ddcc4ddbd8690/content/public/browser/BUILD.gn
[modify] https://crrev.com/99679aa7b04b20488f5e6669a74ddcc4ddbd8690/content/public/child/BUILD.gn
[modify] https://crrev.com/99679aa7b04b20488f5e6669a74ddcc4ddbd8690/google_apis/gcm/BUILD.gn
[modify] https://crrev.com/99679aa7b04b20488f5e6669a74ddcc4ddbd8690/net/BUILD.gn
[modify] https://crrev.com/99679aa7b04b20488f5e6669a74ddcc4ddbd8690/storage/common/BUILD.gn
[modify] https://crrev.com/99679aa7b04b20488f5e6669a74ddcc4ddbd8690/third_party/WebKit/Source/web/BUILD.gn

Labels: Proj-GN-Migration-Ship
Blocking: -621679 62167
Labels: -Proj-GN-Migration
I'm going to try tracking ship-blocking things via the label "Proj-GN-Migration-Ship" (instead of Proj-GN-Migration) as suggested w/ a conversation w/ laforge@ and the monorail folks a while ago.

Querying for "Proj=GN-Migration%" should still return bugs for both labels, and this will allow us to filter out rollup things more easily, so that we can use blocking just for things that are truly blocking other bugs.
Blocking: -431177 -62167
Blockedon: 626064
Blockedon: 626065
Blockedon: 626066
Blockedon: 626067
Blockedon: 626070
Blockedon: 626072
Blockedon: 626078
Blockedon: 626084
Bugs filed for all the remaining differences.
Status: Fixed (was: Assigned)
This is done except for the two sub-bugs, which only cover warnings.

Sign in to add a comment