1685948 - Stop treating gfx/wr as third-party code

Status: NEW

(Developer Infrastructure :: Lint and Formatting, task)

Description

[Markus Stange [:mstange]]

The gfx/wr directory is currently both on the ThirdPartyPaths list and on the clang-format-ignore list.

I think that's by mistake. The "primary repo" for WebRender used to live on github, but as of several years ago, mozilla-central has become the primary repo and the github repo is just a mirror.

I think we should treat all of WebRender as first-party mozilla-central code, so that it can benefit from automatic code formatting and linting.

There is an existing rustfmt.toml in gfx/wr:

reorder_imports = false
reorder_imports_in_group = true
reorder_imported_names = true
error_on_line_overflow_comments = false
max_width = 100
spaces_around_ranges = true

Comment 1

[Markus Stange [:mstange]]

https://treeherder.mozilla.org/jobs?repo=try&revision=10a301073fc9c90a63f8238177564b795579c442

Comment 2

[Markus Stange [:mstange]]

Attachment: Bug 1685948 - Remove gfx/wr from the third party paths list. r=sylvestre

Comment 3

[Markus Stange [:mstange]]

Attachment: Bug 1685948 - Exclude gfx/wr/debugger from linting. r=sylvestre

Depends on D101303

Comment 4

[Markus Stange [:mstange]]

Attachment: Bug 1685948 - Reformat gfx/wr/webrender. r=gw

ignore-this-changeset

Depends on D101304

Comment 5

[Markus Stange [:mstange]]

Attachment: Bug 1685948 - Reformat gfx/wr/webrender_api. r=gw

ignore-this-changeset

Depends on D101305

Comment 6

[Markus Stange [:mstange]]

Attachment: Bug 1685948 - Reformat gfx/wr/wrench and gfx/wr/examples. r=gw

ignore-this-changeset

Depends on D101306

Comment 7

[Markus Stange [:mstange]]

Attachment: Bug 1685948 - Reformat gfx/wr/example-compositor and gfx/wr/tileview. r=gw

ignore-this-changeset

Depends on D101307

Comment 8

[Markus Stange [:mstange]]

Attachment: Bug 1685948 - Reformat gfx/wr/webrender_build. r=gw

ignore-this-changeset

Depends on D101308

Comment 9

[Markus Stange [:mstange]]

Attachment: Bug 1685948 - Reformat gfx/wr/glsl-to-cxx and gfx/wr/swgl. r=lsalzman

ignore-this-changeset

Depends on D101309

Comment 10

[Markus Stange [:mstange]]

Attachment: Bug 1685948 - Remove unnecessary parentheses. r=gw

This was not done by the automatic reformatting, and it now fails the build because of warnings-as-errors.

Depends on D101310

Comment 11

[Markus Stange [:mstange]]

Attachment: Bug 1685948 - Fix two python lint warnings in gfx/wr. r=gw

Depends on D101311

Comment 12

[Markus Stange [:mstange]]

Attachment: Bug 1685948 - Add missing license headers. r=gw

Depends on D101312

Comment 13

[Markus Stange [:mstange]]

Attachment: Bug 1685948 - Address lint errors in tileview code. r=gw

Depends on D101313

Comment 14

[Markus Stange [:mstange]]

Attachment: Bug 1685948 - Address lint errors in the wrench reftest analyzer. r=gw

Depends on D101314

Comment 15

[Markus Stange [:mstange]]

Attachment: Bug 1685948 - Fix line endings in .clang-format-ignore. r=sylvestre

ignore-this-changeset

Depends on D101315

Comment 16

[Markus Stange [:mstange]]

Attachment: Bug 1685948 - Remove gfx/wr from .clang-format-ignore. r=sylvestre

Depends on D101316

Comment 17

[Markus Stange [:mstange]]

Attachment: Bug 1685948 - Reformat example-compositor C++ code. r=gw

ignore-this-changeset

Depends on D101317

Comment 18

[Markus Stange [:mstange]]

Attachment: Bug 1685948 - Reformat SWGL C++ code. r=lsalzman

ignore-this-changeset

Depends on D101318

Comment 19

[Markus Stange [:mstange]]

Attachment: Bug 1685948 - Annotate implicit conversion constructors in SWGL. r=lsalzman

This preserves behavior. I haven't checked whether GLSL actually wants implicit conversions for all of these.

Depends on D101319

Comment 20

[Markus Stange [:mstange]]

https://treeherder.mozilla.org/jobs?repo=try&revision=b45e0cd161c786309ab3219e3c487109614fb6e1

Comment 21

[Markus Stange [:mstange]]

Attachment: Bug 1685948 - Make ClipRect constructors explicit. r=lsalzman

Depends on D101321

Comment 22

[Markus Stange [:mstange]]

https://treeherder.mozilla.org/jobs?repo=try&revision=b01972f086da20c0d582ca5a8bef28cf6df95614

Comment 23

[Markus Stange [:mstange]]

Attachment: Bug 1685948 - Remove some commas. r=gw

Depends on D101311

Comment 24

[Mark Banner (:standard8)]

Attachment: Bug 1685948 - Enable ESLint on gfx/wr/debugger/ where possible. r?mstange!

Depends on D101333

Comment 25

[Phabricator Automation]

Comment on attachment 9196346 [details]
Bug 1685948 - Enable ESLint on gfx/wr/debugger/ where possible. r?mstange!

Revision D101334 was moved to bug 1685986. Setting attachment 9196346 [details] to obsolete.

Comment 26

[Markus Stange [:mstange]]

Landing this is currently blocked on the servo-tidy job, which this turns red: https://treeherder.mozilla.org/jobs?repo=try&revision=b01972f086da20c0d582ca5a8bef28cf6df95614&selectedTaskRun=evEC6KvLRM2Ryn5Y1LkjCg.0

That's because our version of servo-tidy (which is the version that was published as 0.3.0) is rather outdated and checks for two things that rustfmt does not agree with (position of operators w.r.t. line breaks, and multi-line use statements).
I've asked for a new servo-tidy release on https://github.com/servo/servo/issues/28034 .

In the meantime, I will see if I can land the clang-format changes separately.

Comment 27

[Phabricator Automation]

Comment on attachment 9196318 [details]
Bug 1685948 - Reformat example-compositor C++ code. r=gw

Revision D101318 was moved to bug 1686635. Setting attachment 9196318 [details] to obsolete.

Comment 28

[Phabricator Automation]

Comment on attachment 9196319 [details]
Bug 1685948 - Reformat SWGL C++ code. r=lsalzman

Revision D101319 was moved to bug 1686635. Setting attachment 9196319 [details] to obsolete.

Comment 29

[Phabricator Automation]

Comment on attachment 9196322 [details]
Bug 1685948 - Annotate implicit conversion constructors in SWGL. r=lsalzman

Revision D101321 was moved to bug 1686635. Setting attachment 9196322 [details] to obsolete.

Comment 30

[Phabricator Automation]

Comment on attachment 9196324 [details]
Bug 1685948 - Make ClipRect constructors explicit. r=lsalzman

Revision D101323 was moved to bug 1686635. Setting attachment 9196324 [details] to obsolete.

Comment 31

[Phabricator Automation]

Comment on attachment 9196316 [details]
Bug 1685948 - Fix line endings in .clang-format-ignore. r=sylvestre

Revision D101316 was moved to bug 1686635. Setting attachment 9196316 [details] to obsolete.

Comment 32

[Phabricator Automation]

Comment on attachment 9196317 [details]
Bug 1685948 - Remove gfx/wr from .clang-format-ignore. r=sylvestre

Revision D101317 was moved to bug 1686635. Setting attachment 9196317 [details] to obsolete.

Comment 33

[BugBot [:suhaib / :marco]]

There are some r+ patches which didn't land and no activity in this bug for 2 weeks.
:mstange, could you have a look please?
For more information, please visit auto_nag documentation.

Comment 34

[Andrew Halberstadt [:ahal]]

Clearing stale needinfos.

Comment 35

[Markus Stange [:mstange]]

I never finished this, and at this point it would make more sense for somebody on the WR team to take this over if it's desired.