Resolving build issue while disabling webrtc for android

Resolving build issue while disabling webrtc for android

Resolved the build issue for android, when disabling webrtc
./build/gyp_chromium -DOS=android -Denable_webrtc=0

BUG=432389
Committed: https://crrev.com/0df38069d72206488569038c0702bed54bd2c5be
Cr-Commit-Position: refs/heads/master@{#303813}

[MRV, 2014-11-12 03:33:47]

mohan.reddy@samsung.com changed reviewers:
+ serya@chromium.org

[MRV, 2014-11-12 03:33:47]

PTAL

[MRV, 2014-11-12 04:42:49]

The CQ bit was checked by mohan.reddy@samsung.com

[commit-bot: I haz the power, 2014-11-12 04:42:59]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/719453005/1

[commit-bot: I haz the power, 2014-11-12 04:43:01]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-11-12 04:43:02]

No LGTM from a valid reviewer yet. Only full committers are accepted.
Even if an LGTM may have been provided, it was from a non-committer,
_not_ a full super star committer.
See http://www.chromium.org/getting-involved/become-a-committer
Note that this has nothing to do with OWNERS files.

[SeRya, 2014-11-12 07:30:51]

On 2014/11/12 04:43:02, I haz the power (commit-bot) wrote:
> No LGTM from a valid reviewer yet. Only full committers are accepted.
> Even if an LGTM may have been provided, it was from a non-committer,
> _not_ a full super star committer.
> See http://www.chromium.org/getting-involved/become-a-committer
> Note that this has nothing to do with OWNERS files.

Conditional dependency like this must cause linkage error if the feature's
disabled.

I think much better fix would be to change condition in buila/all.gyp:

https://code.google.com/p/chromium/codesearch#chromium/src/build/all.gyp&l=848

          'conditions': [
            ['"<(libpeer_target_type)"=="static_library"', {
              'dependencies': [
                '../components/devtools_bridge.gyp:devtools_bridge_tests_apk',
                '../components/devtools_bridge.gyp:devtools_bridge_tests2_apk',
              ],
            }],
          ],

'"<(libpeer_target_type)"=="static_library"' -> 'enable_webrtc==1 and
<(libpeer_target_type)"=="static_library"'

[MRV, 2014-11-12 07:48:27]

On 2014/11/12 07:30:51, SeRya wrote:
> On 2014/11/12 04:43:02, I haz the power (commit-bot) wrote:
> > No LGTM from a valid reviewer yet. Only full committers are accepted.
> > Even if an LGTM may have been provided, it was from a non-committer,
> > _not_ a full super star committer.
> > See http://www.chromium.org/getting-involved/become-a-committer
> > Note that this has nothing to do with OWNERS files.
> 
> Conditional dependency like this must cause linkage error if the feature's
> disabled.
> 
> I think much better fix would be to change condition in buila/all.gyp:
> 
> https://code.google.com/p/chromium/codesearch#chromium/src/build/all.gyp&l=848
> 
>           'conditions': [
>             ['"<(libpeer_target_type)"=="static_library"', {
>               'dependencies': [
>                 '../components/devtools_bridge.gyp:devtools_bridge_tests_apk',
>                
'../components/devtools_bridge.gyp:devtools_bridge_tests2_apk',
>               ],
>             }],
>           ],
> 
> '"<(libpeer_target_type)"=="static_library"' -> 'enable_webrtc==1 and
> <(libpeer_target_type)"=="static_library"'

Thanks Mr. Serya, Thanks for feedback.
I have incorporated the changes as you suggested.
PTAL.

[SeRya, 2014-11-12 07:59:00]

lgtm

[MRV, 2014-11-12 08:57:15]

The CQ bit was checked by mohan.reddy@samsung.com

[MRV, 2014-11-12 08:57:38]

On 2014/11/12 07:59:00, SeRya wrote:
> lgtm

Thanks Mr. Serya for reviewing my patch.

[commit-bot: I haz the power, 2014-11-12 08:58:26]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/719453005/20001

[commit-bot: I haz the power, 2014-11-12 10:16:51]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2014-11-12 10:17:30]

Patchset 2 (id:??) landed as
https://crrev.com/0df38069d72206488569038c0702bed54bd2c5be
Cr-Commit-Position: refs/heads/master@{#303813}