New issue
Advanced search Search tips

Issue 898837 link

Starred by 1 user

Issue metadata

Status: Available
Owner: ----
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocked on: View detail
issue 800764
issue 899771



Sign in to add a comment

Make gn check run on the whole chromium tree

Project Member Reported by brat...@opera.com, Oct 25

Issue description

We have trybots running gn check to verify that dependencies in the build system are correct, but gn check will only check what is in the check_targets list defined in the file //.gn. That file does not include many new additions and has
not been updated as previously excluded parts have been made correct.

This bug is to track fixing the list in .gn and possibly code and other build files so that gn check works on the whole tree.

There is also https://bugs.chromium.org/p/gn/issues/detail?id=7 which is about a better way to specify the check_targets list so that it requires less manual work to maintain it and so that it will cover more new code automatically.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Oct 25

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ea37a1ae75a4b53819373f92a0aa68d2fa6952e1

commit ea37a1ae75a4b53819373f92a0aa68d2fa6952e1
Author: Daniel Bratell <bratell@opera.com>
Date: Thu Oct 25 15:19:54 2018

[libaddressinput] Make build system pass gn check

There were some missing headers that gn check found in
third_party/ced instead (string_util.h is a common name) and
then complained that libaddressinput can't include files from
third_party/ced. This adds those headers and some more.

It doesn't add the public headers since those have no natural
position in the dependency tree (used externally, and by util).

Bug: 898837
Change-Id: Iec0d622c0da861d149f86267eebafa63221d3906
Reviewed-on: https://chromium-review.googlesource.com/c/1299000
Reviewed-by: Mathieu Perreault <mathp@chromium.org>
Commit-Queue: Daniel Bratell <bratell@opera.com>
Cr-Commit-Position: refs/heads/master@{#602712}
[modify] https://crrev.com/ea37a1ae75a4b53819373f92a0aa68d2fa6952e1/third_party/libaddressinput/BUILD.gn

Project Member

Comment 2 by bugdroid1@chromium.org, Oct 25

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2ce9d5e182ba361ba9c2e377e80d8f685420ae99

commit 2ce9d5e182ba361ba9c2e377e80d8f685420ae99
Author: Daniel Bratell <bratell@opera.com>
Date: Thu Oct 25 16:46:40 2018

[flatbuffers] Make build system pass gn check

There were some broken dependencies that gn check reported
errors for which were easily fixed by moving relevant headers
to :flatbuffers. The other targets depend on :flatbuffers so
they can still use those headers.

Bug: 898837
Change-Id: I259a7e5d781ea17e667b6e6469a1133e875ec7b8
Reviewed-on: https://chromium-review.googlesource.com/c/1299137
Reviewed-by: Balazs Engedy <engedy@chromium.org>
Commit-Queue: Daniel Bratell <bratell@opera.com>
Cr-Commit-Position: refs/heads/master@{#602753}
[modify] https://crrev.com/2ce9d5e182ba361ba9c2e377e80d8f685420ae99/third_party/flatbuffers/BUILD.gn

Blockedon: 800764
Project Member

Comment 4 by bugdroid1@chromium.org, Oct 26

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1e733a06fa25186dae2693f10a2757e64bf7535a

commit 1e733a06fa25186dae2693f10a2757e64bf7535a
Author: Daniel Bratell <bratell@opera.com>
Date: Fri Oct 26 18:39:25 2018

Updated the .gn check_targets list to include all* of third_party/*

gn check by default only works on those targets that are listed in
check_targets, and since we can't whitelist all of //third_party,
every sub-folder has to be individually listed. Some were listed
before but this adds another two hundred.

5-10% of the folders are still excluded because they have one or
more internal check problems. Specifically third_party/blink is
not included yet. Working on it.

Bug: 898837
Change-Id: Ia2a2cc14c00ecfdb956eb553664efe46356dedc3
Reviewed-on: https://chromium-review.googlesource.com/c/1299086
Reviewed-by: Scott Graham <scottmg@chromium.org>
Commit-Queue: Daniel Bratell <bratell@opera.com>
Cr-Commit-Position: refs/heads/master@{#603156}
[modify] https://crrev.com/1e733a06fa25186dae2693f10a2757e64bf7535a/.gn

Blockedon: 899771
Project Member

Comment 6 by bugdroid1@chromium.org, Oct 30

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a65c275f7d38431493d126c7d1b7815769a04a84

commit a65c275f7d38431493d126c7d1b7815769a04a84
Author: Daniel Bratell <bratell@opera.com>
Date: Tue Oct 30 18:52:27 2018

Correct abseil dependencies to make it possible to run gn check

//third_party/abseil-cpp is currently excluded from
gn check because of a couple of dependency fails. This patch
tries to fix those.

Most complicated is the dependency to gmock. The code does
include "gmock/gmock.h" which gn will see as a dependency on
//third_party/googletest:gmock.

Bug: 898837
Change-Id: I2c1d33935511e790e1343cb59db11908cf4fab15
Reviewed-on: https://chromium-review.googlesource.com/c/1307497
Commit-Queue: Daniel Bratell <bratell@opera.com>
Reviewed-by: Patrik Höglund <phoglund@chromium.org>
Cr-Commit-Position: refs/heads/master@{#603976}
[modify] https://crrev.com/a65c275f7d38431493d126c7d1b7815769a04a84/third_party/abseil-cpp/absl/debugging/BUILD.gn
[modify] https://crrev.com/a65c275f7d38431493d126c7d1b7815769a04a84/third_party/abseil-cpp/absl/time/BUILD.gn
[modify] https://crrev.com/a65c275f7d38431493d126c7d1b7815769a04a84/third_party/abseil-cpp/absl/types/BUILD.gn

Project Member

Comment 7 by bugdroid1@chromium.org, Oct 31

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/31be7d10c502a7a4b6a067714cf92faa4c0bbded

commit 31be7d10c502a7a4b6a067714cf92faa4c0bbded
Author: Daniel Bratell <bratell@opera.com>
Date: Wed Oct 31 10:18:06 2018

Move cacheinvalidation testing file to a testing build target

This allows third_party/cache_invalidation to be checked
with gn check (as soon as it's enabled in //.gn).

Bug: 898837
Change-Id: Ica669d8a7277178878d630b54879e1439350f566
Reviewed-on: https://chromium-review.googlesource.com/c/1307496
Reviewed-by: Nicolas Zea <zea@chromium.org>
Commit-Queue: Daniel Bratell <bratell@opera.com>
Cr-Commit-Position: refs/heads/master@{#604196}
[modify] https://crrev.com/31be7d10c502a7a4b6a067714cf92faa4c0bbded/third_party/cacheinvalidation/BUILD.gn

Project Member

Comment 8 by bugdroid1@chromium.org, Oct 31

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f2f7bfe61121c05d9a510b949931e395dce40608

commit f2f7bfe61121c05d9a510b949931e395dce40608
Author: Daniel Bratell <bratell@opera.com>
Date: Wed Oct 31 11:39:12 2018

[zlib] Make build system pass gn check

There are some circular include dependencies in the zlib
build system because some parts are extracted into their
own targets. To make gn check understand that, those targets
need to be added to the allow_circular_includes_from list.

Bug: 898837
Change-Id: Ic94c5f5ac88e10792b458f4d734c4ad0487023c2
Reviewed-on: https://chromium-review.googlesource.com/c/1299135
Reviewed-by: Chris Blume <cblume@chromium.org>
Commit-Queue: Daniel Bratell <bratell@opera.com>
Cr-Commit-Position: refs/heads/master@{#604210}
[modify] https://crrev.com/f2f7bfe61121c05d9a510b949931e395dce40608/third_party/zlib/BUILD.gn

Project Member

Comment 9 by bugdroid1@chromium.org, Oct 31

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/961890d8726e1e49e939fa0d7dd693b6b8a1ae78

commit 961890d8726e1e49e939fa0d7dd693b6b8a1ae78
Author: Daniel Bratell <bratell@opera.com>
Date: Wed Oct 31 19:39:33 2018

Do more gn check of third_party/*

More of third_party/* has been made gn check clean and this CL
enables it for another 14 directories. Most (but not all) of the
rest look non-trivial.

Bug: 898837
Change-Id: I95c7348d8c1fa0bf85e0345aaaa9228f971a300e
Reviewed-on: https://chromium-review.googlesource.com/c/1309743
Reviewed-by: Scott Graham <scottmg@chromium.org>
Commit-Queue: Daniel Bratell <bratell@opera.com>
Cr-Commit-Position: refs/heads/master@{#604365}
[modify] https://crrev.com/961890d8726e1e49e939fa0d7dd693b6b8a1ae78/.gn

Project Member

Comment 10 by bugdroid1@chromium.org, Nov 12

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/768b2946b1ec5f092a99b5c511a3a1f9f8dc3da6

commit 768b2946b1ec5f092a99b5c511a3a1f9f8dc3da6
Author: Daniel Bratell <bratell@opera.com>
Date: Mon Nov 12 17:32:35 2018

Let gn check verify that blink/public dependencies are sane

We can't have gn check look through all of blink because it's
a messy monolithic pile of internal dependencies, but the
public API layer has strict rules that should be enforced by
gn check.

Also removing a mojo header from a build target since it's supposedly
already in a mojo build target and listing it here as well
makes gn check unhappy.

Bug: 898837
Change-Id: Id46485ffa4ffeb0c897f6d22e684f6d45d4804e6
Reviewed-on: https://chromium-review.googlesource.com/c/1307373
Commit-Queue: Daniel Bratell <bratell@opera.com>
Reviewed-by: Scott Graham <scottmg@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#607283}
[modify] https://crrev.com/768b2946b1ec5f092a99b5c511a3a1f9f8dc3da6/.gn
[modify] https://crrev.com/768b2946b1ec5f092a99b5c511a3a1f9f8dc3da6/third_party/blink/public/common/BUILD.gn

Sign in to add a comment