New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 716359 link

Starred by 2 users

Issue metadata

Status: Duplicate
Merged: issue 502447
Owner:
Closed: May 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 3
Type: Bug



Sign in to add a comment

GN file allows non-existing header files

Project Member Reported by wychen@chromium.org, Apr 28 2017

Issue description

It's possible to put non-existing header files in "sources" or "public" properties in GN file without causing any errors.

It seems there's no practical downside of keeping these non-existing header files in GN files. This doesn't cause issues in "gn analyze" like issue 661774. It's still wrong though. We might want to check this in "gn gen --check".
 

Comment 1 by brettw@chromium.org, Apr 28 2017

If header checking is enabled for that target, the check should fail if there are nonexistent files.

Comment 2 by wychen@chromium.org, Apr 28 2017

I see. //third_party/WebKit/public/* is the top offender in https://codereview.chromium.org/2849763002/, and it's not in the white list in https://cs.chromium.org/chromium/src/.gn?type=cs&q=f:%5C.gn+check_targets&l=52. Is it worth the effort to add it?

Comment 3 by brettw@chromium.org, Apr 28 2017

Ideally we should be adding everything to the whitelist.
Project Member

Comment 4 by bugdroid1@chromium.org, Apr 29 2017

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

commit e7a3d6480969e0a0ef64fa44f7bcae57f312b04e
Author: wychen <wychen@chromium.org>
Date: Sat Apr 29 07:12:17 2017

Support checking and fixing non-existing header files in GN

BUG= 716359 

Review-Url: https://codereview.chromium.org/2846593007
Cr-Commit-Position: refs/heads/master@{#468230}

[modify] https://crrev.com/e7a3d6480969e0a0ef64fa44f7bcae57f312b04e/build/check_gn_headers.py
[modify] https://crrev.com/e7a3d6480969e0a0ef64fa44f7bcae57f312b04e/build/fix_gn_headers.py

Project Member

Comment 5 by bugdroid1@chromium.org, Apr 29 2017

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

commit 31719eefcc4e06a6cb0b41364eb91d0857c2ce2f
Author: wychen <wychen@chromium.org>
Date: Sat Apr 29 11:10:07 2017

Remove non-existing headers from the build

I ran check_gn_headers.py on linux and android full builds, merged the
extra header file lists, fed to "fix_gn_headers.py --remove", and
chose which matches to apply interactively.

8 missing headers in the build were fixed because they were moved.

BUG= 716359 

Review-Url: https://codereview.chromium.org/2849763002
Cr-Commit-Position: refs/heads/master@{#468237}

[modify] https://crrev.com/31719eefcc4e06a6cb0b41364eb91d0857c2ce2f/base/allocator/BUILD.gn
[modify] https://crrev.com/31719eefcc4e06a6cb0b41364eb91d0857c2ce2f/chrome/browser/BUILD.gn
[modify] https://crrev.com/31719eefcc4e06a6cb0b41364eb91d0857c2ce2f/chrome/browser/android/vr_shell/BUILD.gn
[modify] https://crrev.com/31719eefcc4e06a6cb0b41364eb91d0857c2ce2f/content/browser/DEPS
[modify] https://crrev.com/31719eefcc4e06a6cb0b41364eb91d0857c2ce2f/extensions/common/BUILD.gn
[modify] https://crrev.com/31719eefcc4e06a6cb0b41364eb91d0857c2ce2f/headless/BUILD.gn
[modify] https://crrev.com/31719eefcc4e06a6cb0b41364eb91d0857c2ce2f/ppapi/c/BUILD.gn
[modify] https://crrev.com/31719eefcc4e06a6cb0b41364eb91d0857c2ce2f/remoting/base/BUILD.gn
[modify] https://crrev.com/31719eefcc4e06a6cb0b41364eb91d0857c2ce2f/remoting/client/BUILD.gn
[modify] https://crrev.com/31719eefcc4e06a6cb0b41364eb91d0857c2ce2f/remoting/client/jni/BUILD.gn
[modify] https://crrev.com/31719eefcc4e06a6cb0b41364eb91d0857c2ce2f/third_party/WebKit/Source/platform/BUILD.gn
[modify] https://crrev.com/31719eefcc4e06a6cb0b41364eb91d0857c2ce2f/third_party/WebKit/public/BUILD.gn
[modify] https://crrev.com/31719eefcc4e06a6cb0b41364eb91d0857c2ce2f/ui/latency/BUILD.gn

Project Member

Comment 6 by bugdroid1@chromium.org, May 2 2017

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

commit 1d2fd608a4f16afad4bfc2f1c856c520da1d2d79
Author: wychen <wychen@chromium.org>
Date: Tue May 02 04:45:05 2017

Fix header dependency in WebKit/public about testing

This fixes disallowed inclusion of testing/gmock/include/gmock/gmock.h
in WebKit/public:blink_headers.

This is a prerequisite of whitelisting //third_party/WebKit/public
for header checking in GN.

BUG= 716359 

Review-Url: https://codereview.chromium.org/2853173002
Cr-Commit-Position: refs/heads/master@{#468560}

[modify] https://crrev.com/1d2fd608a4f16afad4bfc2f1c856c520da1d2d79/third_party/WebKit/public/BUILD.gn

Project Member

Comment 7 by bugdroid1@chromium.org, May 2 2017

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

commit 56a5100afa68236b7c4e2c5e2cf6f92a79245792
Author: wychen <wychen@chromium.org>
Date: Tue May 02 05:04:29 2017

Fix missing headers in WebKit/public:mojo_bindings

This fixes disallowed inclusion of content/common/content_export.h
in WebKit/public:mojo_bindings. Missing headers in some typemap files
are amended.

This is the prerequisite of whitelisting //third_party/WebKit/public
for header checking in GN.

BUG= 716359 

Review-Url: https://codereview.chromium.org/2858453002
Cr-Commit-Position: refs/heads/master@{#468562}

[modify] https://crrev.com/56a5100afa68236b7c4e2c5e2cf6f92a79245792/content/common/service_worker/service_worker_event_dispatcher.typemap
[modify] https://crrev.com/56a5100afa68236b7c4e2c5e2cf6f92a79245792/content/common/service_worker/service_worker_fetch_request.typemap
[modify] https://crrev.com/56a5100afa68236b7c4e2c5e2cf6f92a79245792/content/common/service_worker/service_worker_types.typemap
[modify] https://crrev.com/56a5100afa68236b7c4e2c5e2cf6f92a79245792/mojo/public/tools/bindings/mojom.gni

Project Member

Comment 8 by bugdroid1@chromium.org, May 2 2017

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

commit c1e24b60875991f3e7be47e657a2515b29f19040
Author: wychen <wychen@chromium.org>
Date: Tue May 02 08:49:38 2017

Split GN target v8_headers for browser non-code dependency

This is for https://crrev.com/2851953002/

BUG=  chromium:716359 

Review-Url: https://codereview.chromium.org/2853783002
Cr-Commit-Position: refs/heads/master@{#45009}

[modify] https://crrev.com/c1e24b60875991f3e7be47e657a2515b29f19040/BUILD.gn

Project Member

Comment 9 by bugdroid1@chromium.org, May 4 2017

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

commit 06014b12cb233379169758b863703053d99cacc3
Author: wychen <wychen@chromium.org>
Date: Thu May 04 23:36:23 2017

Update deps in WebKit/public to reflect actual usage

This is a prerequisite of whitelisting //third_party/WebKit/public
for header checking in GN.

BUG= 716359 

Review-Url: https://codereview.chromium.org/2851953002
Cr-Commit-Position: refs/heads/master@{#469522}

[modify] https://crrev.com/06014b12cb233379169758b863703053d99cacc3/third_party/WebKit/public/BUILD.gn

Comment 10 Deleted

Comment 11 by m...@chromium.org, May 5 2017

My mistake on c#10. Nothing to see there. I'm deleting the comment to avoid confusion.
Project Member

Comment 12 by bugdroid1@chromium.org, May 9 2017

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

commit c50f8193152098b3d105545ffc78986e071a8623
Author: wychen <wychen@chromium.org>
Date: Tue May 09 04:37:44 2017

Enable header checking in WebKit/public/*

After the following CLs, third_party/WebKit/public/* is clean enough
to turn on header checking in GN.
- https://codereview.chromium.org/2849763002
- https://codereview.chromium.org/2853173002
- https://codereview.chromium.org/2858453002
- https://codereview.chromium.org/2851953002

BUG= 716359 

Review-Url: https://codereview.chromium.org/2862813003
Cr-Commit-Position: refs/heads/master@{#470213}

[modify] https://crrev.com/c50f8193152098b3d105545ffc78986e071a8623/.gn

Project Member

Comment 13 by bugdroid1@chromium.org, May 17 2017

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

commit 90e87f4bb986def04af8f8310cd495bfbead49bb
Author: wychen <wychen@chromium.org>
Date: Wed May 17 06:50:31 2017

Fix a typo in the header filename in media/base/BUILD.gn

The typo was introduced in https://codereview.chromium.org/2856253004.

BUG=661774,  716359 

Review-Url: https://codereview.chromium.org/2890643002
Cr-Commit-Position: refs/heads/master@{#472358}

[modify] https://crrev.com/90e87f4bb986def04af8f8310cd495bfbead49bb/media/base/BUILD.gn

Mergedinto: 502447
Status: Duplicate (was: Started)

Sign in to add a comment