brave / brave-browser

Brave browser for Android, iOS, Linux, macOS, Windows.
https://brave.com
Mozilla Public License 2.0
17.52k stars 2.27k forks source link

`npm run gn_check` doesn't complain about wrong header including #35772

Open simonhong opened 7 months ago

simonhong commented 7 months ago

After adding #include "chrome/common/channel_info.h" into chrome/elevation_service/elevator.cc, npm run gn_check gives complain like below.

ERROR at //chrome/elevation_service/elevator.cc:22:11: Can't include this header from here.
#include "chrome/common/channel_info.h"
          ^---------------------------
The target:
  //chrome/elevation_service:lib
is including a file from the target:
  //chrome/common:channel_info

It's usually best to depend directly on the destination target.
In some cases, the destination target is considered a subcomponent
of an intermediate target. In this case, the intermediate target
should depend publicly on the destination to forward the ability
to include headers.

Dependency chain (there may also be others):
  //chrome/elevation_service:lib -->
  //brave/browser/brave_vpn/win/brave_vpn_wireguard_service:install_utils --[private]-->
  //brave/browser/brave_vpn/win/brave_vpn_wireguard_service/status_tray:status_tray --[private]-->
  //brave/browser/brave_vpn/win/brave_vpn_wireguard_service/status_tray/ras:ras --[private]-->
  //chrome/common:channel_info

null
null

However, adding same header to brave/chromium_src/chrome/elevation_service/elevator.cc doesn't give above error.

cc @goodov

goodov commented 7 months ago

that's expected, because chromium_src files are not validated by gn and there's no way currently to do that without forking gn or heavy patching gn/gni files..

because of that we need to always be careful about adding things into chromium_src files.