Add a gyp flag to allow use of the system's version of libexpat

Add a gyp flag to allow use of the system's version of libexpat

Hook up this flag for Android and enable it when building with the Android
build system.


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=145854

[Evan Martin, 2012-07-09 16:29:06]

I don't work on Chrome anymore.  I think thestig@chromium.org would be a
good reviewer.


On Tue, Jul 3, 2012 at 7:10 AM, <steveblock@chromium.org> wrote:

> Reviewers: ncarter, Evan Martin,
>
> Description:
> Add a gyp flag to allow use of the system's version of libexpat
>
> Hook up this flag for Android and enable it when building with the Android
> build system.
>
>
> Please review this at
http://codereview.chromium.**org/10704079/<http://codereview.chromium.org/107...
>
> SVN Base:
svn://svn.chromium.org/chrome/**trunk/src<http://svn.chromium.org/chrome/trunk/src>
>
> Affected files:
>   M build/common.gypi
>   M third_party/expat/expat.gyp
>
>
> Index: build/common.gypi
> diff --git a/build/common.gypi b/build/common.gypi
> index 401ceb298b4bf22f978b2019300220**f0747dd898..**
> b076e610045136e184708f3348bcd6**cbcbd3c78e 100644
> --- a/build/common.gypi
> +++ b/build/common.gypi
> @@ -942,13 +942,13 @@
>            }],
>          ],
>          # TODO(steveblock): Investigate using the system versions of
> sqlite,
> -        # libjpeg, expat and stlport.
> +        # libjpeg and stlport.
>          # Enable to use system sqlite.
>          'use_system_sqlite%': 0,  # '<(android_build_type)',
>          # Enable to use system libjpeg.
>          'use_system_libjpeg%': 0,  # '<(android_build_type)',
> -        # Enable to use the system libexpat.
> -        'use_system_libexpat%': 0,  # '<(android_build_type)',
> +        # Enable to use the system expat.
> +        'use_system_expat%': '<(android_build_type)',
>          # Enable to use the system stlport, otherwise statically
>          # link the NDK one?
>          'use_system_stlport%': 0,  # '<(android_build_type)',
> Index: third_party/expat/expat.gyp
> diff --git a/third_party/expat/expat.gyp b/third_party/expat/expat.gyp
> index 5f9d1919906104f1f4c82a59486428**2d08edef87..**
> 5c7eda9f38b7df5ac64ee778e5176d**7e2f82a2c8 100644
> --- a/third_party/expat/expat.gyp
> +++ b/third_party/expat/expat.gyp
> @@ -3,6 +3,17 @@
>  # found in the LICENSE file.
>
>  {
> +  'variables': {
> +    'conditions': [
> +      # On Linux, we implicitly already depend on expat via fontconfig;
> +      # let's not pull it in twice.
> +      ['os_posix == 1 and OS != "mac" and OS != "android"', {
> +        'use_system_expat%': 1,
> +      }, {
> +        'use_system_expat%': 0,
> +      }],
> +    ],
> +  },
>    'target_defaults': {
>      'defines': [
>        '_LIB',
> @@ -11,13 +22,9 @@
>      'include_dirs': [
>        'files/lib',
>      ],
> -    'dependencies': [
> -    ]
>    },
>    'conditions': [
> -    ['os_posix == 1 and OS != "mac" and OS != "android"', {
> -      # On Linux, we implicitly already depend on expat via fontconfig;
> -      # let's not pull it in twice.
> +    ['use_system_expat == 1', {
>        'targets': [
>          {
>            'target_name': 'expat',
> @@ -27,9 +34,18 @@
>                '-lexpat',
>              ],
>            },
> +          'conditions': [
> +            ['OS=="android"', {
> +              'direct_dependent_settings': {
> +                'include_dirs': [
> +                  '<(android_src)/external/**expat/lib',
> +                ],
> +              },
> +            }],
> +          ],
>          },
>        ],
> -    }, {  # OS != linux
> +    }, {  # else: use_system_expat != 1
>        'targets': [
>          {
>            'target_name': 'expat',
>
>
>

[Lei Zhang, 2012-07-09 23:05:33]

lgtm

http://codereview.chromium.org/10704079/diff/1/build/common.gypi
File build/common.gypi (right):

http://codereview.chromium.org/10704079/diff/1/build/common.gypi#newcode951
build/common.gypi:951: 'use_system_expat%': '<(android_build_type)',
Is |android_build_type| ever going to be non-zero? I don't know the future plans
for this variable.

[commit-bot: I haz the power, 2012-07-10 07:42:20]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/steveblock@chromium.org/10704079/1

[commit-bot: I haz the power, 2012-07-10 09:02:32]

Change committed as 145854