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

Use release_build and os dimension consistently. #42012

Merged
merged 5 commits into from
May 15, 2023
Merged

Use release_build and os dimension consistently. #42012

merged 5 commits into from
May 15, 2023

Conversation

godofredoc
Copy link
Contributor

Release_build property is used to signal that this build produces release artifacts that will eventually be SLSA compliant and will need to run from dart_internal.

The os property is used to select the correct drone based on the platform the build should run on.

Bug: flutter/flutter#126118
Bug: flutter/flutter#125983
Bug: flutter/flutter#126116

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.

Release_build property is used to signal that this build produces
release artifacts that will eventually be SLSA compliant and will need
to run from dart_internal.

The os property is used to select the correct drone based on the
platform the build should run on.

Bug: flutter/flutter#126118
Bug: flutter/flutter#125983
Bug: flutter/flutter#126116
.ci.yaml Outdated
@@ -259,41 +259,53 @@ targets:
bringup: true
recipe: engine_v2/engine_v2
timeout: 60
dimensions:
Copy link
Contributor

Choose a reason for hiding this comment

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

Shall this be drone_dimensions?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated

.ci.yaml Outdated
properties:
release_build: "true"
config_name: linux_fuchsia

- name: Linux linux_arm_host_engine
recipe: engine_v2/engine_v2
timeout: 60
dimensions:
os: "Linux"
Copy link
Contributor

Choose a reason for hiding this comment

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

We also use ubuntu, can we make them consistent?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, this property is being used to detect the drone names, if we use ubuntu it will translate to Ubuntu Production Drone

Copy link
Contributor

Choose a reason for hiding this comment

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

Done_dimensions are not determining the Platform of the drone builder. It is the platform of the orchestrator builder instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

My bad based on a local checkout which is not up to date.

Linux should be fine then. Could we keep the Mac ones with the version to enforce same config? We do have consistent all Mac-12 in our fleet as of now, but just in case we have different versions. This makes it consistent with all other os dimension.

@@ -500,7 +519,6 @@ targets:
recipe: engine_v2/engine_v2
timeout: 60
properties:
release_build: "true"
Copy link
Contributor

Choose a reason for hiding this comment

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

From the PR description:

Release_build property is used to signal that this build produces release artifacts that will eventually be SLSA compliant and will need to run from dart_internal.

I didn't quite follow why we need to remove here. Do we not want to run them against dart_internal any more?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Only builders generating artifacts will be running in dart-internal. Builders like "Mac impeller-cmake-example" are test only and should not run in dart-internal.

Copy link
Contributor

@keyonghan keyonghan left a comment

Choose a reason for hiding this comment

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

LGTM

@godofredoc godofredoc added the autosubmit Merge PR when tree becomes green via auto submit App label May 15, 2023
@auto-submit auto-submit bot merged commit 027ca79 into flutter:main May 15, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 15, 2023
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request May 15, 2023
…126876)

flutter/engine@00f20fb...027ca79

2023-05-15 [email protected] Use release_build and os dimension consistently. (flutter/engine#42012)
2023-05-15 [email protected] [Impeller] Add interactive DrawPaint blend test (flutter/engine#42031)
2023-05-15 [email protected] [Impeller] Limit subpass textures and backdrop blurs to the current clip (flutter/engine#42039)
2023-05-15 [email protected] Roll Dart SDK from c302a0252785 to d2b2ac829842 (1 revision) (flutter/engine#42051)
2023-05-15 [email protected] Upload xcresults to LUCI cloud storage (flutter/engine#41647)

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 add_os_dimension branch May 15, 2023 22:21
godofredoc added a commit that referenced this pull request May 15, 2023
CP: 027ca79

## 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
…lutter#126876)

flutter/engine@00f20fb...027ca79

2023-05-15 [email protected] Use release_build and os dimension consistently. (flutter/engine#42012)
2023-05-15 [email protected] [Impeller] Add interactive DrawPaint blend test (flutter/engine#42031)
2023-05-15 [email protected] [Impeller] Limit subpass textures and backdrop blurs to the current clip (flutter/engine#42039)
2023-05-15 [email protected] Roll Dart SDK from c302a0252785 to d2b2ac829842 (1 revision) (flutter/engine#42051)
2023-05-15 [email protected] Upload xcresults to LUCI cloud storage (flutter/engine#41647)

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
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
autosubmit Merge PR when tree becomes green via auto submit App
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants