Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(90)

Issue 1329833002: Improve mini installer and official build GN flags (Closed)

Created:
5 years, 3 months ago by brettw
Modified:
5 years, 3 months ago
Reviewers:
scottmg
CC:
chromium-reviews, grt+watch_chromium.org, wfh+watch_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Improve mini installer and official build GN flags I went through and did some flag auditing to try to get GN's official build mini_installer target as small as GYP's. It's still 512 bytes too large. Adds the custom mini_installer flags to the "lib" target as well as the main mini_installer target. This removes some imports around exception handling. Sync some official build flags: LTCG should go on all optimized targets in official builds, not just "optimize max" ones. Doing this on optimize max has no effect because we only set optimize_max on individual libraries that need to be fast, rather than anything that gets linked. Convert /OPT:REF to /OPT:ICF. GYP specifies both, but ICF seems to imply REF according to MSDN. Added some missing release mode flags "/d2Zi+", "/Zc:inline", and "/cgthreads:8", which I found missing from GN. Removed a dependency on "setup" from mini installer, this is captured by the dependency on ":archive" (which already includes the setup dependency). Committed: https://crrev.com/7cb45ce734f09f358e62fd982c8fa288673b2ab5 Cr-Commit-Position: refs/heads/master@{#347447}

Patch Set 1 #

Patch Set 2 : #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+24 lines, -4 lines) Patch
M build/config/compiler/BUILD.gn View 3 chunks +13 lines, -2 lines 0 comments Download
M chrome/installer/mini_installer/BUILD.gn View 1 5 chunks +11 lines, -2 lines 2 comments Download

Messages

Total messages: 8 (2 generated)
brettw
5 years, 3 months ago (2015-09-03 20:02:14 UTC) #2
scottmg
lgtm https://codereview.chromium.org/1329833002/diff/20001/chrome/installer/mini_installer/BUILD.gn File chrome/installer/mini_installer/BUILD.gn (right): https://codereview.chromium.org/1329833002/diff/20001/chrome/installer/mini_installer/BUILD.gn#newcode16 chrome/installer/mini_installer/BUILD.gn:16: "/FS", # Preserve previous PDB behavior. This is ...
5 years, 3 months ago (2015-09-03 21:22:25 UTC) #3
brettw
https://codereview.chromium.org/1329833002/diff/20001/chrome/installer/mini_installer/BUILD.gn File chrome/installer/mini_installer/BUILD.gn (right): https://codereview.chromium.org/1329833002/diff/20001/chrome/installer/mini_installer/BUILD.gn#newcode16 chrome/installer/mini_installer/BUILD.gn:16: "/FS", # Preserve previous PDB behavior. I have no ...
5 years, 3 months ago (2015-09-04 16:53:15 UTC) #4
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1329833002/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1329833002/20001
5 years, 3 months ago (2015-09-04 16:53:51 UTC) #6
commit-bot: I haz the power
Committed patchset #2 (id:20001)
5 years, 3 months ago (2015-09-04 18:02:47 UTC) #7
commit-bot: I haz the power
5 years, 3 months ago (2015-09-04 18:03:23 UTC) #8
Message was sent while issue was closed.
Patchset 2 (id:??) landed as
https://crrev.com/7cb45ce734f09f358e62fd982c8fa288673b2ab5
Cr-Commit-Position: refs/heads/master@{#347447}

Powered by Google App Engine
This is Rietveld 408576698