Skip to content

Conversation

@Piinks
Copy link
Contributor

@Piinks Piinks commented Dec 7, 2023

Fixes #139673
Cherry picks #139706 to the stable branch to fix the tree.

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, including Features we expect every widget to implement.
  • I signed the CLA.
  • I listed at least one issue that this PR fixes in the description above.
  • I updated/added relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making, or this PR is test-exempt.
  • All existing and new tests are passing.

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

Part of fixing flutter#139673, it will need to be cherry picked into the stable branch to fully resolve.

Originally I tried to come at this from `ci.yaml`, but the syntax does not exist to either conditionally include a dependency based on the branch we are on, or to disable a shard for a given branch (as opposed to enabling it which is supported). I could double every CI shard that uses Gold to try to serve my purpose, but it is already a very large and cumbersome file to keep up to date. Doubling it does not feel like the best solution. Using a RegEx is not my favorite, but I am using the same one we use in our CI, and left a note there to update it if it should ever change. Since there is already a whole infra built around it, I feel it is pretty safe so we can fix the stable tree.

We already had mitigated Gold affecting release branches in the past through flutter/cocoon (flutter#58814), but flutter#139673 exposed a rather rare edge case. A change was CP'd into the stable branch that introduced golden file image changes. Typically this would not cause an issue since any change that has landed on the master branch has golden files accounted for. In this case, the CP'd change on master has generated a different image on canvaskit due to another change that was not on stable. So when the CP landed, it generated a new image Gold had never seen before. Gold only tracks the master branch, so we cannot approve the image, and so cannot fix the stable tree.

This would disable the failing check on release branches and fix the tree.
@Piinks Piinks requested a review from CaseyHillers December 7, 2023 20:59
@github-actions github-actions bot added a: tests "flutter test", flutter_test, or one of our tests framework flutter/packages/flutter repository. See also f: labels. labels Dec 7, 2023
@CaseyHillers CaseyHillers added the autosubmit Merge PR when tree becomes green via auto submit App label Dec 7, 2023
@auto-submit auto-submit bot merged commit 54f1b88 into flutter:flutter-3.16-candidate.0 Dec 7, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Dec 13, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Dec 14, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Dec 14, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Dec 15, 2023
bryanoltman added a commit to shorebirdtech/flutter that referenced this pull request Dec 20, 2023
* [CP] Gold fix for stable branch (flutter#139764)

Fixes flutter#139673
Cherry picks flutter#139706 to the stable branch to fix the tree.

* [macOS] Suppress Xcode 15 createItemModels warning (flutter#138243) (flutter#139782)

As of Xcode 15 on macOS Sonoma, the following message is (repeatedly) output to stderr during builds (repros on non-Flutter apps). It is supppressed in xcode itself, but not when run from the command-line.

```
2023-11-10 10:44:58.031 xcodebuild[61115:1017566] [MT] DVTAssertions: Warning in /System/Volumes/Data/SWE/Apps/DT/BuildRoots/BuildRoot11/ActiveBuildRoot/Library/Caches/com.apple.xbs/Sources/IDEFrameworks/IDEFrameworks-22267/IDEFoundation/Provisioning/Capabilities Infrastructure/IDECapabilityQuerySelection.swift:103
Details:  createItemModels creation requirements should not create capability item model for a capability item model that already exists.
Function: createItemModels(for:itemModelSource:)
Thread:   <_NSMainThread: 0x6000027c0280>{number = 1, name = main}
Please file a bug at https://feedbackassistant.apple.com with this warning message and any useful information you can provide.
```

This suppresses this message from stderr in our macOS build logs.

Issue: flutter#135277  
Cherry-pick: flutter#139284

https://github.com/flutter/flutter/wiki/Tree-hygiene#overview [Tree Hygiene]: https://github.com/flutter/flutter/wiki/Tree-hygiene [test-exempt]:
https://github.com/flutter/flutter/wiki/Tree-hygiene#tests [Flutter Style Guide]:
https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo [Features we expect every widget to implement]:
https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/
https://github.com/flutter/flutter/wiki/Tree-hygiene#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/wiki/Chat

*Replace this paragraph with a description of what this PR is changing or adding, and why. Consider including before/after screenshots.*

*List which issues are fixed by this PR. You must list at least one issue. An issue is not required if the PR fixes something trivial like a typo.*

*If you had to change anything in the [flutter/tests] repo, include a link to the migration guide as per the [breaking change policy].*

* [CP] have `Java.version` return null if `java --version` fails or cannot be run (flutter#139698)

cherry-picks changes from flutter#139614 onto the stable channel

* [CP] Catch error for missing directory in `FontConfigManager` (flutter#138496) (flutter#139743)

Closes:
- flutter#138434

We will catch any errors while attempting to clear the temp directories that don't exist for the `FontConfigManager` class

---------

Co-authored-by: Kate Lovett <[email protected]>
Co-authored-by: Chris Bracken <[email protected]>
Co-authored-by: Andrew Kolos <[email protected]>
Co-authored-by: Elias Yishak <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

a: tests "flutter test", flutter_test, or one of our tests autosubmit Merge PR when tree becomes green via auto submit App framework flutter/packages/flutter repository. See also f: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants