Skip to content

Conversation

@liamappelbe
Copy link
Contributor

@liamappelbe liamappelbe commented May 26, 2025

Notes

  • I'm using on @Native annotations to load all the ffi symbols. Previously this worked because I loaded the dylib into the executable first, then relied on the @Native fallback behavior to load the symbols from the exe. Now I'm explicitly setting the asset name to the built dylib, which appears to disable that fallback behavior. But all the core ObjC runtime symbols (eg objc_msgSend) are linked into the exe, not the dylib. So I had to split the C bindings into a set that loads from the dylib, and a set that loads from the exe. There are now 3 sets of ffigen created bindings: the objective C bindings (loads from the dylib), the bindings for my C code (loads from the dylib), and the bindings for the built in ObjC runtime's C code (loads from the exe).
  • Similarly, I had an issue where the global variables exposed by the ObjC bindings were failing to load. So I moved them to the C runtime bindings so they'd be loaded from the exe. This exposed them as a raw Pointer<ObjCObject>, so I had to manually wrap them in NSStrings in globals.dart. I tried splitting the ObjC bindings like I split the C bindings, but this caused all sorts of other problems.
  • ffigen has a dev dep on package:objective_c for testing. All the ObjC tests are disabled on linux and windows, but afaik there's no way of avoiding pulling in the dependency on those platforms. So I had to modify package:objective_c's build script to no-op on those platforms. Seems like a sub-optimal solution.

TODO: Mac CI failures are due to running the tests through package:coverage (dart-lang/tools#2237). Need to investigate why that's not finding the dylib. The mac tests not running through test_with_coverage are working. If this isn't a quick fix I can temporarily disable coverage collection for these tests.

@github-actions
Copy link

github-actions bot commented May 26, 2025

PR Health

License Headers ⚠️
// Copyright (c) 2025, the Dart project authors. Please see the AUTHORS file
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.
Files
pkgs/objective_c/example/lib/main.dart
pkgs/objective_c/lib/src/ns_input_stream.dart

All source files should start with a license header.

This check can be disabled by tagging the PR with skip-license-check.

API leaks ⚠️

The following packages contain symbols visible in the public API, but not exported by the library. Export these symbols or remove them from your publicly visible API.

Package Leaked API symbol Leaking sources
objective_c _FinalizablePointer internal.dart::_ObjCReference::new::_finalizable

This check can be disabled by tagging the PR with skip-leaking-check.

Breaking changes ⚠️
Package Change Current Version New Version Needed Version Looking good?
objective_c Breaking 9.0.0 9.0.0 10.0.0
Got "9.0.0" expected >= "10.0.0" (breaking changes)
⚠️

This check can be disabled by tagging the PR with skip-breaking-check.

Changelog Entry
Package Changed Files
package:objective_c pkgs/objective_c/example/README.md
pkgs/objective_c/example/ios/Runner/Assets.xcassets/LaunchImage.imageset/README.md
pkgs/objective_c/example/pubspec.yaml
pkgs/objective_c/lib/objective_c.dart
pkgs/objective_c/lib/src/autorelease.dart
pkgs/objective_c/lib/src/c_bindings_generated.dart
pkgs/objective_c/lib/src/globals.dart
pkgs/objective_c/lib/src/internal.dart
pkgs/objective_c/lib/src/ns_input_stream.dart
pkgs/objective_c/lib/src/ns_string.dart
pkgs/objective_c/lib/src/objective_c_bindings_exported.dart
pkgs/objective_c/lib/src/objective_c_bindings_generated.dart
pkgs/objective_c/lib/src/protocol_builder.dart
pkgs/objective_c/lib/src/runtime_bindings_generated.dart
pkgs/objective_c/lib/src/selector.dart
pkgs/objective_c/pubspec.yaml

Changes to files need to be accounted for in their respective changelogs.

This check can be disabled by tagging the PR with skip-changelog-check.

@github-actions github-actions bot added the type-infra A repository infrastructure change or enhancement label Jun 13, 2025
@coveralls
Copy link

coveralls commented Jun 18, 2025

Coverage Status

coverage: 82.093% (+0.1%) from 81.971%
when pulling c69e2f2 on objc_native
into 8215af2 on main.

@liamappelbe liamappelbe changed the title WIP: [objective_c] Migrate to native assets [objective_c] Migrate to native assets Nov 6, 2025
@liamappelbe liamappelbe marked this pull request as ready for review November 6, 2025 08:56
@liamappelbe liamappelbe requested a review from dcharkes November 6, 2025 08:57
@liamappelbe
Copy link
Contributor Author

Still working on fixing CI, but this is ready for review.

Comment on lines +67 to +70
// ignore_for_file: always_specify_types
// ignore_for_file: camel_case_types
// ignore_for_file: non_constant_identifier_names
// ignore_for_file: unused_element
Copy link
Contributor

Choose a reason for hiding this comment

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

fly-by comment: Do you need all these specific ignores given that ffigen generates a generic ignore for all lints?

Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like these ignores snug back in? Or are they for some reason actually needed?

Copy link
Contributor

@goderbauer goderbauer Nov 6, 2025

Choose a reason for hiding this comment

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

For my own understanding: Is it expected that stuff in the temp sub directory is checked in? If so, should the directory be named something else?

(There are some other temp directories further down as well)

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, there's supposed to be gitignores preventing these from being added. Not sure how this was missed.

@dcharkes
Copy link
Collaborator

dcharkes commented Nov 6, 2025

  • ffigen has a dev dep on package:objective_c for testing. All the ObjC tests are disabled on linux and windows, but afaik there's no way of avoiding pulling in the dependency on those platforms. So I had to modify package:objective_c's build script to no-op on those platforms. Seems like a sub-optimal solution.

That is tracked in:

Mac CI failures are due to running the tests through package:coverage. Need to investigate why that's not finding the dylib.

That sounds like something we should fix before build hooks get to stable! Let me know if you need any help with this.

Copy link
Collaborator

@dcharkes dcharkes left a comment

Choose a reason for hiding this comment

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

Very nice to see all the flutter boiler plate removed!

- uses: subosito/flutter-action@fd55f4c5af5b953cc57a2be44cb082c8f6635e8e
with:
channel: 'stable'
channel: master
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can this be beta?
Or do you intend to wait to merge this PR until the next stable is out, and merge it as stable?

Maybe add a todo to flip it back to stable as soon as the next stable is released.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, this is a temporary change so I can test on CI. I'll wait for stable and revert these changes before landing.

Copy link
Contributor

Choose a reason for hiding this comment

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

Since you removed the other temp files just double-checking: Is this file and pkgs/swiftgen/test/integration/temp/classes_output.dart supposed to be here?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants