Bug 140248 - Add a script to check for platform layering violations
Summary: Add a script to check for platform layering violations
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on: 140288
Blocks: 21354
  Show dependency treegraph
 
Reported: 2015-01-08 08:20 PST by Carlos Garcia Campos
Modified: 2015-01-09 03:49 PST (History)
1 user (show)

See Also:


Attachments
Patch (3.57 KB, patch)
2015-01-08 08:24 PST, Carlos Garcia Campos
darin: review+
Details | Formatted Diff | Diff
Current output (28.54 KB, text/plain)
2015-01-08 08:36 PST, Carlos Garcia Campos
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Carlos Garcia Campos 2015-01-08 08:20:46 PST
I wrote a script to get all the platform layering violations, and I think it might be useful for others. We could just add it to Tools/Scripts for now so that it can be manually run. I think there are too many layering violations at the moment to run the script in the bots or as part of the style checker.
Comment 1 Carlos Garcia Campos 2015-01-08 08:24:01 PST
Created attachment 244259 [details]
Patch
Comment 2 Darin Adler 2015-01-08 08:31:24 PST
Could you post a copy of the current output of the script?
Comment 3 Carlos Garcia Campos 2015-01-08 08:35:35 PST
(In reply to comment #2)
> Could you post a copy of the current output of the script?

Sure, I'll attach it.
Comment 4 Carlos Garcia Campos 2015-01-08 08:36:54 PST
Created attachment 244261 [details]
Current output

I think there are a few false positives, that are not layering violations, but that should be fixed anyway using <> instead of "" for example.
Comment 5 Darin Adler 2015-01-08 09:01:38 PST
(In reply to comment #4)
> I think there are a few false positives, that are not layering violations,
> but that should be fixed anyway using <> instead of "" for example.

Before we start fixing the actual violations, I think it’s important to wipe out the false positives. Could you give an example of one where you think using <> is the right thing to do?
Comment 6 Darin Adler 2015-01-08 09:04:04 PST
OK, <stdio.h> clearly falls into that category.

But I’m more worried about the "TextCodec.cpp" one, for example. Why is that showing up as a violation?

I love the idea of the script, and I think I’ll say review+, but I really want this script in a place where its output is actionable as soon as possible, maybe even before you land it?
Comment 7 Carlos Garcia Campos 2015-01-08 09:34:55 PST
(In reply to comment #6)
> OK, <stdio.h> clearly falls into that category.

Yes, exactly.

> But I’m more worried about the "TextCodec.cpp" one, for example. Why is that
> showing up as a violation?

hmm, that's a bug in the script itself, I was assuming all includes are header files, I'll fix that.

> I love the idea of the script, and I think I’ll say review+, but I really
> want this script in a place where its output is actionable as soon as
> possible, maybe even before you land it?

Sure, I'll check and fix all the false positives before, thanks!
Comment 8 Carlos Garcia Campos 2015-01-09 03:48:35 PST
Committed r178167: <http://trac.webkit.org/changeset/178167>
Comment 9 Carlos Garcia Campos 2015-01-09 03:49:56 PST
(In reply to comment #8)
> Committed r178167: <http://trac.webkit.org/changeset/178167>

False positives were fixed in r178166 and I fixed the cpp includes before landing this.