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

Issue 11616021: Revert 173749 (Closed)

Created:
8 years ago by wjia(left Chromium)
Modified:
8 years ago
CC:
chromium-reviews
Visibility:
Public.

Description

Revert 173749 It broke mac asan bots. > Introduce mac_want_real_dsym GYP flag. By default, fake .dSYM files > will be generated for regular builds, and dsymutils will be used to create .dSYMs for ASan builds. > mac_want_real_dsym can be used to override this. > > mac_real_dsym can't be set directly, because it's not a % variable. > According to https://codereview.chromium.org/113999, it can't be made a > % variable now, because of a bug in % handling in GYP that leads to > mac_real_dsym being unconditionally set to 1 for some targets. > > BUG=148383 > > Review URL: https://chromiumcodereview.appspot.com/11587012 TBR=glider@chromium.org Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=173765

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+1 line, -24 lines) Patch
M build/common.gypi View 3 chunks +1 line, -24 lines 0 comments Download

Messages

Total messages: 2 (0 generated)
wjia(left Chromium)
8 years ago (2012-12-18 19:48:06 UTC) #1
Alexander Potapenko
8 years ago (2012-12-18 20:00:23 UTC) #2
LGTM, thanks for taking care

On Tue, Dec 18, 2012 at 11:48 PM,  <wjia@chromium.org> wrote:
> Reviewers: Alexander Potapenko,
>
> Description:
> Revert 173749
> It broke mac asan bots.
>>
>> Introduce mac_want_real_dsym GYP flag. By default, fake .dSYM files
>> will be generated for regular builds, and dsymutils will be used to create
>
> .dSYMs for ASan builds.
>>
>> mac_want_real_dsym can be used to override this.
>
>
>> mac_real_dsym can't be set directly, because it's not a % variable.
>> According to https://codereview.chromium.org/113999, it can't be made a
>> % variable now, because of a bug in % handling in GYP that leads to
>> mac_real_dsym being unconditionally set to 1 for some targets.
>
>
>> BUG=148383
>
>
>> Review URL: https://chromiumcodereview.appspot.com/11587012
>
>
> TBR=glider@chromium.org
>
> Please review this at https://codereview.chromium.org/11616021/
>
> SVN Base: svn://svn.chromium.org/chrome/trunk/src/
>
> Affected files:
>   M     build/common.gypi
>
>
> Index: build/common.gypi
> ===================================================================
> --- build/common.gypi   (revision 173762)
> +++ build/common.gypi   (working copy)
> @@ -252,10 +252,6 @@
>        # Notifications are compiled in by default. Set to 0 to disable.
>        'notifications%' : 1,
>
> -      # Use dsymutil to generate real .dSYM files on Mac. The default is 0
> for
> -      # regular builds and 1 for ASan builds.
> -      'mac_want_real_dsym%': 'default',
> -
>        # If this is set, the clang plugins used on the buildbot will be
> used.
>        # Run tools/clang/scripts/update.sh to make sure they are compiled.
>        # This causes 'clang_chrome_plugins_flags' to be set.
> @@ -681,7 +677,6 @@
>      'input_speech%': '<(input_speech)',
>      'notifications%': '<(notifications)',
>      'clang_use_chrome_plugins%': '<(clang_use_chrome_plugins)',
> -    'mac_want_real_dsym%': '<(mac_want_real_dsym)',
>      'asan%': '<(asan)',
>      'tsan%': '<(tsan)',
>      'tsan_blacklist%': '<(tsan_blacklist)',
> @@ -3367,27 +3362,9 @@
>            # variables that are intended to be set to different values in
>            # different targets, like these.
>            'mac_pie': 1,        # Most executables can be
> position-independent.
> +          'mac_real_dsym': 0,  # Fake .dSYMs are fine in most cases.
>            # Strip debugging symbols from the target.
>            'mac_strip': '<(mac_strip_release)',
> -          'conditions': [
> -            ['asan==1', {
> -              'conditions': [
> -                ['mac_want_real_dsym=="default"', {
> -                  'mac_real_dsym': 1,
> -                }, {
> -                  'mac_real_dsym': '<(mac_want_real_dsym)'
> -                }],
> -              ],
> -            }, {
> -              'conditions': [
> -                ['mac_want_real_dsym=="default"', {
> -                  'mac_real_dsym': 0, # Fake .dSYMs are fine in most cases.
> -                }, {
> -                  'mac_real_dsym': '<(mac_want_real_dsym)'
> -                }],
> -              ],
> -            }],
> -          ],
>          },
>          'xcode_settings': {
>            'GCC_DYNAMIC_NO_PIC': 'NO',               # No -mdynamic-no-pic
>
>



-- 
Alexander Potapenko
Software Engineer
Google Moscow

Powered by Google App Engine
This is Rietveld 408576698