Repository navigation
[pigeon] add version checking for objective_c and jni dependencies - #13283
Merged
Merged
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request adds a verification step to ensure that the objective_c dependency constraint in pubspec.yaml matches the ObjCVersionCheck version from the generated .ffi.dart file, outputting a warning if they are incompatible. Feedback suggests improving this check by handling cases where the dependency is defined as a map in pubspec.yaml and extracting its nested version key.
Comment on lines
+2762
to
+2768
| final Object? rawConstraint = | ||
| dependencyOverrides != null && dependencyOverrides.containsKey('objective_c') | ||
| ? dependencyOverrides['objective_c'] | ||
| : dependencies?['objective_c']; | ||
| if (rawConstraint is! String) { | ||
| return null; | ||
| } |
Contributor
There was a problem hiding this comment.
If the objective_c dependency is specified as a map (for example, when using a hosted dependency with a custom source or other map-based configurations), rawConstraint will be a Map rather than a String. In such cases, the version constraint is typically nested under the version key. Checking if rawConstraint is a Map and extracting the version key ensures that version validation still runs correctly for these configurations.
Suggested change
| final Object? rawConstraint = | |
| dependencyOverrides != null && dependencyOverrides.containsKey('objective_c') | |
| ? dependencyOverrides['objective_c'] | |
| : dependencies?['objective_c']; | |
| if (rawConstraint is! String) { | |
| return null; | |
| } | |
| Object? rawConstraint = | |
| dependencyOverrides != null && dependencyOverrides.containsKey('objective_c') | |
| ? dependencyOverrides['objective_c'] | |
| : dependencies?['objective_c']; | |
| if (rawConstraint is Map) { | |
| rawConstraint = rawConstraint['version']; | |
| } | |
| if (rawConstraint is! String) { | |
| return null; | |
| } |
tarrinneal
changed the title
davidmorgan
self-requested a review
October 16, 2026 09:32
davidmorgan
approved these changes
Oct 16, 2026
davidmorgan
left a comment
davidmorgan
left a comment
There was a problem hiding this comment.
LG, thanks, a few comments.
Pre-existing, not introduced here, but the new check inherits it: the !dartOut.startsWith(basePath) prefix test, also in ffigen_config_generator.dart and jnigen_config_generator.dart, is a string-prefix test, while _getFile always writes the Dart output to path.posix.join(basePath, output). With basePath: 'app' and dartOut: 'app_bindings/lib/messages.g.dart', the Dart file goes to app/app_bindings/lib/ but the .ffi.dart/.jni.dart file, and this check, use app_bindings/lib/. path.isWithin would avoid it; probably worth its own issue.
Minor edge: >=9.6.0 <=10.0.0-dev.1 gets no warning even though it allows 10.0.0-dev.1, because VersionRange(min: 10.0.0, includeMin: true) starts after the 10.0.0 prereleases. Feel free to ignore as this is unlikely to actually happen :)
10 of 11 tasks
tarrinneal
added
the
autosubmit
bkonyi
pushed a commit
to bkonyi/flutter
that referenced
this pull request
Oct 20, 2026
flutter/packages@10a178c...618afba 2026-10-20 [email protected] [pigeon] add version checking for objective_c and jni dependencies (flutter/packages#13283) 2026-10-19 [email protected] [go_router_builder] Adopt shared analyzer helpers from source_helper: ^1.4.0 (flutter/packages#13337) 2026-10-19 [email protected] [pigeon] fix instance manager test flake (flutter/packages#13303) 2026-10-19 [email protected] [vector_graphics] Don't report a handled loading error as unhandled (flutter/packages#13315) 2026-10-19 [email protected] [file_selector_macos] Drop deprecated allowedFileTypes code path (flutter/packages#13302) 2026-10-19 [email protected] [shared_preferences_android] Update androidx.datastore:datastore to 1.2.1 (flutter/packages#13334) 2026-10-19 [email protected] [in_app_purchase_storekit] Suppress StoreKit 1 deprecation warnings (flutter/packages#13301) 2026-10-19 [email protected] [camera_avfoundation] Fix Swift analyzer issue _captureSessionQueue (flutter/packages#13332) 2026-10-19 [email protected] [material_ui] Unskip web tests that pass on CanvasKit and Skwasm: color_scheme_test, elevated_button_test, filled_button_test, icon_button_test (flutter/packages#13327) 2026-10-19 [email protected] [in_app_purchase_android] Update to Play Billing Library 9.1.0 (flutter/packages#12781) 2026-10-19 [email protected] [webview_flutter_wkwebview] Require iOS 15 / macOS 12 and drop deprecated SecTrustGetCertificateAtIndex code path (flutter/packages#13294) If this roll has caused a breakage, revert this CL and set the roller to dry run mode using the controls here: https://autoroll.skia.org/r/flutter-packages-flutter-autoroll Please CC [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
suryaKommana2662
pushed a commit
to suryaKommana2662/flutter
that referenced
this pull request
Oct 29, 2026
flutter/packages@10a178c...618afba 2026-10-20 [email protected] [pigeon] add version checking for objective_c and jni dependencies (flutter/packages#13283) 2026-10-19 [email protected] [go_router_builder] Adopt shared analyzer helpers from source_helper: ^1.4.0 (flutter/packages#13337) 2026-10-19 [email protected] [pigeon] fix instance manager test flake (flutter/packages#13303) 2026-10-19 [email protected] [vector_graphics] Don't report a handled loading error as unhandled (flutter/packages#13315) 2026-10-19 [email protected] [file_selector_macos] Drop deprecated allowedFileTypes code path (flutter/packages#13302) 2026-10-19 [email protected] [shared_preferences_android] Update androidx.datastore:datastore to 1.2.1 (flutter/packages#13334) 2026-10-19 [email protected] [in_app_purchase_storekit] Suppress StoreKit 1 deprecation warnings (flutter/packages#13301) 2026-10-19 [email protected] [camera_avfoundation] Fix Swift analyzer issue _captureSessionQueue (flutter/packages#13332) 2026-10-19 [email protected] [material_ui] Unskip web tests that pass on CanvasKit and Skwasm: color_scheme_test, elevated_button_test, filled_button_test, icon_button_test (flutter/packages#13327) 2026-10-19 [email protected] [in_app_purchase_android] Update to Play Billing Library 9.1.0 (flutter/packages#12781) 2026-10-19 [email protected] [webview_flutter_wkwebview] Require iOS 15 / macOS 12 and drop deprecated SecTrustGetCertificateAtIndex code path (flutter/packages#13294) If this roll has caused a breakage, revert this CL and set the roller to dry run mode using the controls here: https://autoroll.skia.org/r/flutter-packages-flutter-autoroll Please CC [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
auto-submit Bot
pushed a commit
that referenced
this pull request
Nov 11, 2026
…h basePath (#13342) Replaces string-prefix `!....startsWith(basePath)` checks across `ffigen_config_generator.dart`, `jnigen_config_generator.dart`, `pigeon_lib.dart`, and `pigeon_lib_internal.dart` with a path-aware `isWithinOrEquals(basePath, ...)` helper using `path.posix.equals` and `path.posix.isWithin`. Previously, when `basePath` was set (e.g. `basePath: 'app'`) and an output or directory path happened to start with the same string prefix without being inside `basePath` (e.g. `dartOut: 'app_bindings/lib/src/messages.g.dart'`), `.startsWith(basePath)` evaluated to `true` and skipped prepending `basePath`, while `_getFile` unconditionally joined `basePath` and `output`. Follow-up from #13283 (review).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
To avoid surprising build errors