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

fix: Header filename mismatch causing ffigen to produce empty output #141

Merged
merged 1 commit into from
Sep 23, 2023

Conversation

BrutalCoding
Copy link
Contributor

Description

Some minor context how I got here: Been fighting with ffigen for about a week now to get a project called `llama_cpp` ported over to Dart as a convenient package. I've got ffi working in a side project of mine (shady.ai) but creating a plugin_ffi package has been troublesome.

Before this PR, upon checking out the repository, things seemed to work at first glance. No compile errors. I was able to execute the test and run commands with success in the native_add_library package.

However, I was unable to re-generate Dart code from the native header (specified in ffigen.yaml), turns out it's due to a typo in the [headers] section.

Steps I had taken prior this PR:

  1. Navigate to dart_lang_native/pkgs/native_assets_cli/example/native_add_library
  2. Execute dart --enable-experiment=native-assets test
  3. Test passed.
  4. Happy me
  5. Added new C code
  6. Ran ffigen tooling
  7. Analyzer will show some syntax errors here and there due to the missing missing references
  8. native_add_library.dart seems to be an empty file now aside from the file comments.

Before

Result of lib/native_add_library.dart:

// Copyright (c) 2023, 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.

// AUTO GENERATED FILE, DO NOT EDIT.
//
// Generated by `package:ffigen`.
// ignore_for_file: type=lint

After

Now, with this PR, the file (lib/native_add_library.dart) is looking good again.

// Copyright (c) 2023, 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.

// AUTO GENERATED FILE, DO NOT EDIT.
//
// Generated by `package:ffigen`.
// ignore_for_file: type=lint
import 'dart:ffi' as ffi;

@ffi.Native<ffi.Int32 Function(ffi.Int32, ffi.Int32)>(symbol: 'add')
external int add(
  int a,
  int b,
);

  • I’ve reviewed the contributor guide and applied the relevant portions to this PR.
Contribution guidelines:

Note that many Dart repos have a weekly cadence for reviewing PRs - please allow for some latency before initial review feedback.

@dcharkes
Copy link
Collaborator

Thanks @BrutalCoding! 🚀

I must have missed this when moving the examples around.

@auto-submit
Copy link

auto-submit bot commented Sep 23, 2023

auto label is removed for dart-lang/native/141, due to This PR has not met approval requirements for merging. You are not a member of dart-team and need 2 more review(s) in order to merge this PR.

  • Merge guidelines: You need at least one approved review if you are already part of flutter-hackers or two member reviews if you are not a flutter-hacker before re-applying the autosubmit label. Reviewers: If you left a comment approving, please use the "approve" review action instead.

@dcharkes dcharkes self-requested a review September 23, 2023 12:05
@dcharkes dcharkes merged commit be4aaf7 into dart-lang:main Sep 23, 2023
17 checks passed
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.

2 participants