Bug 140248 – Add a script to check for platform layering violations

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Carlos Garcia Campos]

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.

[Carlos Garcia Campos]

Created attachment 244259 [details]
Patch

[Darin Adler]

Could you post a copy of the current output of the script?

[Carlos Garcia Campos]

(In reply to comment #2)
> Could you post a copy of the current output of the script?

Sure, I'll attach it.

[Carlos Garcia Campos]

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.

[Darin Adler]

(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?

[Darin Adler]

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?

[Carlos Garcia Campos]

(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!

[Carlos Garcia Campos]

Committed r178167: <http://trac.webkit.org/changeset/178167>

[Carlos Garcia Campos]

(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.