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.

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
Show file tree
Hide file tree
Changes from 18 commits
Commits
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
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
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
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
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
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
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
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
4 changes: 4 additions & 0 deletions pkgs/ffigen/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,10 @@
`ffi-native` option.
- Add `retainAndReturnPointer` method to ObjC objects and blocks, and add
`castFromPointer` method to blocks.
- __Breaking change__: Use `package:objective_c` in ObjC bindings. This means
that ObjC packages will have a flutter dependency (until #1068 is fixed), and
also makes some minor API changes (`castFrom` and `isInstance` now take a
`lib` argument).

## 11.0.0

Expand Down
8 changes: 6 additions & 2 deletions pkgs/ffigen/analysis_options.yaml
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