dart-lang / native

Dart packages related to FFI and native assets bundling.
BSD 3-Clause "New" or "Revised" License
115 stars 40 forks source link

[native_assets_cli] build_mode #50

Closed dcharkes closed 1 year ago

dcharkes commented 1 year ago

We should support a build mode (debug, release) that can be passed in by the launcher (Flutter/DartSDK).

CMake equivalent: build_type Rust equivalent: profile

dcharkes commented 1 year ago

@stuartmorgan How common is doing something different in debug vs release in Flutter plugins? Should we consider this a requirement for MVP for Flutter?

(In the Dart ecosystem we haven't seen any use of debug/release yet. The Dart SDK itself is almost always release for our end users, and packages with FFI use only publish with release mode and only the package authors locally might build a debug mode. So in that case it can just be a const in the top level build.dart that the package author can flip if they need to.)

stuartmorgan commented 1 year ago

I don't have any metrics on this, but having assertions and/or logging that's only enabled in debug mode is not uncommon in general in native code. We have it in some our own plugins, for instance.

So in that case it can just be a const in the top level build.dart that the package author can flip if they need to.

Plugin clients not getting debug logging from plugins they use would not be great. That kind of logging can be useful for figuring out that you are using the plugin wrong.

dcharkes commented 1 year ago

Typical values include Debug, Release, RelWithDebInfo and MinSizeRel, but custom build types can also be defined.

https://cmake.org/cmake/help/latest/variable/CMAKE_BUILD_TYPE.html

Cargo has 4 built-in profiles: dev, release, test, and bench. In addition to the built-in profiles, custom user-defined profiles can also be specified.

https://doc.rust-lang.org/cargo/reference/profiles.html

A brand new Xcode project defines two build configurations, Debug and Release.

https://cocoacasts.com/tips-and-tricks-managing-build-configurations-in-xocde

Gradle has debug and release.

https://developer.android.com/build/building-cmdline#DebugMode

Visual Studio projects have separate release and debug configurations for your program.

https://learn.microsoft.com/en-us/visualstudio/debugger/how-to-set-debug-and-release-configurations?view=vs-2022

I believe the Dart SDK is only available in release mode (unless built manually), and consequently so are dart, dartaotruntime and the products of dart compile. (Manually built, we have debug/release/profile.)

Use debug mode during development, when you want to use hot reload. Use profile mode when you want to analyze performance. Use release mode when you are ready to release your app.

https://docs.flutter.dev/testing/build-modes

We could make the config for build_mode either an open-ended string, or a prefined set of strings (or enum). An open-ended string only is useful if launchers/embedders provide end-users a way to set them. If the launchers do not provide such option, the only values the CLI invocations will ever see are the ones set by the launchers.

We can always opt to open up to arbitrary build_mode names later, so let's start with a closed set.

Proposed set:

Flutter profile will use release for native asset builds. Unless, we would like to have a profile mode for native assets as well that keeps the native symbols but does not have asserts for example. We could add it later, but that would be a breaking change (because the predefined set of possible values gets an extra value). @stuartmorgan would a profile build-mode be useful?

stuartmorgan commented 1 year ago

I don't know how often (if ever) someone would have different behavior for release vs profile. If adding it later would be breaking, then it's probably better to err or the side of including it, and just having most people treat it like release.

dcharkes commented 1 year ago

I don't know how often (if ever) someone would have different behavior for release vs profile. If adding it later would be breaking, then it's probably better to err or the side of including it, and just having most people treat it like release.

It won't be breaking if we just make it opaque objects (like we have done with Target which we are likely to extend in the future). No enum means no completeness in switches.