Skip to content

[native_toolchain_c] Track header files as build dependencies - #3728

Open
Yusufihsangorgel wants to merge 2 commits into
dart-lang:mainfrom
Yusufihsangorgel:cbuilder-header-dependencies
Open

Yusufihsangorgel wants to merge 2 commits into
dart-lang:mainfrom
Yusufihsangorgel:cbuilder-header-dependencies

Conversation

@Yusufihsangorgel

Copy link
Copy Markdown
Contributor

Description

Header files can now be listed in the sources of a CBuilder. They are reported as hook dependencies, and editing one reruns the build. Files ending in .h are dropped before any compiler-specific code runs. The sources doc asks users to list their headers.

Following #1332 and the review of #2098, the examples and test projects list their headers in sources. The CBuilder tests check that headers are not passed to the compiler and show up as dependencies. In build_runner_caching_test.dart, editing a header reruns the build.

The native workflow passed in my fork on Linux, macOS and Windows: https://github.com/Yusufihsangorgel/native/actions/runs/37115593562. On Windows the CBuilder tests ran with MSVC, including both tests changed here.

The Health changelog check flags code_assets. Its changes, like those in ffigen, are limited to the README, library docs, examples and (for ffigen) the hook that builds its test helpers. Neither package gets a changelog entry.

Related Issues

Fixes #1332

PR Checklist

  • I’ve reviewed the contributor guide and applied the relevant portions to this PR.
  • I've run dart tool/ci.dart --all locally and resolved all issues identified. This ensures the PR is formatted, has no lint errors, and ran all code generators. This applies to the packages part of the toplevel pubspec.yaml workspace.
  • All existing and new tests are passing. I added new tests to check the change I am making.
  • The PR is actually solving the issue. PRs that don't solve the issue will be closed. Please be respectful of the maintainers' time. If it's not clear what the issue is, feel free to ask questions on the GitHub issue before submitting a PR.
  • I have updated CHANGELOG.md for the relevant packages. (Not needed for small changes such as doc typos).
  • I have updated the pubspec package version if necessary.

testPackageUri.resolve('src/debug.h'),
testPackageUri.resolve('src/add.c'),
testPackageUri.resolve('src/add.h'),
testPackageUri.resolve('src/math.h'),

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

math .h twice?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

math.c and add.c both include it, and the dependencies builder doesn't dedupe.

testPackageUri.resolve('src/debug.h'),
testPackageUri.resolve('src/add.c'),
testPackageUri.resolve('src/add.h'),
testPackageUri.resolve('src/math.h'),

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

ditto?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Same cause.

@codecov

codecov Bot commented Oct 6, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 87.93%. Comparing base (f7ffe96) to head (fb81c38).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #3728   +/-   ##
=======================================
  Coverage   87.93%   87.93%           
=======================================
  Files         310      310           
  Lines       19799    19800    +1     
=======================================
+ Hits        17411    17412    +1     
  Misses       2388     2388           
Flag Coverage Δ
ffigen 91.48% <ø> (ø)
native_pkgs_macos 86.34% <100.00%> (+<0.01%) ⬆️
native_pkgs_ubuntu 71.97% <100.00%> (+<0.01%) ⬆️
native_pkgs_windows 75.32% <100.00%> (+<0.01%) ⬆️
objective_c 63.11% <ø> (ø)
swiftgen 80.41% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Components Coverage Δ
code_assets 94.96% <ø> (ø)
data_assets 91.56% <ø> (ø)
ffi 100.00% <ø> (ø)
ffigen 91.58% <ø> (ø)
hooks 86.40% <ø> (ø)
hooks_runner 89.72% <ø> (ø)
jni 66.34% <ø> (ø)
jni_flutter ∅ <ø> (∅)
jni_util 25.00% <ø> (ø)
jnigen 82.85% <ø> (ø)
json_syntax_generator ∅ <ø> (∅)
native_test_helpers 85.71% <ø> (ø)
native_toolchain_c 95.94% <100.00%> (+<0.01%) ⬆️
objective_c 79.53% <ø> (ø)
pub_formats ∅ <ø> (∅)
record_use 81.48% <ø> (ø)
snippet_tool 66.23% <ø> (ø)
swift2objc 92.95% <ø> (ø)
swiftgen 80.41% <ø> (ø)
test_case_selector 97.28% <ø> (ø)
Files with missing lines Coverage Δ
.../native_toolchain_c/lib/src/cbuilder/cbuilder.dart 97.26% <100.00%> (+0.03%) ⬆️
...kgs/native_toolchain_c/lib/src/cbuilder/ctool.dart 100.00% <ø> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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.

[native_toolchain_c] Changes to header files should be added to the dependencies

2 participants