Skip to content

[ffigen] Add Header Files to CBuilder Sources #2098

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
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
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,10 @@ void main(List<String> arguments) async {
final cbuilder = CBuilder.library(
name: packageName,
assetName: 'src/${packageName}_bindings_generated.dart',
sources: ['src/$packageName.c'],
sources: [
'src/$packageName.c',
'src/$packageName.h',
],
);
await cbuilder.run(
input: input,
Expand Down
17 changes: 17 additions & 0 deletions pkgs/native_assets_cli/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,23 @@ experiment is only available on the master channel.
We do breaking changes regularly! So frequently bump `package:native_assets_cli`
and use dev/master SDK for CI.

**Note:** When configuring `CBuilder` in your `hook/build.dart`,
Copy link
Collaborator

Choose a reason for hiding this comment

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

We should not have CBuilder documentation here. The native assets cli can be used with other builders. Add the documentation to the CBuilder instead.

include both `.c` source files and `.h` header files in the `sources` list.
This ensures that changes to header files (e.g., `native_add_library.h`)
invalidate the build cache, triggering a rebuild when necessary. For example:
```dart
final cbuilder = CBuilder.library(
name: 'native_add_library',
assetName: 'src/native_add_library_bindings_generated.dart',
sources: [
'src/native_add_library.c',
'src/native_add_library.h',
'src/dart_api_dl.c',
'src/dart_api_dl.h',
],
);
```

## Development

The development of the feature can be tracked in [dart-lang#50565],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,12 @@ void main(List<String> arguments) async {
final cbuilder = CBuilder.library(
name: packageName,
assetName: 'src/${packageName}_bindings_generated.dart',
sources: ['src/$packageName.c', 'src/dart_api_dl.c'],
sources: [
'src/$packageName.c',
'src/$packageName.h',
'src/dart_api_dl.c',
'src/dart_api_dl.h',
],
);
await cbuilder.run(
input: input,
Expand Down
1 change: 1 addition & 0 deletions pkgs/native_assets_cli/lib/src/api/build.dart
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import '../validation.dart';
/// assetName: '$packageName.dart',
/// sources: [
/// 'src/$packageName.c',
/// 'src/$packageName.h',
/// ],
/// );
/// await cbuilder.run(
Expand Down
18 changes: 18 additions & 0 deletions pkgs/native_toolchain_c/lib/src/cbuilder/cbuilder.dart
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,24 @@ class CBuilder extends CTool implements Builder {
CBuilder.library({
required super.name,
super.assetName,
/// The list of source files to build the library.
///
/// This should include both C source files (e.g., `.c`) and header files
/// (e.g., `.h`). Including header files ensures that changes to them
/// invalidate the build cache, triggering recompilation when necessary.
/// For example, for a package named `native_add_library`:
/// ```dart
/// sources: [
/// 'src/native_add_library.c',
/// 'src/native_add_library.h',
/// 'src/dart_api_dl.c',
Copy link
Collaborator

Choose a reason for hiding this comment

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

Omit dart_api_dl from the documentation, it doesn't add extra info but it does complicate things. That's not great for documentation.

/// 'src/dart_api_dl.h',
/// ],
/// ```
/// Supported by Clang-like compilers, which can optimize with precompiled
Copy link
Collaborator

Choose a reason for hiding this comment

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

The CBuilder should work for all currently supported compilers. If this doesn't work for example for MSVC we need to filter the header files. Please remove this comment and test if it works for Windows.

Copy link
Author

Choose a reason for hiding this comment

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

done

/// headers when `.h` files are included. If a compiler does not support this,
/// the build system may filter `.h` files from the compilation step while
/// still tracking them as dependencies.
super.sources = const [],
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think the comment should go on the field rather than the constructor argument.

Copy link
Author

Choose a reason for hiding this comment

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

should the whole segment be moved or just this portion?

Copy link
Collaborator

Choose a reason for hiding this comment

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

We comment fields not constructor params, so all of it.

Copy link
Author

Choose a reason for hiding this comment

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

yup done removed that part

super.includes = const [],
super.frameworks = CTool.defaultFrameworks,
Expand Down
Loading