Closed Bug 1183143 Opened 5 years ago Closed 5 years ago

List of 3rd party source directories and files

Categories

(Firefox Build System :: Source Code Analysis, defect)

defect
Not set

Tracking

(firefox42 fixed)

RESOLVED FIXED
mozilla42
Tracking Status
firefox42 --- fixed

People

(Reporter: poiru, Assigned: poiru)

References

Details

Attachments

(1 file, 1 obsolete file)

It would be useful to have a list of 3rd party source directories and files in order to ignore them when running tools like clang-tidy. An alternative solution would be to have this information in the moz.build files, but I think a simple text file would be fine as well.

Ehsan, any thoughts?
Flags: needinfo?(ehsan)
The question is, what format do the tooling based tools would expect for that.  I'm not familiar with that.  Are you?
Flags: needinfo?(ehsan) → needinfo?(birunthan)
(In reply to Ehsan Akhgari (not reviewing patches, not reading bugmail, needinfo? me!) from comment #1)
> The question is, what format do the tooling based tools would expect for
> that.  I'm not familiar with that.  Are you?

AFAIK, clang-tidy is not able to exclude paths. clang-modernize, on the other hand, can read a list of path prefixes to ignore. I think that would be sufficient for our needs. I can work on teaching clang-tidy about excluded files, but given this file, we could always just do this:

  clang-tidy ...
  cat third-party-paths | xargs hg revert (or git checkout)
Flags: needinfo?(birunthan)
OK then, a plain text file should fit the bill for that purpose!

IIRC the bug where we did the original PRBool conversion should have a list that you can get started with!
Attached patch Add THIRD_PARTY_PATHS file (obsolete) — Splinter Review
This is likely missing some paths, but should cover the majority.
Attachment #8633172 - Flags: review?(ehsan)
Assignee: nobody → birunthan
Status: NEW → ASSIGNED
Comment on attachment 8633172 [details] [diff] [review]
Add THIRD_PARTY_PATHS file

Review of attachment 8633172 [details] [diff] [review]:
-----------------------------------------------------------------

This should not be a top-level file.

Please move it to tools/rewriting (create that directory!)

Also, I'd like to know how you came up with this list.  I can't think of anything off the top of my head that this doesn't cover!  Your method should at least be documented in the commit message.

::: THIRD_PARTY_PATHS
@@ +1,4 @@
> +browser/components/translation/cld2/
> +build/stlport/
> +db/sqlite3/src/
> +extensions/spellcheck/hunspell/src/

This is incomplete, as some of the files in that directory are our code.  I think you need to make this format also accept files, and include the individual  third-party files there.  Since these files are easily identifiable through their extension, bonus points for using a glob format!

@@ +10,5 @@
> +gfx/harfbuzz/
> +gfx/ots/
> +gfx/qcms/
> +gfx/skia/
> +gfx/skia/

This is duplicate.
Attachment #8633172 - Flags: review?(ehsan) → review-
(In reply to Ehsan Akhgari (not reviewing patches, not reading bugmail, needinfo? me!) from comment #5)
> Also, I'd like to know how you came up with this list.  I can't think of
> anything off the top of my head that this doesn't cover!  Your method should
> at least be documented in the commit message.

I already had most of these in a exclude list I use for my text editor. I initially discovered most of them by searching for README and LICENSE files.
Attachment #8633172 - Attachment is obsolete: true
Attachment #8633279 - Flags: review?(ehsan)
Attachment #8633279 - Flags: review?(ehsan) → review+
Oh, please remove dom/media/webaudio/blink/.  That's our code.
https://hg.mozilla.org/mozilla-central/rev/8a56286ef959
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla42
Depends on: 1315438
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.