Skip to content

[native_toolchain_c] Add support for configuring preprocessor macro defines #120

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

Merged
merged 1 commit into from
Sep 6, 2023

Conversation

blaugold
Copy link
Contributor

@blaugold blaugold commented Sep 2, 2023

This PR adds CBuilder.defines which can be used to specify definitions for preprocessor macros. Providing a value for a macro is optional.

Whether a macro with the name of the current BuildMode is defined is now configurable through CBuilder.buildModeDefine. Per default, this option is enabled.

Additionally, the standard NDEBUG macro is now being defined when not building with BuildMode.debug. This behavior is also configurable (CBuilder.ndebugDefine) and enabled by default.

Closes #61

Copy link
Collaborator

@dcharkes dcharkes left a comment

Choose a reason for hiding this comment

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

Please also bump the version number in pubspec.yaml to 0.2.1-wip.0 (or 0.2.1 if you'd like a release right away) and add an entry to the CHANGELOG.md

Thanks @blaugold !


/// Whether to define a macro for the current [BuildMode].
///
/// The macro name is the uppercase name of the build mode and does not have a
Copy link
Collaborator

@dcharkes dcharkes Sep 4, 2023

Choose a reason for hiding this comment

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

Note to self: I found this in some build systems to be the default, but I didn't add documentation here. 🙄

@blaugold
Copy link
Contributor Author

blaugold commented Sep 4, 2023

Sorry about the many CI runs. 🙈 😅 The changelog/version validation step complained that the versions don't agree, even though they did, so I've removed wip.0 from the version to allow CI to run to completion if everything else works.

@dcharkes
Copy link
Collaborator

dcharkes commented Sep 4, 2023

Sorry about the many CI runs. 🙈 😅 The changelog/version validation step complained that the versions don't agree, even though they did, so I've removed wip.0 from the version to allow CI to run to completion if everything else works.

It should work, error: pubspec version (0.2.1-wip.0) and changelog (0.2.1-wip) don't agree they were different.

But I'm fine with merging this and just publishing 0.2.1.

@dcharkes
Copy link
Collaborator

dcharkes commented Sep 4, 2023

flutter pub publish --dry-run

@mosuem Did we opt all repositories in to using flutter instead of dart for auto publishing? There should be nothing Flutter on this repo.

@mosuem
Copy link
Member

mosuem commented Sep 5, 2023

Should be resolved with dart-lang/ecosystem#162 I hope?

@dcharkes
Copy link
Collaborator

dcharkes commented Sep 5, 2023

Should be resolved with dart-lang/ecosystem#162 I hope?

@mosuem has that been published yet? I've rerun the job, but it fails with the same error.

@blaugold
Copy link
Contributor Author

blaugold commented Sep 5, 2023

The reruns always use the commit before the fix in dart-lang/ecosystem:

Uses: dart-lang/ecosystem/.github/workflows/publish.yaml@refs/heads/main (2e532e38cb6a4d784f58bfaf5afbabfcbd3e3531)

I've pushed a new commit, maybe that helps.

@dcharkes dcharkes merged commit 0a4e5f8 into dart-lang:main Sep 6, 2023
@dcharkes
Copy link
Collaborator

dcharkes commented Sep 6, 2023

Thanks @blaugold ! 🚀

@blaugold blaugold deleted the c_builder-defines branch September 6, 2023 09:24
HosseinYousefi pushed a commit that referenced this pull request Nov 16, 2023
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] Add defines to API
3 participants