Initial version

[dcharkes]

Initial version of the Native Assets CLI and C compiler wrapper packages.

Package features

package:native_assets_cli:

• All data structures involved in the input and output of the CLI have YAML serializability.
• The API will be exercised by the Dart SDK / Flutter Tools.
• Specification in internal design doc.

package:c_compiler:

• Tools
  • An abstraction for tool (e.g. "Clang", "GCC") and tool instances "Clang 14.0.0 at path X on the file system".
  • Logic for finding tool instances on the file system and recognizing which tool (Clang or GCC or something) something is based from a path.
  • A spec for GCC, Clang, and Clang in the NDK.
• CBuilder
  • Knows how to use the different tools to build/link binaries.
  • Uses the Native Assets CLI API in it's API.

The minimal build script with these two packages is:

import 'package:c_compiler/c_compiler.dart';
import 'package:native_assets_cli/native_assets_cli.dart';

const packageName = 'native_add';

void main(List<String> args) async {
  final buildConfig = await BuildConfig.fromArgs(args);
  final buildOutput = BuildOutput();
  final cbuilder = CBuilder.library(
    name: packageName,
    assetName:
        'package:$packageName/src/${packageName}_bindings_generated.dart',
    sources: [
      'src/$packageName.c',
    ],
  );
  await cbuilder.run(buildConfig: buildConfig, buildOutput: buildOutput);
  await buildOutput.writeToFile(outDir: buildConfig.outDir);
}

Coverage in this initial PR

• Host OSes covered: Linux (missing Windows, MacOS)
• Target OSes covered: Linux/Android (missing Windows, MacOS, iOS)
• Languages covered: C.
• Toolchains covered: only a single one for each combination. No fallback.
• Above is covered on the GitHub CI.

Dart SDK CL pulling these changes in and exercising them https://dart-review.googlesource.com/c/sdk/+/267340.

We're not intending on publishing a v0.1.0 just yet. Landing this on the main branch enables us pulling it in via DEPS in the Dart SDK. (If we don't land it, we might risk a git hash that is garbage collected, breaking reproducibility of old Dart SDK builds.)

Because we're not yet publishing a version, we don't have to pin ourselves down on an API just yet.

Follow up work

1. Expand this package to cover more OSes/Archs and include this in GitHub CI.
2. Roll this in Dart SDK https://dart-review.googlesource.com/c/sdk/+/267340
   a. The package itself and run tests on Dart bots
   b. Add implementation to dartdev and run tests for it on the Dart bots.
3. Roll this into Flutter SDK and add implementation to Flutter Tools

I'd like to land this initial version as is, because it's already quite big.

[coveralls]

coverage: 100.0%. remained the same
when pulling 3907fc8 on initial-version
into 35dc332 on main.

[devoncarew]

Hey - you generally don't (almost never? never?) want to squash and force push to a PR (and esp. after a PR has any review comments). You lose all the metadata and history that github PR reviews use. This is still a draft, but you still generally want to avoid squashing commits.

The workflow is generally to build up commits in the PR - from before the review(s) or in response to them - and then when ready to merge, merge into the default branch as a single (squashed) commit via 'squash and merge'.

[dcharkes]

I prefer squashing locally for:

• easier diff view (+ easier cleanup before pushing)
• removing noise from code that once existed but got removed or refactored
• easier rebasing/merging (that is just a pain with a stack of commits, especially if they contain code that got removed/refactored).

I prevent losing history locally by keeping some branches/tags.

I wish GitHub would keep snapshots like Gerrit does...

This is still a draft

To ensure the CI setup with all the deps works. Otherwise I'd just do this on a branch before making a PR.

I'll try to avoid force pushing once reviews come in.

Maybe there are some parts of my GitHub workflow that could be optimized. I'm open to suggestions.

[dcharkes]

Hey @liamappelbe PTAL at this PR in conjunction with https://dart-review.googlesource.com/c/sdk/+/267340.

[dcharkes]

@HosseinYousefi FYI: The build for package:jni would look something like pkgs/native_assets_cli/example/native_add/build.dart in this PR.

[dcharkes]

Thanks @HosseinYousefi @liamappelbe!

I'm going to hit merge so that we have a hash on main to pull in to the Dart SDK.

[mkustermann]

Some comments while looking briefly over it.

[mkustermann]

We use the native assets file format for dart build / flutter build / ... and the VM (all under our control). The CLI and package's build.dart don't need to know about it.

[dcharkes]

I was thinking that it could be conceivable that people end up compiling kernel themselves and want to provide a manually baked native_assets.yaml similiar to how they can provide a manual package_config.json.

We could move the implementation to pkg/native_assets_builder in https://dart-review.googlesource.com/c/sdk/+/267340 if we want it to be private.

[mkustermann]

The dart build / flutter build can only tell it's preference. Why would we need for static / dynamic here? The bundling tool can still emit an error if it tells prefer-static but gets dynamic libraries.

Could this be a simple enum LinkingPreference { static, dynamic, any }?

[dcharkes]

That means we cannot eagerly fail on the build in the package itself returning an error and non-0 exit code.
Maybe that's okay, but it would lead to some wasted work if it's a longer build.

Renamed to LinkModePreference (to keep consistent with LinkMode) in #11.

[mkustermann]

Wouldn't a deterministic hashability of the Config object be sufficient?

[dcharkes]

I'm not sure I get what you mean. Do you mean identity hashCode?

I want the hashCode to be consistent with equals (so not identity hashCode). And equality is needed for checking if anything changed between builds, and useful for tests.

[mkustermann]

It seems that this BuildOutput doesn't make a real API (such as output.addAsset(...)) instead it exposes it's constituent fields. Some of those fields like assets are mutable, others are not mutable.

It may be better to provide an API that one can use to add things to the output. The API can be hierarchical just like the config.

[dcharkes]

Thanks @mkustermann ! I'll address them in a follow up PR.

edit: it looks like the comments I drafter earlier were never sent.