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

package:objective_c #1061

Merged
merged 24 commits into from
Apr 11, 2024
Merged

package:objective_c #1061

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
d0df41c
Initial commit of package:objective_c
liamappelbe Mar 28, 2024
c2036a1
Fix some of the PR health warnings
liamappelbe Apr 2, 2024
3557789
Remove the objective_c dep from ffigen. Need to separate those tests
liamappelbe Apr 2, 2024
9ea9ddb
Disable analysis and move tests
liamappelbe Apr 2, 2024
50a2ac5
Fix tests
liamappelbe Apr 4, 2024
1e8d650
Workflows
liamappelbe Apr 4, 2024
7c47494
Fix analysis
liamappelbe Apr 5, 2024
5249b79
Fix analysis
liamappelbe Apr 7, 2024
8d69c7f
Fix analysis
liamappelbe Apr 8, 2024
62d5340
Fix analysis
liamappelbe Apr 8, 2024
e48d38b
Fix analysis
liamappelbe Apr 8, 2024
09a4734
Fix analysis
liamappelbe Apr 8, 2024
c34d5f8
Fix tests
liamappelbe Apr 8, 2024
4dcd715
Fix tests and address comments
liamappelbe Apr 9, 2024
0396b3c
Improve ffigen's CHANGELOG entry
liamappelbe Apr 9, 2024
91aa71c
Merge branch 'main' into pkgobjc
liamappelbe Apr 9, 2024
a5d0d99
Gather objective_c coverage during ffigen tests
liamappelbe Apr 9, 2024
6413bab
Daco's comments
liamappelbe Apr 9, 2024
466166e
Merge branch 'main' into pkgobjc
liamappelbe Apr 10, 2024
85473c7
Move objc_msgSend into package:objective_c
liamappelbe Apr 11, 2024
2b67592
Merge branch 'pkgobjc' of github.com:dart-lang/native into pkgobjc
liamappelbe Apr 11, 2024
03a7693
Changelog
liamappelbe Apr 11, 2024
0b004e6
Fix analysis
liamappelbe Apr 11, 2024
ff7c4b8
Update changelog
liamappelbe Apr 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .github/ISSUE_TEMPLATE/objective_c.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
name: "package:objective_c"
about: "Create a bug or file a feature request against package:objective_c."
labels: "package:objective_c"
---

39 changes: 21 additions & 18 deletions .github/workflows/ffigen.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,17 +1,20 @@
name: ffigen

on:
# Run on PRs and pushes to the default branch.
# Run on PRs and pushes to the default branch, in either the ffigen directory,
# or the objective_c directory.
push:
branches: [main, stable]
paths:
- '.github/workflows/ffigen.yml'
- 'pkgs/ffigen/**'
- 'pkgs/objective_c/**'
pull_request:
branches: [main, stable]
paths:
- '.github/workflows/ffigen.yml'
- 'pkgs/ffigen/**'
- 'pkgs/objective_c/**'
schedule:
- cron: "0 0 * * 0"

Expand All @@ -27,25 +30,22 @@ jobs:
working-directory: pkgs/ffigen/
strategy:
fail-fast: false
matrix:
sdk: [dev]
# sdk: [3.3.0]
steps:
- uses: actions/checkout@9bb56186c3b09b4f86b1c65136769dd318469633
- uses: dart-lang/setup-dart@fedb1266e91cf51be2fdb382869461a434b920a3
- uses: subosito/flutter-action@44ac965b96f18d999802d4b807e3256d5a3f9fa1
with:
sdk: ${{ matrix.sdk }}
flutter-version: 3.19.0
channel: 'stable'
- id: install
name: Install dependencies
run: dart pub get && dart pub get --directory="example/shared_bindings"
run: flutter pub get && flutter pub get --directory="example/shared_bindings"
- name: Check formatting
run: dart format --output=none --set-exit-if-changed .
if: always() && steps.install.outcome == 'success'
- name: Build test dylib and bindings
run: dart test/setup.dart
- name: Analyze code
run: dart analyze --fatal-infos
if: always() && steps.install.outcome == 'success'
run: flutter analyze --fatal-infos

test-linux:
needs: analyze
Expand All @@ -55,11 +55,12 @@ jobs:
working-directory: pkgs/ffigen/
steps:
- uses: actions/checkout@9bb56186c3b09b4f86b1c65136769dd318469633
- uses: dart-lang/setup-dart@fedb1266e91cf51be2fdb382869461a434b920a3
- uses: subosito/flutter-action@44ac965b96f18d999802d4b807e3256d5a3f9fa1
with:
sdk: dev #3.3.0
flutter-version: 3.19.0
channel: 'stable'
- name: Install dependencies
run: dart pub get
run: flutter pub get
Copy link
Collaborator

Choose a reason for hiding this comment

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

Shouldn't the majority of steps not be able to run in Dart standalone?

In other words, developers working on FFIgen on Linux/Windows should be able to run things on Dart standalone right? Only the ObjectiveC tests, which only run on MacOS, require Flutter?

Can we keep the Windows and Linux configs on Dart standalone?

(The JNIgen yaml file only has pub for the examples, not for analyzing package:jnigen, etc.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The Linux/Windows tests can run without flutter, but pub get fails because of the package:objective_c dev dependency. I don't know of a way of declaring that dependency only for mac/ios.

https://github.com/dart-lang/native/actions/runs/8593210918/job/23544623224

- name: Install libclang-14-dev
run: sudo apt-get install libclang-14-dev
- name: Build test dylib and bindings
Expand All @@ -76,11 +77,12 @@ jobs:
working-directory: pkgs/ffigen/
steps:
- uses: actions/checkout@9bb56186c3b09b4f86b1c65136769dd318469633
- uses: dart-lang/setup-dart@fedb1266e91cf51be2fdb382869461a434b920a3
- uses: subosito/flutter-action@44ac965b96f18d999802d4b807e3256d5a3f9fa1
with:
sdk: dev #3.3.0
flutter-version: 3.19.0
channel: 'stable'
- name: Install dependencies
run: dart pub get
run: flutter pub get
- name: Build test dylib and bindings
run: dart test/setup.dart
- name: Run VM tests
Expand Down Expand Up @@ -109,11 +111,12 @@ jobs:
working-directory: pkgs/ffigen/
steps:
- uses: actions/checkout@9bb56186c3b09b4f86b1c65136769dd318469633
- uses: dart-lang/setup-dart@fedb1266e91cf51be2fdb382869461a434b920a3
- uses: subosito/flutter-action@44ac965b96f18d999802d4b807e3256d5a3f9fa1
with:
sdk: dev #3.3.0
flutter-version: 3.19.0
channel: 'stable'
- name: Install dependencies
run: dart pub get
run: flutter pub get
- name: Build test dylib and bindings
run: dart test/setup.dart
- name: Run VM tests
Expand Down
7 changes: 4 additions & 3 deletions .github/workflows/ffigen_weekly.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,12 @@ jobs:
working-directory: pkgs/ffigen/
steps:
- uses: actions/checkout@9bb56186c3b09b4f86b1c65136769dd318469633
- uses: dart-lang/setup-dart@fedb1266e91cf51be2fdb382869461a434b920a3
- uses: subosito/flutter-action@44ac965b96f18d999802d4b807e3256d5a3f9fa1
with:
sdk: dev #3.3.0
flutter-version: 3.19.0
channel: 'stable'
- name: Install dependencies
run: dart pub get
run: flutter pub get
- name: Build test dylib and bindings
run: dart test/setup.dart
- name: Run VM tests
Expand Down
1 change: 0 additions & 1 deletion .github/workflows/health.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ on:
# Remove paths after https://github.com/bmw-tech/dart_apitool/issues/177 is addressed.
paths:
- "pkgs/ffi/**"
- "pkgs/ffigen/**"
- "pkgs/native_assets_builder/**"
- "pkgs/native_assets_cli/**"
- "pkgs/native_toolchain_c/**"
Expand Down
1 change: 1 addition & 0 deletions .github/workflows/health_jnigen.yaml → .github/workflows/health_flutter.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ on:
branches: [ main ]
# Merge back into health.yaml after https://github.com/bmw-tech/dart_apitool/issues/177 is addressed.
paths:
- "pkgs/ffigen/**"
- "pkgs/jni/**"
- "pkgs/jnigen/**"
types: [opened, synchronize, reopened, labeled, unlabeled]
Expand Down
44 changes: 44 additions & 0 deletions .github/workflows/objective_c.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
name: objective_c
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe it is better to fold this into ffigen.yml as a matrix. That way we don't have to duplicate things.

For testing, I want to avoid having the package:objective_c tests use ffigen. So there's not much I can test at the moment, since the only things in the package are internal details of ffigen. Those are all covered by the ffigen tests anyway. There will be more to test in this package once I add some of the higher level classes, like NSString.

Either we should figure out how to do coverage from the package:ffigen run, or we should have tests in this package. (@HosseinYousefi what's the situation for packag:jni in this regard?)

Copy link
Member

Choose a reason for hiding this comment

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

JNI has more classes (like JString, JList, ...). And we also test Jni.env stuff. It makes sense to have a separate workflow for those. However, the coverage doesn't get added. I have an open issue for that: #1010.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll be adding classes like NSObject and NSString soon, so there will be more to test in this package.

The coverage issue should be pretty easy to fix. The coverage tool explicitly filters out coverage info from all packages other than the one directly being tested. All we have to do is change the flags to make sure all the packages in this repo are allowed through that filter.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, that worked. I'm gathering package:objective_c coverage during package:ffigen's tests. You can see that the pkgs/objective_c directory is present in the coverage data, where it was missing from earlier runs: https://coveralls.io/builds/66793256


on:
# Run on PRs and pushes to the default branch.
push:
branches: [main, stable]
paths:
- '.github/workflows/objective_c.yml'
- 'pkgs/objective_c/**'
pull_request:
branches: [main, stable]
paths:
- '.github/workflows/objective_c.yml'
- 'pkgs/objective_c/**'
schedule:
- cron: "0 0 * * 0"

env:
PUB_ENVIRONMENT: bot.github

jobs:
# Check code formatting and static analysis.
analyze:
runs-on: macos-latest
defaults:
run:
working-directory: pkgs/objective_c/
strategy:
fail-fast: false
steps:
- uses: actions/checkout@9bb56186c3b09b4f86b1c65136769dd318469633
- uses: subosito/flutter-action@44ac965b96f18d999802d4b807e3256d5a3f9fa1
with:
flutter-version: 3.19.0
channel: 'stable'
- id: install
name: Install dependencies
run: flutter pub get
- name: Check formatting
run: dart format --output=none --set-exit-if-changed .
if: always() && steps.install.outcome == 'success'
- name: Analyze code
run: flutter analyze --fatal-infos
if: always() && steps.install.outcome == 'success'
1 change: 1 addition & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ building and bundling.
| --- | --- | --- |
| [ffi](pkgs/ffi/) | Utilities for working with Foreign Function Interface (FFI) code. | [![pub package](https://img.shields.io/pub/v/ffi.svg)](https://pub.dev/packages/ffi) |
| [ffigen](pkgs/ffigen/) | Generator for FFI bindings, using LibClang to parse C, Objective-C, and Swift files. | [![pub package](https://img.shields.io/pub/v/ffigen.svg)](https://pub.dev/packages/ffigen) |
| [objective_c](pkgs/objective_c/) | A library to access Objective C from Flutter that acts as a support library for package:ffigen. | [![pub package](https://img.shields.io/pub/v/objective_c.svg)](https://pub.dev/packages/objective_c) |
| [jni](pkgs/jni/) | A library to access JNI from Dart and Flutter that acts as a support library for `package:jnigen`. | [![pub package](https://img.shields.io/pub/v/jni.svg)](https://pub.dev/packages/jni) |
| [jnigen](pkgs/jnigen/) | A Dart bindings generator for Java and Kotlin that uses JNI under the hood to interop with Java virtual machine. | [![pub package](https://img.shields.io/pub/v/jnigen.svg)](https://pub.dev/packages/jnigen) |
| [native_assets_builder](pkgs/native_assets_builder/) | This package is the backend that invokes build hooks. | [![pub package](https://img.shields.io/pub/v/native_assets_builder.svg)](https://pub.dev/packages/native_assets_builder) |
Expand Down
15 changes: 15 additions & 0 deletions pkgs/ffigen/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,21 @@
- Add `retainAndReturnPointer` method to ObjC objects and blocks, and add
`castFromPointer` method to blocks.
- Add `-Wno-nullability-completeness` as default compiler option for MacOS.
- __Breaking change__: Use `package:objective_c` in ObjC bindings.
- ObjC packages will have a flutter dependency (until
https://github.com/dart-lang/native/issues/1068 is fixed).
- ObjC class methods don't need the ubiquitous `lib` argument anymore. In
fact, ffigen won't even generate the native library class (unless it needs
to bind top level functions without using `@Native`). It is still necessary
to `DynamicLibrary.open` the dylib though, to load the classes and methods.
- Adapting to this change:
- Update ffigen and re-run the code generation. If the generated code no
longer contains the native library class, it means it isn't needed
anymore. So `final lib = FooNativeLib(DynamicLibrary.open('foo.dylib'));`
must be changed to `DynamicLibrary.open('foo.dylib');`.
- Regardless of whether the native library class still exists, delete the
`lib` parameter from all ObjC object constructors and static method calls
and block constructors.

## 11.0.0

Expand Down
8 changes: 6 additions & 2 deletions pkgs/ffigen/analysis_options.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,12 @@ analyzer:
todo: ignore
exclude:
- 'test/**_expected*'
# Goldens cannot be generated outside MacOS causing analysis errors.
- test/native_objc_test/**

# Exclude anything that uses package:objective_c, due to flutter dependency.
# https://github.com/dart-lang/native/issues/1068
- example/objective_c/**
- example/swift/**
- test_flutter/native_objc_test/**
language:
strict-casts: true
strict-inference: true
Expand Down
Loading