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

Build mode #62

Merged
merged 5 commits into from
Jun 8, 2023
Merged

Build mode #62

merged 5 commits into from
Jun 8, 2023

Conversation

dcharkes
Copy link
Collaborator

@dcharkes dcharkes commented Jun 7, 2023

Introduces BuildMode with release and debug values.

This is not an enum, but a collection with two opaque objects. (Similar to Target.) This way adding more values later will not be a breaking change.

BuildMode is required, this means that launchers will have to set it. (Currently, dart has no concept of build modes, we will have to default to release for now and add some logic to dart run and dart build later.) Since it is required, we will have to do a manual roll into the Dart SDK where we start passing BuildMode.release to the new required argument.

Package c_compiler picks up the configuration and sets either the DEBUG or RELEASE macro to 1.

We should probably do the mapping from BuildMode to defines in build.dart files, because it might be project specific. The defines should be added to the CBuilder constructors or run function. (Filed #61 to track this.)

Bug:

@coveralls
Copy link

coveralls commented Jun 7, 2023

Coverage Status

coverage: 99.338% (-0.07%) from 99.404%
when pulling c0e8d44 on build-mode
into 973f3ed on main.

Copy link
Member

@HosseinYousefi HosseinYousefi left a comment

Choose a reason for hiding this comment

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

LGTM w 1 comment. Thanks @dcharkes.

pkgs/c_compiler/lib/src/cbuilder/run_cbuilder.dart Outdated Show resolved Hide resolved
@dcharkes dcharkes merged commit 78cf286 into main Jun 8, 2023
11 checks passed
@dcharkes dcharkes deleted the build-mode branch June 8, 2023 07:23
copybara-service bot pushed a commit to dart-lang/sdk that referenced this pull request Jun 8, 2023
Manual roll of: dart-lang/native#62

Bug: #50565
Change-Id: Ie5b9ef9e9cdfbb9c19eac299f9f0294496b77520
Cq-Include-Trybots: luci.dart.try:pkg-win-release-try,pkg-mac-release-arm64-try,pkg-mac-release-try,pkg-linux-release-try,pkg-linux-debug-try
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/308040
Reviewed-by: Hossein Yousefi <yousefi@google.com>
Commit-Queue: Daco Harkes <dacoharkes@google.com>
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.

None yet

3 participants