Isolate check_internal.py to the .gypi files that need it.

Isolate check_internal.py to the .gypi files that need it.

This works around the bug we've had where on some Windows machines, one of the invocations of check_internal.py mysteriously causes a file to open up in Visual Studio.  We have no idea why, but this should avoid the problem.  See details in bug.

BUG=241208
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=200514

[M-A Ruel, 2013-05-08 18:11:23]

https://codereview.chromium.org/12330155/diff/1/google_apis/determine_use_off...
File google_apis/determine_use_official_keys.gypi (right):

https://codereview.chromium.org/12330155/diff/1/google_apis/determine_use_off...
google_apis/determine_use_official_keys.gypi:28: 'defines':
['USE_OFFICIAL_GOOGLE_API_KEYS=1'],
align -2 :)

[Jói, 2013-05-15 20:58:45]

Marc-Antoine,

Please review, the current version seems to work.  Since (I think) it shouldn't
trigger the exact command-line that triggers the bug, this is hopefully
sufficient to work around the issue.

This also enables more minimal rebuilds when people change the setting of the
use_official_google_api_keys flag.

Cheers,
Jói

[Jói, 2013-05-15 20:59:05]

Updated issue subject.

[Jói, 2013-05-15 21:00:13]

FYI I updated the issue subject so this is now in a new thread.

On Wed, May 15, 2013 at 1:58 PM,  <joi@chromium.org> wrote:
> Reviewers: Marc-Antoine Ruel,
>
> Message:
> Marc-Antoine,
>
> Please review, the current version seems to work.  Since (I think) it
> shouldn't
> trigger the exact command-line that triggers the bug, this is hopefully
> sufficient to work around the issue.
>
> This also enables more minimal rebuilds when people change the setting of
> the
> use_official_google_api_keys flag.
>
> Cheers,
> Jói
>
>
> Description:
> Isolate check_internal.py to the .gypi files that need it.
>
> This works around the bug we've had where on some Windows machines, one of
> the
> invocations of check_internal.py mysteriously causes a file to open up in
> Visual
> Studio.  We have no idea why, but this should avoid the problem.  See
> details in
> bug.
>
> BUG=241208
>
> Please review this at https://codereview.chromium.org/12330155/
>
> SVN Base: svn://svn.chromium.org/chrome/trunk/src
>
> Affected files:
>   M build/common.gypi
>   M chrome/chrome_tests_unit.gypi
>   A google_apis/determine_use_official_keys.gypi
>   M google_apis/google_apis.gyp
>
>
> Index: build/common.gypi
> diff --git a/build/common.gypi b/build/common.gypi
> index
>
d3b0b279c29e8e11243a1f4884d9d6a61333f65a..df540986e1a6e9f8f920bf8cc96f7d95f3ae20a2
> 100644
> --- a/build/common.gypi
> +++ b/build/common.gypi
> @@ -675,22 +675,33 @@
>        # for the HRTF panner in WebAudio.
>        'use_concatenated_impulse_responses': 1,
>
> -      # Set this to 1 to use the Google-internal file containing
> -      # official API keys for Google Chrome even in a developer build.
> -      # Setting this variable explicitly to 1 will cause your build to
> -      # fail if the internal file is missing.
> +      # You can set the variable 'use_official_google_api_keys' to 1
> +      # to use the Google-internal file containing official API keys
> +      # for Google Chrome even in a developer build.  Setting this
> +      # variable explicitly to 1 will cause your build to fail if the
> +      # internal file is missing.
>        #
> -      # Set this to 0 to not use the internal file, even when it
> -      # exists in your checkout.
> +      # The variable is documented here, but not handled in this file;
> +      # see //google_apis/determine_use_official_keys.gypi for the
> +      # implementation.
>        #
> -      # Leave set to 2 to have this variable implicitly set to 1 if
> -      # you have src/google_apis/internal/google_chrome_api_keys.h in
> -      # your checkout, and implicitly set to 0 if not.
> +      # Set the variable to 0 to not use the internal file, even when
> +      # it exists in your checkout.
>        #
> -      # Note that official builds always behave as if this variable
> +      # Leave it unset in your include.gypi to have the variable
> +      # implicitly set to 1 if you have
> +      # src/google_apis/internal/google_chrome_api_keys.h in your
> +      # checkout, and implicitly set to 0 if not.
> +      #
> +      # Note that official builds always behave as if the variable
>        # was explicitly set to 1, i.e. they always use official keys,
>        # and will fail to build if the internal file is missing.
> -      'use_official_google_api_keys%': 2,
> +      #
> +      # NOTE: You MUST NOT explicitly set the variable to 2 in your
> +      # include.gypi or by other means. Due to subtleties of GYP, this
> +      # is not the same as leaving the variable unset, even though its
> +      # default value in
> +      # //google_apis/determine_use_official_keys.gypi is 2.
>
>        # Set these to bake the specified API keys and OAuth client
>        # IDs/secrets into your build.
> @@ -797,7 +808,6 @@
>      'enable_app_list%': '<(enable_app_list)',
>      'use_default_render_theme%': '<(use_default_render_theme)',
>      'enable_settings_app%': '<(enable_settings_app)',
> -    'use_official_google_api_keys%': '<(use_official_google_api_keys)',
>      'google_api_key%': '<(google_api_key)',
>      'google_default_client_id%': '<(google_default_client_id)',
>      'google_default_client_secret%': '<(google_default_client_secret)',
> @@ -1094,15 +1104,6 @@
>          # TODO(jschuh): crbug.com/177664 Investigate Win64 pyauto build.
>          'python_arch%': 'ia32',
>        }],
> -      # If use_official_google_api_keys is already set (to 0 or 1), we
> -      # do none of the implicit checking.  If it is set to 1 and the
> -      # internal keys file is missing, the build will fail at compile
> -      # time.  If it is set to 0 and keys are not provided by other
> -      # means, a warning will be printed at compile time.
> -      ['use_official_google_api_keys==2', {
> -        'use_official_google_api_keys%':
> -            '<!(python <(DEPTH)/google_apis/build/check_internal.py
> <(DEPTH)/google_apis/internal/google_chrome_api_keys.h)',
> -      }],
>        ['os_posix==1 and OS!="mac" and OS!="ios"', {
>          # Figure out the python architecture to decide if we build pyauto.
>          'python_arch%': '<!(<(DEPTH)/build/linux/python_arch.sh
> <(sysroot)/usr/<(system_libdir)/libpython<(python_ver).so.1.0)',
> Index: chrome/chrome_tests_unit.gypi
> diff --git a/chrome/chrome_tests_unit.gypi b/chrome/chrome_tests_unit.gypi
> index
>
76fee091a83bba1fef2605fbdf741640740a2309..bdea7eb34069e53a5d95153f57a7b76b03d3ced4
> 100644
> --- a/chrome/chrome_tests_unit.gypi
> +++ b/chrome/chrome_tests_unit.gypi
> @@ -445,6 +445,11 @@
>          'chrome_resources.gyp:chrome_resources',
>          'chrome_resources.gyp:chrome_strings',
>        ],
> +      # TODO(joi): Remove this section once the google_apis tests are
> +      # a separate unit test binary.
> +      'includes': [
> +        '../google_apis/determine_use_official_keys.gypi',
> +      ],
>        'include_dirs': [
>          '..',
>        ],
> @@ -2389,11 +2394,6 @@
>              ['exclude', '^browser/importer/'],
>            ],
>          }],
> -        # TODO(joi): Remove from this file once separate
> -        # google_apis_unittests target is established.
> -        ['use_official_google_api_keys==1', {
> -          'defines': ['USE_OFFICIAL_GOOGLE_API_KEYS=1'],
> -        }],
>          ['enable_managed_users!=1', {
>            'sources/': [
>              ['exclude', '^browser/managed_mode/'],
> Index: google_apis/determine_use_official_keys.gypi
> diff --git a/google_apis/determine_use_official_keys.gypi
> b/google_apis/determine_use_official_keys.gypi
> new file mode 100644
> index
>
0000000000000000000000000000000000000000..d9f11164af7f1f93ba24f576be46ce57d7503224
> --- /dev/null
> +++ b/google_apis/determine_use_official_keys.gypi
> @@ -0,0 +1,37 @@
> +# Copyright (c) 2013 The Chromium Authors. All rights reserved.
> +# Use of this source code is governed by a BSD-style license that can be
> +# found in the LICENSE file.
> +
> +# Include this .gypi in your target to dynamically set the
> +# use_official_google_api_keys variable (unless it is already
> +# explicitly set) and the associated preprocessor define.
> +
> +{
> +  'variables': {
> +    'variables': {
> +      # See documentation of this variable in //build/common.gypi.
> +      'use_official_google_api_keys%': 2,
> +    },
> +
> +    # Copy conditionally-set variables out one scope.
> +    'use_official_google_api_keys%': '<(use_official_google_api_keys)',
> +
> +    'conditions': [
> +      # If use_official_google_api_keys is already set (to 0 or 1), we
> +      # do none of the implicit checking.  If it is set to 1 and the
> +      # internal keys file is missing, the build will fail at compile
> +      # time.  If it is set to 0 and keys are not provided by other
> +      # means, a warning will be printed at compile time.
> +      ['use_official_google_api_keys==2', {
> +          'use_official_google_api_keys%':
> +            '<!(python <(DEPTH)/google_apis/build/check_internal.py
> <(DEPTH)/google_apis/internal/google_chrome_api_keys.h)',
> +        }],
> +    ]
> +  },
> +
> +  'conditions': [
> +    ['use_official_google_api_keys==1', {
> +        'defines': ['USE_OFFICIAL_GOOGLE_API_KEYS=1'],
> +      }],
> +  ],
> +}
> Index: google_apis/google_apis.gyp
> diff --git a/google_apis/google_apis.gyp b/google_apis/google_apis.gyp
> index
>
dd346a83c12a6446268b9c6b1f158e2a2055f606..9a6ec478b6582f97aa1a95988b27a5ed313f7535
> 100644
> --- a/google_apis/google_apis.gyp
> +++ b/google_apis/google_apis.gyp
> @@ -13,6 +13,9 @@
>      {
>        'target_name': 'google_apis',
>        'type': 'static_library',
> +      'includes': [
> +        'determine_use_official_keys.gypi',
> +      ],
>        'dependencies': [
>          '../base/base.gyp:base',
>          '../crypto/crypto.gyp:crypto',
> @@ -45,9 +48,6 @@
>
> 'GOOGLE_CLIENT_SECRET_REMOTING="<(google_default_client_secret)"',
>            ]
>          }],
> -        ['use_official_google_api_keys==1', {
> -          'defines': ['USE_OFFICIAL_GOOGLE_API_KEYS=1'],
> -        }],
>        ],
>        'sources': [
>          'gaia/gaia_auth_consumer.cc',
>
>

[M-A Ruel, 2013-05-16 02:00:00]

lgtm

[commit-bot: I haz the power, 2013-05-16 02:00:54]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/joi@chromium.org/12330155/4001

[commit-bot: I haz the power, 2013-05-16 11:39:56]

Change committed as 200514

[iannucci, 2013-05-28 22:59:13]

On 2013/05/16 11:39:56, I haz the power (commit-bot) wrote:
> Change committed as 200514

If this is really the only thing holding us back from python 2.7 on windows, can
we put in a merge request for this to 28 and 27? I talked to laforge and he said
it would be 'not unreasonable' :)

Even if we only get this in 28, it cuts our waiting period down to 4 weeks
instead of 10...

[M-A Ruel, 2013-05-28 23:48:03]

On 2013/05/28 22:59:13, iannucci wrote:
> On 2013/05/16 11:39:56, I haz the power (commit-bot) wrote:
> > Change committed as 200514
> 
> If this is really the only thing holding us back from python 2.7 on windows,
can
> we put in a merge request for this to 28 and 27? I talked to laforge and he
said
> it would be 'not unreasonable' :)
> 
> Even if we only get this in 28, it cuts our waiting period down to 4 weeks
> instead of 10...

Please do if you can. I just couldn't allocate time to expedite this.