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

Specify allowed Macmini models #41219

Merged
merged 4 commits into from
Apr 14, 2023
Merged

Specify allowed Macmini models #41219

merged 4 commits into from
Apr 14, 2023

Conversation

zanderso
Copy link
Member

This excludes Macmini7,1.

Fixes flutter/flutter#124877

@zanderso zanderso mentioned this pull request Apr 14, 2023
Closed
@zanderso
Copy link
Member Author

It looks like a string is expected, not a map:

FormatException: Protobuf JSON decoding failed at: root["platform_properties"]["mac"]["properties"]["dimensions"]. Expected String value

@godofredoc
Copy link
Contributor

It looks like a string is expected, not a map:

FormatException: Protobuf JSON decoding failed at: root["platform_properties"]["mac"]["properties"]["dimensions"]. Expected String value

The error is coming from the .ci.yaml proto https://cs.opensource.google/flutter/cocoon/+/main:app_dart/lib/src/model/ci_yaml/target.dart;l=40, seems like some changes are going to be needed in the cocoon scheduler.

godofredoc
godofredoc reviewed Apr 14, 2023
View reviewed changes
.ci.yaml Outdated
@@ -55,6 +55,9 @@ platform_properties:
cpu: x86
os: Mac-12
xcode: 14a5294e # xcode 14.0 beta 5
dimensions:
mac_model: >-
["Macmini8,1", "Macmini9.1"]
Copy link
Contributor

Choose a reason for hiding this comment

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

It seems like mac_model: "Macmini8,1|Macmini9.1" would do the trick

dnfield
dnfield reviewed Apr 14, 2023
View reviewed changes
.ci.yaml Outdated
@@ -55,6 +55,8 @@ platform_properties:
cpu: x86
os: Mac-12
xcode: 14a5294e # xcode 14.0 beta 5
dimensions:
mac_model: "Macmini8,1|Macmini9.1"
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
mac_model: "Macmini8,1|Macmini9.1"
mac_model: "Macmini8,1|Macmini9,1"

@godofredoc
Copy link
Contributor

Seems like things are working as expected:
image

godofredoc
godofredoc approved these changes Apr 14, 2023
View reviewed changes
Copy link
Contributor

@godofredoc godofredoc left a comment

Choose a reason for hiding this comment

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

Thanks for the change!

@zanderso
Copy link
Member Author

👍🏻 No worries. It would be great if this could be self-service, but I think the documentation needs a lot of improvements before that's realistic.

ricardoamador
ricardoamador approved these changes Apr 14, 2023
View reviewed changes
Copy link
Contributor

@ricardoamador ricardoamador left a comment

Choose a reason for hiding this comment

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

LGTM

@zanderso zanderso added the autosubmit Merge PR when tree becomes green via auto submit App label Apr 14, 2023
@ricardoamador
Copy link
Contributor

@zanderso are you referring to the ciyaml readme? If so I can open a follow up task to add more detail.

@zanderso
Copy link
Member Author

zanderso commented Apr 14, 2023 •
edited
Loading

@zanderso are you referring to the ciyaml readme? If so I can open a follow up task to add more detail.

Yeah, it would be great to get something like a schema for the file, what fields can go where, what their types and values can be, etc.. And of course what effects they actually have on CI.

@ricardoamador
Copy link
Contributor

ricardoamador commented Apr 14, 2023 •
edited
Loading

Okay cool! I will open an infra ticket for that.

Created: flutter/flutter#124893 for tracking ci.yaml updates.

@jmagman
Copy link
Member

jmagman commented Apr 14, 2023

You could also remove the Macmini8,1 dimensions on this change. Example:

engine/.ci.yaml

Line 342 in 4ce42fe

mac_model: "Macmini8,1"

@auto-submit auto-submit bot merged commit 89d9657 into main Apr 14, 2023
@auto-submit auto-submit bot deleted the zanderso-patch-5 branch April 14, 2023 21:24
@jmagman jmagman mentioned this pull request Apr 14, 2023
Merged
@jmagman
Copy link
Member

jmagman commented Apr 14, 2023

You could also remove the Macmini8,1 dimensions on this change. Example:

engine/.ci.yaml

Line 342 in 4ce42fe

mac_model: "Macmini8,1"

On it: #41223

@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Apr 14, 2023
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Apr 14, 2023
@engine-flutter-autoroll
89d965757 Specify allowed Macmini models (flutter/engine#41219)
3d6528c
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Apr 15, 2023
@engine-flutter-autoroll
Roll Flutter Engine from e9739bca5c66 to f7945293aced (5 revisions) (#…
c40eed1 
…124900)

flutter/engine@e9739bc...f794529

2023-04-14 [email protected] Change v2 `mac_unopt` orchestrator builder to Linux (flutter/engine#41184)
2023-04-14 [email protected] Specify allowed Macmini models (flutter/engine#41219)
2023-04-14 [email protected] Migrate mac_host_engine to engine v2 builds. (flutter/engine#41149)
2023-04-14 [email protected] Remove cores dimension from Mac Host clang-tidy (flutter/engine#41214)
2023-04-14 [email protected] Run mac_host_engine orchestrator on arm Macs (flutter/engine#41185)

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
exaby73 pushed a commit to NevercodeHQ/flutter that referenced this pull request Apr 17, 2023
@engine-flutter-autoroll @exaby73
Roll Flutter Engine from e9739bca5c66 to f7945293aced (5 revisions) (f…
69e3df6 
…lutter#124900)

flutter/engine@e9739bc...f794529

2023-04-14 [email protected] Change v2 `mac_unopt` orchestrator builder to Linux (flutter/engine#41184)
2023-04-14 [email protected] Specify allowed Macmini models (flutter/engine#41219)
2023-04-14 [email protected] Migrate mac_host_engine to engine v2 builds. (flutter/engine#41149)
2023-04-14 [email protected] Remove cores dimension from Mac Host clang-tidy (flutter/engine#41214)
2023-04-14 [email protected] Run mac_host_engine orchestrator on arm Macs (flutter/engine#41185)

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
auto-submit bot pushed a commit that referenced this pull request May 2, 2023
@jmagman
Stop specifiying Macmini8,1 in ci builders, use inherited mac_model d…
d7375eb 
…imension (#41223)

As of #41219 all the .ci.yaml builders will run on either Macmini8,1 (x64) or Macmini9,1 (arm).  Stop specifying `Macmini8,1` on individual builders and let the specified architecture be the deciding factor.

Reverts #41203
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@dnfield dnfield dnfield left review comments

@ricardoamador ricardoamador ricardoamador approved these changes

@godofredoc godofredoc godofredoc approved these changes

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.

Do not Macmini 7,1 (Intel Iris 5100) to run engine tests
5 participants
@zanderso @godofredoc @ricardoamador @jmagman @dnfield