Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Migrate mac host clang tidy to engine v2. #41824

Merged
merged 6 commits into from
May 10, 2023
Merged

Migrate mac host clang tidy to engine v2. #41824

merged 6 commits into from
May 10, 2023

Conversation

godofredoc
Copy link
Contributor

This removes the additional gn command from the ci/lint tool and uses a single gn command.

Pre-launch Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I read the Tree Hygiene wiki page, which explains my responsibilities.
  • I read and followed the Flutter Style Guide and the C++, Objective-C, Java style guides.
  • I listed at least one issue that this PR fixes in the description above.
  • I added new tests to check the change I am making or feature I am adding, or Hixie said the PR is test-exempt. See testing the engine for instructions on writing and running engine tests.
  • I updated/added relevant documentation (doc comments with ///).
  • I signed the CLA.
  • All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

@godofredoc godofredoc requested review from zanderso and jmagman May 8, 2023 20:57
zanderso
zanderso reviewed May 9, 2023
View reviewed changes
ci/builders/standalone/mac_host_clang_tidy.json Outdated
"host_debug",
"--lint-all",
"--shard-id=1",
"--shard-variants=host_debug"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for remembering this a bit late, but if you look in the engine v1 recipe here:

https://flutter.googlesource.com/recipes/+/refs/heads/main/recipes/engine/engine_lint.py#101

Notice that the shardVariant is specified as ios_debug_sim. That's because the engine's linter script is looking at the compile commands for other variants, and only running a shard of the lint commands in the intersection, to avoid duplicating the work. That's why it was doing two gn commands.

As written, this will only lint half of the files. In order to duplicate the behavior of engine v1, we'd need to be able to run multiple gn commands. Another option would be to allow duplication between host_debug and ios_debug_sim, but to shard them with themselves. We could do that by running list.sh in parallel on two machines, one with --shard-id=0 and the other with --shard-id=1.

Not sure which is easier.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll give it another try merging mac host and ios clang tidy into a single build to avoid duplication and move the scripts to run as global tests. I could add the double gn logic but that may add more complexity to all the other configurations to cover this single use case.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is ready for review it is now making a 1:1 translation of the existing builds https://luci-milo.appspot.com/raw/build/logs.chromium.org/flutter/led/godofredoc_google.com/d3c09280c1196e2320dae4cee33f5ab5ea2044d27ef22554af37c71bfba6377c/+/build.proto

@godofredoc godofredoc added the autosubmit Merge PR when tree becomes green via auto submit App label May 10, 2023
@auto-submit auto-submit bot merged commit 0df6740 into flutter:main May 10, 2023
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request May 10, 2023
Merged
zanderso pushed a commit to engine-flutter-autoroll/flutter that referenced this pull request May 10, 2023
@engine-flutter-autoroll @zanderso
0df6740f4 Migrate mac host clang tidy to engine v2. (flutter/engine#4…
01ad986 
…1824)
zanderso pushed a commit to flutter/flutter that referenced this pull request May 10, 2023
@engine-flutter-autoroll
Manual roll Flutter Engine from 10ac36c52f08 to 892f88d2513f (11 revi…
ce3b4f8 
…sions) (#126455)

Manual roll requested by [email protected]

Cannot build log URL because revision "892f88d2513f" is invalid: Luci
builds of "Linux Fuchsia FEMU" for
892f88d2513f4a60ebd6d1496c4ac272e53c55ba was STARTED

2023-05-10 [email protected] Revert "Move linux fuchsia
engine v2 build to prod." (flutter/engine#41902)
2023-05-10 [email protected] Migrate mac host clang tidy to engine
v2. (flutter/engine#41824)
2023-05-10 [email protected] [Impeller] delete special handling
of RRect. (flutter/engine#41872)
2023-05-10 [email protected] Roll Fuchsia Mac SDK from
JiOACcaGrDphuHIql... to a3rrULFaXlwS8mfjW... (flutter/engine#41898)
2023-05-10 [email protected] Only
register top level window message listener upon registering service
binding (flutter/engine#41733)
2023-05-10 [email protected] Roll Skia from 8c936fb9ba8e to
32f4cfc2460b (27 revisions) (flutter/engine#41891)
2023-05-10 [email protected] [web] dialog a11y fixes
(flutter/engine#41681)
2023-05-10 [email protected] Roll Dart SDK from
2cf089614e1c to 7ad028c26344 (2 revisions) (flutter/engine#41882)
2023-05-10 [email protected] [Impeller] Increase minimum size of
alpha glyph atlas. (flutter/engine#41880)
2023-05-10 [email protected] [Impeller] Use separate atlases and
shaders for color and alpha (flutter/engine#41780)
2023-05-10 [email protected] Move linux fuchsia engine v2 build to
prod. (flutter/engine#41865)

Also rolling transitive DEPS:
  fuchsia/sdk/core/mac-amd64 from JiOACcaGrDph to a3rrULFaXlwS

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-engine-flutter-autoroll
Please CC [email protected],[email protected],[email protected] on the
revert to ensure that a human
is aware of the problem.

To file a bug in Flutter:
https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request May 10, 2023
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 10, 2023
@engine-flutter-autoroll
0df6740f4 Migrate mac host clang tidy to engine v2. (flutter/engine#4…
a6e0a91 
…1824)
zanderso pushed a commit to flutter/flutter that referenced this pull request May 10, 2023
@engine-flutter-autoroll
Roll Flutter Engine from 10ac36c52f08 to 406b3546a2ac (13 revisions) (#…
a3257ca 
…126468)

flutter/engine@10ac36c...406b354

2023-05-10 [email protected] [web] Re-enable history tests on Safari
(flutter/engine#41901)
2023-05-10 [email protected] switch from MockCanvas to DisplayListBuilder
in layer unit tests (flutter/engine#41889)
2023-05-10 [email protected] Revert "Move linux fuchsia
engine v2 build to prod." (flutter/engine#41902)
2023-05-10 [email protected] Migrate mac host clang tidy to engine
v2. (flutter/engine#41824)
2023-05-10 [email protected] [Impeller] delete special handling
of RRect. (flutter/engine#41872)
2023-05-10 [email protected] Roll Fuchsia Mac SDK from
JiOACcaGrDphuHIql... to a3rrULFaXlwS8mfjW... (flutter/engine#41898)
2023-05-10 [email protected] Only
register top level window message listener upon registering service
binding (flutter/engine#41733)
2023-05-10 [email protected] Roll Skia from 8c936fb9ba8e to
32f4cfc2460b (27 revisions) (flutter/engine#41891)
2023-05-10 [email protected] [web] dialog a11y fixes
(flutter/engine#41681)
2023-05-10 [email protected] Roll Dart SDK from
2cf089614e1c to 7ad028c26344 (2 revisions) (flutter/engine#41882)
2023-05-10 [email protected] [Impeller] Increase minimum size of
alpha glyph atlas. (flutter/engine#41880)
2023-05-10 [email protected] [Impeller] Use separate atlases and
shaders for color and alpha (flutter/engine#41780)
2023-05-10 [email protected] Move linux fuchsia engine v2 build to
prod. (flutter/engine#41865)

Also rolling transitive DEPS:
  fuchsia/sdk/core/mac-amd64 from JiOACcaGrDph to a3rrULFaXlwS

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-engine-flutter-autoroll
Please CC [email protected],[email protected],[email protected] on the
revert to ensure that a human
is aware of the problem.

To file a bug in Flutter:
https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
godofredoc added a commit that referenced this pull request May 13, 2023
@godofredoc
CP: Migrate mac host clang tidy to engine v2. (#41824) (#42005)
fb4b403 
CP: 0df6740

## Pre-launch Checklist

- [X] I read the [Contributor Guide] and followed the process outlined
there for submitting PRs.
- [X] I read the [Tree Hygiene] wiki page, which explains my
responsibilities.
- [X] I read and followed the [Flutter Style Guide] and the [C++,
Objective-C, Java style guides].
- [X] I listed at least one issue that this PR fixes in the description
above.
- [X] I added new tests to check the change I am making or feature I am
adding, or Hixie said the PR is test-exempt. See [testing the engine]
for instructions on writing and running engine tests.
- [X] I updated/added relevant documentation (doc comments with `///`).
- [X] I signed the [CLA].
- [X] All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel
on [Discord].

<!-- Links -->
[Contributor Guide]:
https://github.com/flutter/flutter/wiki/Tree-hygiene#overview
[Tree Hygiene]: https://github.com/flutter/flutter/wiki/Tree-hygiene
[Flutter Style Guide]:
https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo
[C++, Objective-C, Java style guides]:
https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
[testing the engine]:
https://github.com/flutter/flutter/wiki/Testing-the-engine
[CLA]: https://cla.developers.google.com/
[flutter/tests]: https://github.com/flutter/tests
[breaking change policy]:
https://github.com/flutter/flutter/wiki/Tree-hygiene#handling-breaking-changes
[Discord]: https://github.com/flutter/flutter/wiki/Chat
CaseyHillers pushed a commit to CaseyHillers/flutter that referenced this pull request May 24, 2023
@engine-flutter-autoroll
Manual roll Flutter Engine from 10ac36c52f08 to 892f88d2513f (11 revi…
76782c2 
…sions) (flutter#126455)

Manual roll requested by [email protected]

Cannot build log URL because revision "892f88d2513f" is invalid: Luci
builds of "Linux Fuchsia FEMU" for
892f88d2513f4a60ebd6d1496c4ac272e53c55ba was STARTED

2023-05-10 [email protected] Revert "Move linux fuchsia
engine v2 build to prod." (flutter/engine#41902)
2023-05-10 [email protected] Migrate mac host clang tidy to engine
v2. (flutter/engine#41824)
2023-05-10 [email protected] [Impeller] delete special handling
of RRect. (flutter/engine#41872)
2023-05-10 [email protected] Roll Fuchsia Mac SDK from
JiOACcaGrDphuHIql... to a3rrULFaXlwS8mfjW... (flutter/engine#41898)
2023-05-10 [email protected] Only
register top level window message listener upon registering service
binding (flutter/engine#41733)
2023-05-10 [email protected] Roll Skia from 8c936fb9ba8e to
32f4cfc2460b (27 revisions) (flutter/engine#41891)
2023-05-10 [email protected] [web] dialog a11y fixes
(flutter/engine#41681)
2023-05-10 [email protected] Roll Dart SDK from
2cf089614e1c to 7ad028c26344 (2 revisions) (flutter/engine#41882)
2023-05-10 [email protected] [Impeller] Increase minimum size of
alpha glyph atlas. (flutter/engine#41880)
2023-05-10 [email protected] [Impeller] Use separate atlases and
shaders for color and alpha (flutter/engine#41780)
2023-05-10 [email protected] Move linux fuchsia engine v2 build to
prod. (flutter/engine#41865)

Also rolling transitive DEPS:
  fuchsia/sdk/core/mac-amd64 from JiOACcaGrDph to a3rrULFaXlwS

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-engine-flutter-autoroll
Please CC [email protected],[email protected],[email protected] on the
revert to ensure that a human
is aware of the problem.

To file a bug in Flutter:
https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
CaseyHillers pushed a commit to CaseyHillers/flutter that referenced this pull request May 24, 2023
@engine-flutter-autoroll
Roll Flutter Engine from 10ac36c52f08 to 406b3546a2ac (13 revisions) (f…
5113c0e 
…lutter#126468)

flutter/engine@10ac36c...406b354

2023-05-10 [email protected] [web] Re-enable history tests on Safari
(flutter/engine#41901)
2023-05-10 [email protected] switch from MockCanvas to DisplayListBuilder
in layer unit tests (flutter/engine#41889)
2023-05-10 [email protected] Revert "Move linux fuchsia
engine v2 build to prod." (flutter/engine#41902)
2023-05-10 [email protected] Migrate mac host clang tidy to engine
v2. (flutter/engine#41824)
2023-05-10 [email protected] [Impeller] delete special handling
of RRect. (flutter/engine#41872)
2023-05-10 [email protected] Roll Fuchsia Mac SDK from
JiOACcaGrDphuHIql... to a3rrULFaXlwS8mfjW... (flutter/engine#41898)
2023-05-10 [email protected] Only
register top level window message listener upon registering service
binding (flutter/engine#41733)
2023-05-10 [email protected] Roll Skia from 8c936fb9ba8e to
32f4cfc2460b (27 revisions) (flutter/engine#41891)
2023-05-10 [email protected] [web] dialog a11y fixes
(flutter/engine#41681)
2023-05-10 [email protected] Roll Dart SDK from
2cf089614e1c to 7ad028c26344 (2 revisions) (flutter/engine#41882)
2023-05-10 [email protected] [Impeller] Increase minimum size of
alpha glyph atlas. (flutter/engine#41880)
2023-05-10 [email protected] [Impeller] Use separate atlases and
shaders for color and alpha (flutter/engine#41780)
2023-05-10 [email protected] Move linux fuchsia engine v2 build to
prod. (flutter/engine#41865)

Also rolling transitive DEPS:
  fuchsia/sdk/core/mac-amd64 from JiOACcaGrDph to a3rrULFaXlwS

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-engine-flutter-autoroll
Please CC [email protected],[email protected],[email protected] on the
revert to ensure that a human
is aware of the problem.

To file a bug in Flutter:
https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
@godofredoc godofredoc deleted the mac_host_clang_tidy branch June 2, 2023 18:46
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@zanderso zanderso zanderso approved these changes

@jmagman jmagman Awaiting requested review from jmagman

Assignees
No one assigned
Labels
autosubmit Merge PR when tree becomes green via auto submit App
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@godofredoc @zanderso