Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[pigeon] Support other hosts in generated file CI checks #5944

Merged
merged 16 commits into from
Jan 20, 2024
Merged

[pigeon] Support other hosts in generated file CI checks #5944

merged 16 commits into from
Jan 20, 2024

Conversation

stuartmorgan
Copy link
Contributor

Reworks Pigeon's CI validation of generated files to support multiple hosts, rather than only Linux, with the ability to set specific languages per host, to allow us to run validation for each language on whatever host it is most convenient to set up a formatter for that language, rather than having to support all languages on Linux.

See discussion in #5928

Part of flutter/flutter#41129

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 relevant style guides and ran the auto-formatter. (Unlike the flutter/flutter repo, the flutter/packages repo does use dart format.)
  • I signed the CLA.
  • The title of the PR starts with the name of the package surrounded by square brackets, e.g. [shared_preferences]
  • I listed at least one issue that this PR fixes in the description above.
  • I updated pubspec.yaml with an appropriate new version according to the pub versioning philosophy, or this PR is exempt from version changes.
  • I updated CHANGELOG.md to add a description of the change, following repository CHANGELOG style.
  • 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.

@stuartmorgan stuartmorgan requested a review from jmagman January 19, 2024 18:00
@flutter-dashboard
Copy link

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact "@test-exemption-reviewer" in the #hackers channel in Chat (don't just cc them here, they won't see it! Use Discord!).

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

@stuartmorgan stuartmorgan mentioned this pull request Jan 19, 2024
Merged
11 tasks
@stuartmorgan
Copy link
Contributor Author

test-exempt: is a test

jmagman
jmagman approved these changes Jan 19, 2024
View reviewed changes
packages/pigeon/tool/run_tests.dart Outdated
};
} else if (Platform.isMacOS) {
languagesToValidate = <GeneratorLanguage>{
// TODO(stuartmorgan): Enable this in or after https://github.com/flutter/packages/pull/5928
Copy link
Member

Choose a reason for hiding this comment

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

Merged! #5928 (comment)

tarrinneal
tarrinneal reviewed Jan 20, 2024
View reviewed changes
packages/pigeon/tool/run_tests.dart Outdated
Comment on lines 141 to 146
GeneratorLanguage.cpp => <String>{'cc', 'cpp', 'h'},
GeneratorLanguage.dart => <String>{'dart'},
GeneratorLanguage.java => <String>{'java'},
GeneratorLanguage.kotlin => <String>{'kt'},
GeneratorLanguage.swift => <String>{'swift'},
GeneratorLanguage.objc => <String>{'m', 'mm'},
Copy link
Contributor

Choose a reason for hiding this comment

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

how are we handling .h files for objc?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oops! They should be listed here; having duplicates will be fine since.

@stuartmorgan stuartmorgan added the override: no changelog needed Override the check requiring CHANGELOG updates for most changes label Jan 20, 2024
@stuartmorgan
Copy link
Contributor Author

changelog override: the only non-test change is formatting of an example file, which isn't going to be interesting to clients.

@stuartmorgan stuartmorgan added the override: no versioning needed Override the check requiring version bumps for most changes label Jan 20, 2024
@stuartmorgan
Copy link
Contributor Author

version override: quick_actions code changes are just formatting, so do not affect clients.

@stuartmorgan stuartmorgan added the autosubmit Merge PR when tree becomes green via auto submit App label Jan 20, 2024
@auto-submit auto-submit bot merged commit 2bb7d14 into flutter:main Jan 20, 2024
80 checks passed
PROGrand added a commit to mtbo-org/packages that referenced this pull request Jan 21, 2024
@PROGrand
Manual roll Flutter from f77f824 to 684247a (39 revisions) (flutter#5948
5409ad9 
)

Manual roll Flutter from f77f824 to 684247a (39 revisions)

Manual roll requested by [email protected]

flutter/flutter@f77f824...684247a

2024-01-19 [email protected] Use Integer instead of int in map in flutter.groovy  (flutter/flutter#141895)
2024-01-19 [email protected] Roll Flutter Engine from c953c83112ba to f2b441a26416 (4 revisions) (flutter/flutter#141894)
2024-01-19 [email protected] Native assets: package in framework on iOS and MacOS (flutter/flutter#140907)
2024-01-19 [email protected] Revert "Make tests more resilient to Skia gold failures and refactor flutter_goldens for extensive technical debt removal (#140101)" (flutter/flutter#141814)
2024-01-19 [email protected] Roll Flutter Engine from 538975f2511b to c953c83112ba (3 revisions) (flutter/flutter#141886)
2024-01-19 [email protected] Add `showDragHandle` to `showBottomSheet` (flutter/flutter#141754)
2024-01-19 [email protected] Roll Flutter Engine from 9a6c64de8a46 to 538975f2511b (8 revisions) (flutter/flutter#141881)
2024-01-19 [email protected] Make pumpWidget's arguments named (flutter/flutter#141728)
2024-01-19 [email protected] Tiny fix inaccurate documentations about bindings (flutter/flutter#140282)
2024-01-19 [email protected] Roll engine to 9a6c64de8a4694cef59a338cd33ac1a9e7d23d9d (flutter/flutter#141870)
2024-01-19 [email protected] Roll Packages from 83c2c4d to 129e08c (13 revisions) (flutter/flutter#141865)
2024-01-19 [email protected] Add mac_x64_ios configuration. (flutter/flutter#141828)
2024-01-19 [email protected] Roll Flutter Engine from 90be25d8aac3 to d1afda52d254 (1 revision) (flutter/flutter#141825)
2024-01-19 [email protected] Move the requestKeyboard up to the widgets layer (flutter/flutter#141655)
2024-01-19 [email protected] Roll Flutter Engine from dde3ebf6551a to 90be25d8aac3 (1 revision) (flutter/flutter#141817)
2024-01-18 [email protected] enable more tests in web mode (flutter/flutter#141791)
2024-01-18 [email protected] Roll Flutter Engine from 9dded186bcff to dde3ebf6551a (2 revisions) (flutter/flutter#141811)
2024-01-18 [email protected] Update margin between label and icon in Tab to better reflect Material specs (flutter/flutter#140698)
2024-01-18 [email protected] Roll Flutter Engine from 3106e08e1219 to 9dded186bcff (2 revisions) (flutter/flutter#141807)
2024-01-18 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Enable native compilation for windows-arm64 " (flutter/flutter#141809)
2024-01-18 [email protected] Run framework_tests_misc in arm64 and x64. (flutter/flutter#141797)
2024-01-18 [email protected] Roll Flutter Engine from f4a4f046b173 to 3106e08e1219 (1 revision) (flutter/flutter#141802)
2024-01-18 49699333+dependabot[bot]@users.noreply.github.com Bump actions/upload-artifact from 4.1.0 to 4.2.0 (flutter/flutter#141803)
2024-01-18 [email protected] Update labeler.yml (flutter/flutter#141697)
2024-01-18 [email protected] Roll Flutter Engine from 75400c49fa0b to f4a4f046b173 (2 revisions) (flutter/flutter#141800)
2024-01-18 [email protected] Reland "Remove hack from PageView." (flutter/flutter#141533)
2024-01-18 [email protected] ScaleGestureRecognizer pointerCount=2 for trackpad gestures (flutter/flutter#140745)
2024-01-18 [email protected] Roll Flutter Engine from de68e7612948 to 75400c49fa0b (2 revisions) (flutter/flutter#141796)
2024-01-18 [email protected] Run `flutter_gallery_ios__start_up` test on Mac-14 in staging (flutter/flutter#141795)
2024-01-18 [email protected] Roll Flutter Engine from d80fe1cb5854 to de68e7612948 (1 revision) (flutter/flutter#141789)
2024-01-18 [email protected] Enable native compilation for windows-arm64  (flutter/flutter#137618)
2024-01-18 [email protected] [github actions] Fix token issue on actions/checkout package (flutter/flutter#141652)
2024-01-18 [email protected] Roll Flutter Engine from b75d6d80d813 to d80fe1cb5854 (2 revisions) (flutter/flutter#141785)
2024-01-18 [email protected] Revert "Native assets: roll deps" (flutter/flutter#141748)
2024-01-18 [email protected] Deprecate M2 curves (flutter/flutter#134417)
2024-01-18 [email protected] Fix: TextField can inherit `errorStyle` from `InputDecorationTheme`. (flutter/flutter#141227)
2024-01-18 [email protected] Add check for Bank of Brazil security module to Windows Flutter Doctor validators (flutter/flutter#141135)
2024-01-18 [email protected] Fix gradle lints No semantic change should be present. (flutter/flutter#141692)
2024-01-18 [email protected] Roll Packages from 1a2b780 to 83c2c4d (5 revisions) (flutter/flutter#141778)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages
Please CC [email protected],[email protected],[email protected] on the revert to ensure that a human
...
[ci] Run Swift formatter and linter during CI formatting (flutter#5928)

Make `swift-format` CIPD package available on the `PATH` https://flutter-review.googlesource.com/c/recipes/+/54020

Run `format --no-clang-format --no-java --no-kotlin --no-dart`  on the macOS builder so it doesn't duplicate same `format` call run on Linux.

Filter out generated files until flutter/flutter#141799 is done.

Also add `swift-format lint` call during `format` command.  Fix the one casing issue it found in a test file.

Failing run:
https://logs.chromium.org/logs/flutter/buildbucket/cr-buildbucket/8758535881172004177/+/u/Run_package_tests/Swift_format/stdout

Successful run:
https://logs.chromium.org/logs/flutter/buildbucket/cr-buildbucket/8758492350529121249/+/u/Run_package_tests/Swift_format/stdout

Fixes flutter/flutter#41129
non-breaking change: increment lower component.

[camera]fix a sample buffer memory leak on pause resume recording (flutter#5927)

This memory leak happens when pause/resume the recording. Depending on camera resolution, about 1-2MB leaks for every resume, so it can add up pretty fast if we frequently pause/resume.

We only reference this sample buffer inside the scope of this method (e.g. directly piping into file IO), so no need to retain it again, as it's already retained and will be released afterwards by apple. (We did keep around the pixel buffer, but we already manually [retain](https://github.com/flutter/packages/blob/d7ee75ad59ad7bc45e659d0599e935e9e7981ea1/packages/camera/camera_avfoundation/ios/Classes/FLTCam.m#L403) and [release](https://github.com/flutter/packages/blob/d7ee75ad59ad7bc45e659d0599e935e9e7981ea1/packages/camera/camera_avfoundation/ios/Classes/FLTCam.m#L415) that)

Bug was introduced in flutter/plugins#1370.

*List which issues are fixed by this PR. You must list at least one issue.*

flutter/flutter#132013

*If you had to change anything in the [flutter/tests] repo, include a link to the migration guide as per the [breaking change policy].*
implemented using RecordSettings with std::optional.

RecorderSettings native test added.

suggestions applied

Merge branch 'main' into camera-with-settings-platform-implementations
 indicates a new feature< according to https://dart.dev/tools/pub/versioning#semantic-versions

splitted .having

Roll Flutter from 684247a to ddf60fb (12 revisions) (flutter#5949)

flutter/flutter@684247a...ddf60fb

2024-01-20 [email protected] Roll Flutter Engine from 1385f76018ab to 704ef3399012 (1 revision) (flutter/flutter#141932)
2024-01-20 [email protected] Roll Flutter Engine from 6f6649bdfef2 to 1385f76018ab (1 revision) (flutter/flutter#141926)
2024-01-20 [email protected] Roll Flutter Engine from 4d67f267b115 to 6f6649bdfef2 (1 revision) (flutter/flutter#141920)
2024-01-20 [email protected] Roll Flutter Engine from 53a2436cf75c to 4d67f267b115 (1 revision) (flutter/flutter#141917)
2024-01-20 [email protected] Roll Flutter Engine from a9b87c6288d2 to 53a2436cf75c (1 revision) (flutter/flutter#141916)
2024-01-20 [email protected] Roll Flutter Engine from d00e55f7f8ed to a9b87c6288d2 (3 revisions) (flutter/flutter#141914)
2024-01-20 [email protected] Remove more textScaleFactor references (flutter/flutter#141816)
2024-01-19 [email protected] Roll Flutter Engine from 517fc2bbfa51 to d00e55f7f8ed (1 revision) (flutter/flutter#141909)
2024-01-19 [email protected] Roll Flutter Engine from f2b441a26416 to 517fc2bbfa51 (2 revisions) (flutter/flutter#141904)
2024-01-19 [email protected] Fix "Delete" tooltip is shown disabled on chips with `onDeleted` callback (flutter/flutter#141770)
2024-01-19 [email protected] Run module_test_ios on arm and x64 (flutter/flutter#141815)
2024-01-19 [email protected] `SearchAnchor` search view clear button only shows up when text input is not empty (flutter/flutter#141755)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages
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 Packages: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
[pigeon] Improve style of generated Swift code (flutter#5938)

Cleans up our Swift output to be more idiomatic (or in a couple of cases to match swift-format` in particular):
- Don't indent `case`s in `switch`es.
- Remove some stray `;`s
- Remove `()`s around `if` conditions
- Add trailing newlines in multi-element arrays
- Fix some missing standard whitespace in a handful of places where it was missing (after named arguments, between `()` and `{`)
- Indent inside `#if` constructs.
- Two changes that are more arguable, as it's not clear that they are non-idiomatic vs just non-`swift-format`-style (and they add branching to the generator that would otherwise not be needed):
  - Don't put a trailing comma on the list element of a one-element list
  - Don't put `()` on zero-arg method calls with a trailing closure

With these changes, as far as can tell the diffs created by `swift-format` are just line wrapping.

Fixes flutter/flutter#141799
Add stubs to satisfy [FLTCamSampleBufferTests testDidOutputSampleBuffer_mustNotChangeSampleBufferRetainCountAfterPauseResumeRecording] test error.

[FLTCamSampleBufferTests testDidOutputSampleBuffer_mustNotChangeSampleBufferRetainCountAfterPauseResumeRecording] : *** -[AVAssetWriterInputPixelBufferAdaptor initWithAssetWriterInput:sourcePixelBufferAttributes:] invalid parameter not satisfying: [[input mediaType] isEqualToString:AVMediaTypeVideo] || [[input mediaType] isEqualToString:AVMediaTypeAuxiliaryPicture] (NSInvalidArgumentException)

Merge branch 'main' into camera-with-settings-platform-implementations
[pigeon] Support other hosts in generated file CI checks (flutter#5944)

Reworks Pigeon's CI validation of generated files to support multiple hosts, rather than only Linux, with the ability to set specific languages per host, to allow us to run validation for each language on whatever host it is most convenient to set up a formatter for that language, rather than having to support all languages on Linux.

See discussion in flutter#5928

Part of flutter/flutter#41129
optional integer testing

renamed to toPositiveOrNull

commented behaviour

Merge remote-tracking branch 'origin/camera-with-settings-platform-implementations' into camera-with-settings-platform-implementations

Merge branch 'main' into camera-with-settings-platform-implementations
new feature: increment last version number part.

Update platform label rules for shared iOS/macOS (flutter#5801)

Update automatic platform label rules for more implementations that are
shared between iOS and macOS.
Merge branch 'main' into camera-with-settings-platform-implementations
@stuartmorgan stuartmorgan deleted the pigeon-format-check branch January 22, 2024 13:04
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Jan 22, 2024
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jan 22, 2024
@engine-flutter-autoroll
2bb7d14 [pigeon] Support other hosts in generated file CI checks (flu…
ecee838 
…tter/packages#5944)
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Jan 22, 2024
@engine-flutter-autoroll
Roll Packages from 129e08c to e4cbf23 (11 revisions) (#141962)
bfa245f 
flutter/packages@129e08c...e4cbf23

2024-01-21 [email protected] Roll Flutter from ddf60fb to 5dea6b9 (5 revisions) (flutter/packages#5951)
2024-01-21 [email protected] Update platform label rules for shared iOS/macOS (flutter/packages#5801)
2024-01-20 [email protected] [pigeon] Support other hosts in generated file CI checks (flutter/packages#5944)
2024-01-20 [email protected] [pigeon] Improve style of generated Swift code (flutter/packages#5938)
2024-01-20 [email protected] Roll Flutter from 684247a to ddf60fb (12 revisions) (flutter/packages#5949)
2024-01-20 [email protected] [camera]fix a sample buffer memory leak on pause resume recording (flutter/packages#5927)
2024-01-19 [email protected] [ci] Run Swift formatter and linter during CI formatting (flutter/packages#5928)
2024-01-19 [email protected] Manual roll Flutter from f77f824 to 684247a (39 revisions) (flutter/packages#5948)
2024-01-19 [email protected] Expose registered widget libraries and local widget library widgets. (flutter/packages#5936)
2024-01-19 49699333+dependabot[bot]@users.noreply.github.com Bump actions/upload-artifact from 4.1.0 to 4.2.0 (flutter/packages#5937)
2024-01-19 [email protected] Manual roll Flutter (stable) from ef1af02 to 67457e6 (1 revision) (flutter/packages#5932)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages-flutter-autoroll
Please CC [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://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
@reidbaker reidbaker mentioned this pull request Feb 19, 2024
Merged
16 tasks
arc-yong pushed a commit to Arctuition/packages-arc that referenced this pull request Jun 14, 2024
@stuartmorgan @arc-yong
[pigeon] Support other hosts in generated file CI checks (flutter#5944)
ed0ec2f 
Reworks Pigeon's CI validation of generated files to support multiple hosts, rather than only Linux, with the ability to set specific languages per host, to allow us to run validation for each language on whatever host it is most convenient to set up a formatter for that language, rather than having to support all languages on Linux.

See discussion in flutter#5928

Part of flutter/flutter#41129
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tarrinneal tarrinneal tarrinneal approved these changes

@jmagman jmagman jmagman approved these changes

@hellohuanlin hellohuanlin Awaiting requested review from hellohuanlin hellohuanlin is a code owner

Assignees
No one assigned
Labels
autosubmit Merge PR when tree becomes green via auto submit App override: no changelog needed Override the check requiring CHANGELOG updates for most changes override: no versioning needed Override the check requiring version bumps for most changes p: pigeon p: quick_actions platform-ios platform-macos
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@stuartmorgan @jmagman @tarrinneal