[native_toolchain_c] Add linking for Android #2347

goderbauer · 2025-06-06T10:02:16Z

Towards #1376

This enables linking for Android on Windows, Linux, and MacOS hosts.

With this PR the linker is no longer invoked directly. Instead, it is invoked via the compiler driver (clang, etc.), which has the advantage that we don't have to hand-translate the arguments to the format the linker expects them to be. The compiler driver is doing that for us and we can mostly reuse the compiler driver invocation.

I’ve reviewed the contributor guide and applied the relevant portions to this PR.

Contribution guidelines:

See our contributor guide for general expectations for PRs.
Larger or significant changes should be discussed in an issue before creating a PR.
Contributions to our repos should follow the Dart style guide and use dart format.
Most changes should add an entry to the changelog and may need to rev the pubspec package version.
Changes to packages require corresponding tests.

Note that many Dart repos have a weekly cadence for reviewing PRs - please allow for some latency before initial review feedback.

github-actions · 2025-06-06T10:04:22Z

PR Health

Changelog Entry ✔️

Package	Changed Files

Changes to files need to be accounted for in their respective changelogs.

API leaks ✔️

The following packages contain symbols visible in the public API, but not exported by the library. Export these symbols or remove them from your publicly visible API.

Package	Leaked API symbols

License Headers ✔️

// Copyright (c) 2025, the Dart project authors. Please see the AUTHORS file
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.

Files
no missing headers

All source files should start with a license header.

Unrelated files missing license headers

Files
pkgs/jni/lib/src/third_party/generated_bindings.dart
pkgs/native_doc_dartifier/example/native_doc_dartifier_example.dart
pkgs/native_doc_dartifier/lib/native_doc_dartifier.dart
pkgs/native_doc_dartifier/lib/src/native_doc_dartifier_base.dart
pkgs/objective_c/lib/src/ns_input_stream.dart

github-actions · 2025-06-06T12:41:36Z

Package publishing

Package	Version	Status	Publish tag (post-merge)
package:code_assets	0.19.3	already published at pub.dev
package:data_assets	0.19.1	already published at pub.dev
package:ffi	2.1.4	already published at pub.dev
package:ffigen	19.1.0-wip	WIP (no publish necessary)
package:hooks	0.19.3	already published at pub.dev
package:hooks_runner	0.21.0	already published at pub.dev
package:jni	0.14.2	already published at pub.dev
package:jnigen	0.15.0-wip	WIP (no publish necessary)
package:native_doc_dartifier	0.0.1-pre	already published at pub.dev
package:native_toolchain_c	0.16.3	ready to publish	`native_toolchain_c-v0.16.3`
package:objective_c	8.1.0-wip	WIP (no publish necessary)
package:swift2objc	0.0.1-wip	WIP (no publish necessary)
package:swiftgen	0.0.1-wip	WIP (no publish necessary)

Documentation at https://github.com/dart-lang/ecosystem/wiki/Publishing-automation.

goderbauer · 2025-06-06T13:37:05Z

Hm, it's not clear to me why the bot wants the version to be bumped to 0.17.0

dcharkes · 2025-06-06T14:28:41Z

Hm, it's not clear to me why the bot wants the version to be bumped to 0.17.0

You should be able to repro it with the dart_apitool yourself with https://github.com/dart-lang/ecosystem/blob/64aac3a9c4606950bcf6c8729f01ed8548b9ed87/pkgs/firehose/lib/src/health/health.dart#L152

dcharkes · 2025-06-06T14:30:04Z

pkgs/native_toolchain_c/test/clinker/objects_cross_test.dart

+
+//TODO(mosuem): Enable for windows and mac.
+// See https://github.com/dart-lang/native/issues/1376.
+@TestOn('linux')


Should this be removed now?

I believe this is still correct. This test checks whether we can link on the current host OS for different architectures running the same (host) OS. For that, we still only support Linux for now. The new functionality (cross linking for Android from Linux/Windows/MaxOS) I added the new *_cross_android_test.dart tests, which do not have a @TestOn limitation.

(Technically, this test doesn't belong to this change as it is unrelated to the Android work and testing existing functionality. I just saw this as a whole in the existing testing story and added it for completeness - and to make sure that my Android work doesn't break this scenario).

goderbauer · 2025-06-10T13:30:24Z

You should be able to repro it with the dart_apitool yourself

Thank you! I am pretty sure that the tool is wrong here based on what it has identified as a breaking change in a local run:

BREAKING CHANGES
├── Function "environmentFromBatchFile" removed (CE10)
├─┬ Class CBuilder
│ ├─┬ Constructor library
│ │ └── Type of parameter "linkModePreference" changed. InvalidType -> LinkModePreference? (CE08)
│ └─┬ Field linkModePreference
│   └── Type of field changed. InvalidType -> LinkModePreference? (CF04)
└─┬ Class CLinker
  ├─┬ Constructor library
  │ └── Type of parameter "linkModePreference" changed. InvalidType -> LinkModePreference? (CE08)
  └─┬ Field linkModePreference
    └── Type of field changed. InvalidType -> LinkModePreference? (CF04)

The environmentFromBtachFile wasn't touched in this PR, it is still there and properly exported. The tool also seems to be unable to correctly infer the type of the linkModePreference parameter in the old version of this package and now that it can determine the correct type it thinks the type has changed. That's not correct, the type remains unchanged. I am going to overwrite the check for now.

Edit: It looks like the dart_apitool may have trouble with native_toolchain_c being part of a pub workspace. It seems to be checking out the previous version of the package from pub and analyzing that code outside of its workspace, which seems to result in some problems. If I actually check out the whole repository at the old version's revision and have the tool compare that with the current revision, it correctly detects no breaking change: No breaking changes!

coveralls · 2025-06-10T14:23:29Z

coverage: 81.518%. first build
when pulling 330b8db on goderbauer:android-linker
into fd9415b on dart-lang:main.

dcharkes · 2025-06-10T19:37:19Z

Edit: It looks like the dart_apitool may have trouble with native_toolchain_c being part of a pub workspace. It seems to be checking out the previous version of the package from pub and analyzing that code outside of its workspace, which seems to result in some problems. If I actually check out the whole repository at the old version's revision and have the tool compare that with the current revision, it correctly detects no breaking change: No breaking changes!

It does indeed check out the package from pub. See the discussion here: bmw-tech/dart_apitool#216

The packages on pub.dev should work, they are used in the flutter template and flutter integration tests. It's fishy that it fails. (Yeah indeed just apply the label for now.)

dcharkes · 2025-06-10T19:39:23Z

pkgs/native_toolchain_c/lib/src/cbuilder/clinker.dart

@@ -53,10 +53,12 @@ class CLinker extends CTool implements Linker {
    required LinkOutputBuilder output,
    required Logger? logger,
  }) async {
-    if (OS.current != OS.linux || input.config.code.targetOS != OS.linux) {
+    const supportedTargetOSs = [OS.linux, OS.android];
+    if (!supportedTargetOSs.contains(input.config.code.targetOS)) {


This no longer checks if the hostOS is Linux if the targetOS is Linux.

dcharkes · 2025-06-10T19:40:00Z

pkgs/native_toolchain_c/test/clinker/objects_helper.dart

@@ -0,0 +1,78 @@
+// Copyright (c) 2024, the Dart project authors. Please see the AUTHORS file


new file 2025

dcharkes · 2025-06-10T19:40:20Z

pkgs/native_toolchain_c/test/clinker/treeshake_cross_android_test.dart

@@ -0,0 +1,30 @@
+// Copyright (c) 2024, the Dart project authors. Please see the AUTHORS file


mosuem

Does this mean that users need to have clang or similar installed? Linux will have ld preinstalled, but not clang.

mosuem · 2025-06-11T07:46:32Z

pkgs/native_toolchain_c/lib/src/cbuilder/linker_options.dart

@@ -51,7 +51,7 @@ class LinkerOptions {
  }) : _linkerFlags = <String>[
         ...flags ?? [],
         '--strip-debug',
-         if (symbols != null) ...symbols.expand((e) => ['-u', e]),
+         if (symbols != null) ...symbols.expand((e) => ['-u,$e']),


Suggested change

if (symbols != null) ...symbols.expand((e) => ['-u,$e']),

if (symbols != null) ...symbols.map((e) => '-u,$e'),

(in case you want to go with interpolation instead of expand)

mosuem · 2025-06-11T07:48:22Z

pkgs/native_toolchain_c/test/helpers.dart

+  Architecture.riscv64: 'elf64-littleriscv',
+};
+
+Future<void> expectMachineArchitecture(Uri libUri, Architecture target) async {


Could use some documentation

mosuem · 2025-06-11T07:48:47Z

pkgs/native_toolchain_c/test/helpers.dart

+/// From https://docs.flutter.dev/reference/supported-platforms.
+const flutterAndroidNdkVersionHighestSupported = 34;
+
+const objdumpFileFormat = {


Also could use some documentation- where are the strings taken from?

mosuem · 2025-06-11T07:54:18Z

pkgs/native_toolchain_c/lib/src/cbuilder/run_cbuilder.dart

-    final toolInstance_ = linkerOptions != null
-        ? await linker()
-        : await compiler();
+    final toolInstance_ = await compiler();


Can we now delete linker?

github-actions bot added the package:native_toolchain_c label Jun 6, 2025

goderbauer force-pushed the android-linker branch 3 times, most recently from 84b1b48 to ca6c274 Compare June 6, 2025 13:22

goderbauer mentioned this pull request Jun 6, 2025

[native_toolchain_c] Implement CLinker for different platforms #1376

Open

6 tasks

goderbauer requested review from dcharkes and mosuem June 6, 2025 13:36

dcharkes reviewed Jun 6, 2025

View reviewed changes

goderbauer added the skip-breaking-check label Jun 10, 2025

goderbauer requested a review from dcharkes June 10, 2025 13:30

goderbauer added 5 commits June 10, 2025 15:46

[native_toolchain_c] Add linking for Android

f4846fa

fix -u syntax

c0987f5

speculative windows fix

7a18291

test on more android API levels

c4c8cc4

fix naming

330b8db

goderbauer force-pushed the android-linker branch from ca6c274 to 330b8db Compare June 10, 2025 13:46

goderbauer added skip-breaking-check and removed skip-breaking-check labels Jun 10, 2025

dcharkes approved these changes Jun 10, 2025

View reviewed changes

mosuem reviewed Jun 11, 2025

View reviewed changes

		@@ -0,0 +1,78 @@
		// Copyright (c) 2024, the Dart project authors. Please see the AUTHORS file

		@@ -0,0 +1,30 @@
		// Copyright (c) 2024, the Dart project authors. Please see the AUTHORS file

	if (symbols != null) ...symbols.expand((e) => ['-u,$e']),
	if (symbols != null) ...symbols.map((e) => '-u,$e'),

[native_toolchain_c] Add linking for Android #2347

Are you sure you want to change the base?

[native_toolchain_c] Add linking for Android #2347

Conversation

goderbauer commented Jun 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Jun 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Health

Uh oh!

github-actions bot commented Jun 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Package publishing

Uh oh!

goderbauer commented Jun 6, 2025

Uh oh!

dcharkes commented Jun 6, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

goderbauer commented Jun 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coveralls commented Jun 10, 2025

Uh oh!

dcharkes commented Jun 10, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mosuem left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

goderbauer commented Jun 6, 2025 •

edited

Loading

github-actions bot commented Jun 6, 2025 •

edited

Loading

github-actions bot commented Jun 6, 2025 •

edited

Loading

goderbauer commented Jun 10, 2025 •

edited

Loading