ffi plugin template ffigen command doesn't work on Windows

@stuartmorgan

Seen behavior

lib\hello_bindings_generated.dart contains no bindings.

Expected behavior

lib\hello_bindings_generated.dart contains bindings for sum and sum_long_running.

Steps to reproduce

On Windows:

flutter create --template plugin_ffi hello
cd hello
flutter pub run ffigen --config ffigen.yaml

Root cause

Path separator on Windows not converted to backslash:

Bindings are not generated for files declared under "include-directives:" dart-lang/native#239

Fix

Bump package:ffigen to a new version after landing:

Fix include-directives on Windows dart-archive/ffigen#397

Testing

We should possibly also add a test exercising the run of the bindings generator on the bots, but this requires having libclang available.

On the package:ffigen bots we use GitHub workflows.

      - name: Install libclang-10-dev
        run: sudo apt-get install libclang-10-dev

https://github.com/dart-lang/ffigen/blob/master/.github/workflows/test-package.yml#L51-L52

@stuartmorgan Do we have libclang on the Flutter bots? Can we have this on the bots?

@stuartmorgan

@stuartmorgan Do we have libclang on the Flutter bots? Can we have this on the bots?

I have no idea if it's currently there; adding it if not would involve an infra ticket to create a new dependency package, then adding it to the bot configs.

Issue is reproducible on stable and master using the steps outlined in #105513 (comment)

hello_bindings_generated.dart

// ignore_for_file: always_specify_types
// ignore_for_file: camel_case_types
// ignore_for_file: non_constant_identifier_names

// AUTO GENERATED FILE, DO NOT EDIT.
//
// Generated by `package:ffigen`.

flutter doctor -v

[√] Flutter (Channel stable, 3.0.1, on Microsoft Windows [Version 10.0.19043.1706], locale en-GH)
    • Flutter version 3.0.1 at c:\src\sdks\stable\flutter
    • Upstream repository https://github.com/flutter/flutter.git
    • Framework revision fb57da5f94 (3 weeks ago), 2022-05-19 15:50:29 -0700
    • Engine revision caaafc5604
    • Dart version 2.17.1
    • DevTools version 2.12.2

[√] Android toolchain - develop for Android devices (Android SDK version 32.1.0-rc1)
    • Android SDK at C:\Users\Deana\AppData\Local\Android\sdk
    • Platform android-32, build-tools 32.1.0-rc1
    • Java binary at: C:\Program Files\Android\Android Studio\jre\bin\java
    • Java version OpenJDK Runtime Environment (build 11.0.12+7-b1504.28-7817840)
    • All Android licenses accepted.

[√] Chrome - develop for the web
    • Chrome at C:\Program Files\Google\Chrome\Application\chrome.exe

[√] Visual Studio - develop for Windows (Visual Studio Community 2022 17.1.4)
    • Visual Studio at C:\Program Files\Microsoft Visual Studio\2022\Community
    • Visual Studio Community 2022 version 17.1.32407.343
    • Windows 10 SDK version 10.0.22000.0

[√] Android Studio (version 2021.2)
    • Android Studio at C:\Program Files\Android\Android Studio
    • Flutter plugin can be installed from:
       https://plugins.jetbrains.com/plugin/9212-flutter
    • Dart plugin can be installed from:
       https://plugins.jetbrains.com/plugin/6351-dart
    • Java version OpenJDK Runtime Environment (build 11.0.12+7-b1504.28-7817840)

[√] VS Code (version 1.67.2)
    • VS Code at C:\Users\Deana\AppData\Local\Programs\Microsoft VS Code
    • Flutter extension version 3.42.0

[√] Connected device (4 available)
    • M2007J20CG (mobile) • 5dd3be00 • android-arm64  • Android 11 (API 30)
    • Windows (desktop)   • windows  • windows-x64    • Microsoft Windows [Version 10.0.19043.1706]
    • Chrome (web)        • chrome   • web-javascript • Google Chrome 102.0.5005.63
    • Edge (web)          • edge     • web-javascript • Microsoft Edge 102.0.1245.33

[√] HTTP Host Availability
    • All required HTTP hosts are available

• No issues found!

[√] Flutter (Channel master, 3.1.0-0.0.pre.1142, on Microsoft Windows [Version 10.0.19043.1706], locale en-GH)
    • Flutter version 3.1.0-0.0.pre.1142 at c:\src\sdks\master\flutter
    • Upstream repository https://github.com/flutter/flutter.git
    • Framework revision be0c1bd0f0 (5 hours ago), 2022-06-07 05:18:06 -0400
    • Engine revision 0968438071
    • Dart version 2.18.0 (build 2.18.0-170.0.dev)
    • DevTools version 2.14.0

[√] Android toolchain - develop for Android devices (Android SDK version 32.1.0-rc1)
    • Android SDK at C:\Users\Deana\AppData\Local\Android\sdk
    • Platform android-32, build-tools 32.1.0-rc1
    • Java binary at: C:\Program Files\Android\Android Studio\jre\bin\java
    • Java version OpenJDK Runtime Environment (build 11.0.12+7-b1504.28-7817840)
    • All Android licenses accepted.

[√] Chrome - develop for the web
    • Chrome at C:\Program Files\Google\Chrome\Application\chrome.exe

[√] Visual Studio - develop for Windows (Visual Studio Community 2022 17.1.4)
    • Visual Studio at C:\Program Files\Microsoft Visual Studio\2022\Community
    • Visual Studio Community 2022 version 17.1.32407.343
    • Windows 10 SDK version 10.0.22000.0

[√] Android Studio (version 2021.2)
    • Android Studio at C:\Program Files\Android\Android Studio
    • Flutter plugin can be installed from:
       https://plugins.jetbrains.com/plugin/9212-flutter
    • Dart plugin can be installed from:
       https://plugins.jetbrains.com/plugin/6351-dart
    • Java version OpenJDK Runtime Environment (build 11.0.12+7-b1504.28-7817840)

[√] VS Code (version 1.67.2)
    • VS Code at C:\Users\Deana\AppData\Local\Programs\Microsoft VS Code
    • Flutter extension version 3.42.0

[√] Connected device (4 available)
    • M2007J20CG (mobile) • 5dd3be00 • android-arm64  • Android 11 (API 30)
    • Windows (desktop)   • windows  • windows-x64    • Microsoft Windows [Version 10.0.19043.1706]
    • Chrome (web)        • chrome   • web-javascript • Google Chrome 102.0.5005.63
    • Edge (web)          • edge     • web-javascript • Microsoft Edge 102.0.1245.33

[√] HTTP Host Availability
    • All required HTTP hosts are available

• No issues found!

@zanderso

cc @zanderso

@dcharkes

@dcharkes Could you also test this using your existing GitHub workflow (where you know you have libclang) by also doing a checkout of Flutter there?

@dcharkes

@dcharkes Could you also test this using your existing GitHub workflow (where you know you have libclang) by also doing a checkout of Flutter there?

We don't currently cover Windows there, so we wouldn't have caught this issue. I've filed an issue to address that.

Run Windows CI dart-lang/native#240

Running it there would mean we run something on the CI there that is not the main branch of the package. (We could force the main branch, but then we would start to get rolling issues if the generated code changes slightly.) And if we break something here in flutter_tools, the build of package:ffigen would be broken. Both are undeseriable, so running it on the Flutter bots would be better.

Or did you mean adding such a GitHub workflow to the Flutter repository? edit: I just realized that even using a flutter-latest on a GitHub workflow would use the last published build, not the PR being submitted. So, we would get the failure on a later commit/PR, which is undeseriable.

@stuartmorgan

We don't currently cover Windows there

Fixed as of

Add Windows CI dart-archive/ffigen#399

We have libclang on the Cirrus CI bot that runs the FFI + ffigen test

@stuartmorgan what is the relationship between the Cirrus CI bots and LUCI bots? Should we support the test on both? Can we skip the test on the LUCI bots? (It might not be worth spending the effort to set up and maintain libclang as DEPS for our bots for a single test. We have extensive testing on package:ffigen already. We just want to have a sanity check here that the generator runs.)

@dcharkes

@dcharkes Could you also test this using your existing GitHub workflow (where you know you have libclang) by also doing a checkout of Flutter there?

I got GitHub actions working here on the Flutter repo, with a fork of the test not relying on the cache folder. It works for Linux, MacOS and Windows. See:

@zanderso @stuartmorgan Which test would you prefer?

Cirrus CI (Linux only)
GitHub actions (Linux, MacOS, Windows)

If Cirrus CI, where should we place the test so that we exclude it on LUCI but run it via .cirrus.yml?

If GitHub actions, where should we place the test so that it is not picked up by Cirrus CI and LUCI bots? (They run everything named /test/**_test.dart, and we have a check against naming files non-*_test.dart in the test/ folder.)

Also, it would be the first time using GitHub actions for tests. Do we even want that on the Flutter repo? And if yes, what is the naming convention that we should adopt?

We could go with option 3.

LUCI with CIPD packages. But I foresee a maintenance burden there.

@athomas and I explored adding libclang to the LUCI Dart/Flutter image instead, but Bionic is stuck at libclang 6 and ffigen needs >= 9.

@stuartmorgan

@stuartmorgan what is the relationship between the Cirrus CI bots and LUCI bots?

Assuming you are talking about flutter/flutter, I'm not very familiar with the division there. IIRC the main testing is LUCI and Cirrus is to smokescreen non-project-member PRs, but @godofredoc could probably provide better details.

Also, it would be the first time using GitHub actions for tests. Do we even want that on the Flutter repo?

This is a question for @godofredoc, but my guess is that the answer is no (because the more unrelated CI systems we have the harder it is to maintain).

I'd encourage to use LUCI for all the new testing. CIPD is the preferred way to provide dependencies as they can be changed independently of the VM image.

GitHub actions even if they apparently work they will not connect with metrics, bots, validations, etc. and adding that support requires a huge effort from infra and becomes a maintenance burden.

@godofredoc

Whatever testing capabilities we need, we should use LUCI for, as @godofredoc says. I have not studied this particular issue in depth, but I'm wondering if there's anything missing from our unit testing coverage that would have caught this. Is there a regression unite test that could be written for this issue that does not require libclang?

We are missing a regression test that runs FFIgen. (Which would have caught the above described issue on Windows.)

I am not aware of a regression that that could be written without libclang. To check that FFIgen generates an identical file requires parsing the header file, which requires libclang.

(We do have a tests in FFIgen itself now that check that the Windows output is identical to the Linux output on other C headers. But that does not test the header in the plugin_ffi Flutter template.)

As mentioned in the libclang tracking issue. I don't feel very happy about submitting 3 archives with the libclang + its dependencies for the major OSes and not using a package manager to install it so that all dependencies are properly installed. It seems fragile. I can do it if its the only way forward here.

On Flutter Engine CI, we pull clang from CIPD for Linux, macOS, and Windows: https://github.com/flutter/engine/blob/main/DEPS#L611. If those CIPD packages contain the library you need, then I don't think it would be too difficult to ensure those are available on a framework LUCI builder.

Unfortunately, these do not contain libclang (find . -name libclang*.dylib, find . -name libclang*.so). I see these are uploaded by the fuchsia-prod-builder. I'll ping Fuchsia engprod. If they have automated it for clang, maybe adding libclang would be easy. (Definitely better than adding the version of libclang released today by hand and never updating it.)

Edit: https://bugs.fuchsia.dev/p/fuchsia/issues/detail?id=78851 for a request to add them.

Plugin FFI template bump ffigen to 6.0.1

Given that adding/maintaining a libclang CIPD package is undesirable, I'll add a test to the FFIgen repo.

This means the test will be only run post-merge, when the CI on FFIgen runs. And it also means that if we ever want to change the generated code in FFIgen, the test will temporarily break until that FFIgen version is pinned in the FFI plugin template. (But it's better than having no testing or having to maintain a libclang CIPD package ourselves.)

Flutter PR:

Plugin FFI template bump ffigen to 6.0.1 #108792

FFIgen Flutter FFI plugin integration test (currently failing because the Windows fix is not in Flutter yet) :

Run Flutter FFI plugin template ffigen on CI dart-archive/ffigen#433

Merged #108792

This thread has been automatically locked since there has not been any recent activity after it was closed. If you are still experiencing a similar issue, please open a new bug, including the output of flutter doctor -v and a minimal reproduction of the issue.

Navigation Menu

Repository navigation

ffi plugin template ffigen command doesn't work on Windows #105513

Seen behavior

Expected behavior

Steps to reproduce

Root cause

Fix

Testing

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

ffi plugin template ffigen command doesn't work on Windows #105513

Description

Seen behavior

Expected behavior

Steps to reproduce

Root cause

Fix

Testing

Activity

stuartmorgan-g commented on Jun 7, 2022

danagbemava-nc commented on Jun 7, 2022

christopherfujino commented on Jun 7, 2022

zanderso commented on Jun 8, 2022

dcharkes commented on Jun 8, 2022

dcharkes commented on Jun 14, 2022

dcharkes commented on Jun 15, 2022

stuartmorgan-g commented on Jun 15, 2022

godofredoc commented on Jun 15, 2022

zanderso commented on Jun 20, 2022

dcharkes commented on Jun 28, 2022

zanderso commented on Jun 28, 2022

dcharkes commented on Jun 29, 2022

dcharkes commented on Aug 2, 2022

dcharkes commented on Aug 5, 2022

github-actions commented on Aug 19, 2022

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions