Guard dbus component and config with assert(use_dbus)

Guard dbus component and config with assert(use_dbus)

Often times dbus dependencies get added without proper guards.
By guarding the component with assert(use_dbus) it forces proper guarding
when depending on dbus. This prevents downstream breakages in
chromecast.

BUG=internal b/28962923

Committed: https://crrev.com/a12e5a5b7a36268c6196d0ab185a0d8f6cf1441c
Cr-Commit-Position: refs/heads/master@{#396948}

[mbjorge, 2016-05-25 21:30:49]

Description was changed from

==========
Wrap dbus component and config with if (use_dbus)

Often times dbus dependencies get added without proper guards.
By wrapping the component with if (use_dbus) it forces proper guarding
when depending on dbus. This prevents downstream breakages in
chromecast.

BUG=
==========

to

==========
Wrap dbus component and config with if (use_dbus)

Often times dbus dependencies get added without proper guards.
By wrapping the component with if (use_dbus) it forces proper guarding
when depending on dbus. This prevents downstream breakages in
chromecast.

BUG=internal b/28962923
==========

[mbjorge, 2016-05-25 21:30:49]

mbjorge@chromium.org changed reviewers:
+ brettw@chromium.org, slan@chromium.org

[slan, 2016-05-25 22:35:19]

Just nits. Thanks!

https://codereview.chromium.org/2009983002/diff/1/chrome/browser/BUILD.gn
File chrome/browser/BUILD.gn (right):

https://codereview.chromium.org/2009983002/diff/1/chrome/browser/BUILD.gn#new...
chrome/browser/BUILD.gn:873: configs += [ "//build/config/linux/dbus" ]
nit: rm - we get this for free via //dbus.

https://codereview.chromium.org/2009983002/diff/1/dbus/BUILD.gn
File dbus/BUILD.gn (right):

https://codereview.chromium.org/2009983002/diff/1/dbus/BUILD.gn#newcode9
dbus/BUILD.gn:9: if (use_dbus) {
What about assert(use_dbus) at the top of the file instead?

https://codereview.chromium.org/2009983002/diff/1/extensions/browser/BUILD.gn
File extensions/browser/BUILD.gn (right):

https://codereview.chromium.org/2009983002/diff/1/extensions/browser/BUILD.gn...
extensions/browser/BUILD.gn:89: configs += [ "//build/config/linux/dbus" ]
ditto; rm

[slan, 2016-05-25 22:35:27]

The CQ bit was checked by slan@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-05-25 22:36:11]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2009983002/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/2009983002/1

[commit-bot: I haz the power, 2016-05-25 22:44:03]

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

[commit-bot: I haz the power, 2016-05-25 22:44:03]

Dry run: Try jobs failed on following builders:
  chromeos_amd64-generic_chromium_compile_only_ng on tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_amd64-...)
  chromeos_daisy_chromium_compile_only_ng on tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_daisy_...)
  chromeos_x86-generic_chromium_compile_only_ng on tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_x86-ge...)

[mbjorge, 2016-05-25 22:47:28]

The CQ bit was checked by mbjorge@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-05-25 22:48:45]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2009983002/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/2009983002/20001

[commit-bot: I haz the power, 2016-05-25 22:48:47]

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

[commit-bot: I haz the power, 2016-05-25 22:48:48]

Dry run: No L-G-T-M from a valid reviewer yet. 
CQ run can only be started by full committers or once the patch has
received an L-G-T-M from a full committer.
Even if an L-G-T-M 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.

[mbjorge, 2016-05-25 22:50:26]

On 2016/05/25 at 22:44:03, commit-bot wrote:
> Dry run: Try jobs failed on following builders:
>   chromeos_amd64-generic_chromium_compile_only_ng on tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_amd64-...)
>   chromeos_daisy_chromium_compile_only_ng on tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_daisy_...)
>   chromeos_x86-generic_chromium_compile_only_ng on tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_x86-ge...)

3 failures are goma proxy failing to start. Seem unrelated to this CL

[mbjorge, 2016-05-25 22:51:10]

Description was changed from

==========
Wrap dbus component and config with if (use_dbus)

Often times dbus dependencies get added without proper guards.
By wrapping the component with if (use_dbus) it forces proper guarding
when depending on dbus. This prevents downstream breakages in
chromecast.

BUG=internal b/28962923
==========

to

==========
Guard dbus component and config with assert(use_dbus)

Often times dbus dependencies get added without proper guards.
By wrapping the component with if (use_dbus) it forces proper guarding
when depending on dbus. This prevents downstream breakages in
chromecast.

BUG=internal b/28962923
==========

[mbjorge, 2016-05-25 22:51:30]

Description was changed from

==========
Guard dbus component and config with assert(use_dbus)

Often times dbus dependencies get added without proper guards.
By wrapping the component with if (use_dbus) it forces proper guarding
when depending on dbus. This prevents downstream breakages in
chromecast.

BUG=internal b/28962923
==========

to

==========
Guard dbus component and config with assert(use_dbus)

Often times dbus dependencies get added without proper guards.
By guarding the component with assert(use_dbus) it forces proper guarding
when depending on dbus. This prevents downstream breakages in
chromecast.

BUG=internal b/28962923
==========

[slan, 2016-05-25 23:05:47]

lgtm

[slan, 2016-05-25 23:05:54]

The CQ bit was checked by slan@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-05-25 23:06:42]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2009983002/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/2009983002/20001

[commit-bot: I haz the power, 2016-05-26 01:20:00]

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

[commit-bot: I haz the power, 2016-05-26 01:20:01]

Dry run: Try jobs failed on following builders:
  linux_chromium_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[brettw, 2016-05-31 20:22:37]

lgtm

https://codereview.chromium.org/2009983002/diff/20001/chrome/test/BUILD.gn
File chrome/test/BUILD.gn (right):

https://codereview.chromium.org/2009983002/diff/20001/chrome/test/BUILD.gn#ne...
chrome/test/BUILD.gn:1121: "//dbus",
You can remove this and just keep test support since you're touching it. The
convention is that the test_support version has a public dep on the
corresponding non-test target (and dbus does) so that only it is necessary.

https://codereview.chromium.org/2009983002/diff/20001/chrome/test/BUILD.gn#ne...
chrome/test/BUILD.gn:2055: "//dbus",
Ditto

[mbjorge, 2016-05-31 20:38:39]

The CQ bit was checked by mbjorge@chromium.org

[mbjorge, 2016-05-31 20:38:40]

https://codereview.chromium.org/2009983002/diff/1/build/config/linux/dbus/BUI...
File build/config/linux/dbus/BUILD.gn (right):

https://codereview.chromium.org/2009983002/diff/1/build/config/linux/dbus/BUI...
build/config/linux/dbus/BUILD.gn:10: if (use_dbus) {
Changed this to an assert also

https://codereview.chromium.org/2009983002/diff/1/chrome/browser/BUILD.gn
File chrome/browser/BUILD.gn (right):

https://codereview.chromium.org/2009983002/diff/1/chrome/browser/BUILD.gn#new...
chrome/browser/BUILD.gn:873: configs += [ "//build/config/linux/dbus" ]
On 2016/05/25 at 22:35:18, slan wrote:
> nit: rm - we get this for free via //dbus.

Done

https://codereview.chromium.org/2009983002/diff/1/dbus/BUILD.gn
File dbus/BUILD.gn (right):

https://codereview.chromium.org/2009983002/diff/1/dbus/BUILD.gn#newcode9
dbus/BUILD.gn:9: if (use_dbus) {
On 2016/05/25 at 22:35:19, slan wrote:
> What about assert(use_dbus) at the top of the file instead?

Yeah, that's probably better as it matches what I'm actually trying to enforce
more clearly

https://codereview.chromium.org/2009983002/diff/1/extensions/browser/BUILD.gn
File extensions/browser/BUILD.gn (right):

https://codereview.chromium.org/2009983002/diff/1/extensions/browser/BUILD.gn...
extensions/browser/BUILD.gn:89: configs += [ "//build/config/linux/dbus" ]
On 2016/05/25 at 22:35:19, slan wrote:
> ditto; rm

Done

https://codereview.chromium.org/2009983002/diff/20001/chrome/test/BUILD.gn
File chrome/test/BUILD.gn (right):

https://codereview.chromium.org/2009983002/diff/20001/chrome/test/BUILD.gn#ne...
chrome/test/BUILD.gn:1121: "//dbus",
Done

https://codereview.chromium.org/2009983002/diff/20001/chrome/test/BUILD.gn#ne...
chrome/test/BUILD.gn:2055: "//dbus",
Done

[mbjorge, 2016-05-31 20:38:40]

The patchset sent to the CQ was uploaded after l-g-t-m from slan@chromium.org,
brettw@chromium.org
Link to the patchset: https://codereview.chromium.org/2009983002/#ps40001
(title: "Remove unnecessary //dbus when //dbus:test_support exists")

[commit-bot: I haz the power, 2016-05-31 20:39:11]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2009983002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/2009983002/40001

[commit-bot: I haz the power, 2016-05-31 22:15:37]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-05-31 22:17:27]

Description was changed from

==========
Guard dbus component and config with assert(use_dbus)

Often times dbus dependencies get added without proper guards.
By guarding the component with assert(use_dbus) it forces proper guarding
when depending on dbus. This prevents downstream breakages in
chromecast.

BUG=internal b/28962923
==========

to

==========
Guard dbus component and config with assert(use_dbus)

Often times dbus dependencies get added without proper guards.
By guarding the component with assert(use_dbus) it forces proper guarding
when depending on dbus. This prevents downstream breakages in
chromecast.

BUG=internal b/28962923

Committed: https://crrev.com/a12e5a5b7a36268c6196d0ab185a0d8f6cf1441c
Cr-Commit-Position: refs/heads/master@{#396948}
==========

[commit-bot: I haz the power, 2016-05-31 22:17:28]

Patchset 3 (id:??) landed as
https://crrev.com/a12e5a5b7a36268c6196d0ab185a0d8f6cf1441c
Cr-Commit-Position: refs/heads/master@{#396948}