[native_toolchain_c] Add libraries and libraryDirectories options…

… to `CTool` (#1423)
dart-lang · Dec 10, 2024 · e00afd9 · e00afd9
1 parent 8ea1a9d
commit e00afd9
Show file tree

Hide file tree

Showing 17 changed files with 272 additions and 75 deletions.
diff --git a/.github/workflows/native.yaml b/.github/workflows/native.yaml
@@ -142,6 +142,9 @@ jobs:
 
       - run: dart --enable-experiment=native-assets test
         working-directory: pkgs/${{ matrix.package }}/example/build/native_dynamic_linking/
+        # TODO(https://github.com/dart-lang/native/issues/190): Enable on windows once
+        # https://github.com/dart-lang/sdk/commit/903eea6bfb8ee405587f0866a1d1e92eea45d29e
+        # has landed in dev channel.
         if: ${{ matrix.package == 'native_assets_cli' && matrix.sdk == 'dev' && !matrix.breaking-change && matrix.os != 'windows' }}
 
       - run: dart --enable-experiment=native-assets test

diff --git a/pkgs/native_assets_builder/test_data/native_dynamic_linking/hook/build.dart b/pkgs/native_assets_builder/test_data/native_dynamic_linking/hook/build.dart
@@ -26,23 +26,19 @@ void main(List<String> args) async {
         sources: [
           'src/math.c',
         ],
-        // TODO(https://github.com/dart-lang/native/issues/190): Use specific
-        // API for linking once available.
-        flags: config.dynamicLinkingFlags('debug'),
+        libraries: ['debug'],
       ),
       CBuilder.library(
         name: 'add',
         assetName: 'add.dart',
         sources: [
           'src/add.c',
         ],
-        // TODO(https://github.com/dart-lang/native/issues/190): Use specific
-        // API for linking once available.
-        flags: config.dynamicLinkingFlags('math'),
+        libraries: ['math'],
       )
     ];
 
-    // Note: This builders need to be run sequentially because they depend on
+    // Note: These builders need to be run sequentially because they depend on
     // each others output.
     for (final builder in builders) {
       await builder.run(
@@ -53,21 +49,3 @@ void main(List<String> args) async {
     }
   });
 }
-
-extension on BuildConfig {
-  List<String> dynamicLinkingFlags(String libraryName) => switch (targetOS) {
-        OS.macOS => [
-            '-L${outputDirectory.toFilePath()}',
-            '-l$libraryName',
-          ],
-        OS.linux => [
-            r'-Wl,-rpath=$ORIGIN',
-            '-L${outputDirectory.toFilePath()}',
-            '-l$libraryName',
-          ],
-        OS.windows => [
-            outputDirectory.resolve('$libraryName.lib').toFilePath(),
-          ],
-        _ => throw UnimplementedError('Unsupported OS: $targetOS'),
-      };
-}
diff --git a/pkgs/native_assets_builder/test_data/native_dynamic_linking/src/debug.c b/pkgs/native_assets_builder/test_data/native_dynamic_linking/src/debug.c
@@ -16,5 +16,7 @@ int debug_printf(const char* format, ...) {
   int ret = vprintf(format, args);
   va_end(args);
   return ret;
+#else
+  return 0;
 #endif
 }
diff --git a/pkgs/native_assets_cli/example/build/native_dynamic_linking/hook/build.dart b/pkgs/native_assets_cli/example/build/native_dynamic_linking/hook/build.dart
@@ -26,23 +26,19 @@ void main(List<String> args) async {
         sources: [
           'src/math.c',
         ],
-        // TODO(https://github.com/dart-lang/native/issues/190): Use specific
-        // API for linking once available.
-        flags: config.dynamicLinkingFlags('debug'),
+        libraries: ['debug'],
       ),
       CBuilder.library(
         name: 'add',
         assetName: 'add.dart',
         sources: [
           'src/add.c',
         ],
-        // TODO(https://github.com/dart-lang/native/issues/190): Use specific
-        // API for linking once available.
-        flags: config.dynamicLinkingFlags('math'),
+        libraries: ['math'],
       )
     ];
 
-    // Note: This builders need to be run sequentially because they depend on
+    // Note: These builders need to be run sequentially because they depend on
     // each others output.
     for (final builder in builders) {
       await builder.run(
@@ -53,20 +49,3 @@ void main(List<String> args) async {
     }
   });
 }
-
-extension on BuildConfig {
-  List<String> dynamicLinkingFlags(String libraryName) => switch (targetOS) {
-        OS.macOS => [
-            '-L${outputDirectory.toFilePath()}',
-            '-l$libraryName',
-          ],
-        OS.linux => [
-            '-Wl,-rpath=\$ORIGIN/.',
-            '-L${outputDirectory.toFilePath()}',
-            '-l$libraryName',
-          ],
-        // TODO(https://github.com/dart-lang/native/issues/1415): Enable support
-        // for Windows once linker flags are supported by CBuilder.
-        _ => throw UnimplementedError('Unsupported OS: $targetOS'),
-      };
-}
diff --git a/pkgs/native_assets_cli/example/build/native_dynamic_linking/src/debug.c b/pkgs/native_assets_cli/example/build/native_dynamic_linking/src/debug.c
@@ -16,5 +16,7 @@ int debug_printf(const char* format, ...) {
   int ret = vprintf(format, args);
   va_end(args);
   return ret;
+#else
+  return 0;
 #endif
 }
diff --git a/pkgs/native_assets_cli/test/example/native_dynamic_linking_test.dart b/pkgs/native_assets_cli/test/example/native_dynamic_linking_test.dart
@@ -6,8 +6,9 @@
   'mac-os': Timeout.factor(2),
   'windows': Timeout.factor(10),
 })
-// TODO(https://github.com/dart-lang/native/issues/1415): Enable support
-// for Windows once linker flags are supported by CBuilder.
+// TODO(https://github.com/dart-lang/native/issues/190): Enable on windows once
+// https://github.com/dart-lang/sdk/commit/903eea6bfb8ee405587f0866a1d1e92eea45d29e
+// has landed in dev channel.
 @TestOn('!windows')
 library;
 
@@ -20,12 +21,6 @@ import 'package:test/test.dart';
 import '../helpers.dart';
 
 void main() async {
-  if (Platform.isWindows) {
-    // TODO(https://github.com/dart-lang/native/issues/1415): Enable support
-    // for Windows once linker flags are supported by CBuilder.
-    return;
-  }
-
   late Uri tempUri;
   const name = 'native_dynamic_linking';
 
@@ -51,19 +46,22 @@ void main() async {
 
       final configBuilder = BuildConfigBuilder()
         ..setupHookConfig(
-            packageRoot: testPackageUri,
-            packageName: name,
-            targetOS: OS.current,
-            buildAssetTypes: [CodeAsset.type],
-            buildMode: dryRun ? null : BuildMode.debug)
+          packageRoot: testPackageUri,
+          packageName: name,
+          targetOS: OS.current,
+          buildAssetTypes: [CodeAsset.type],
+          buildMode: dryRun ? null : BuildMode.debug,
+        )
         ..setupBuildRunConfig(
-            outputDirectory: outputDirectory,
-            outputDirectoryShared: outputDirectoryShared)
+          outputDirectory: outputDirectory,
+          outputDirectoryShared: outputDirectoryShared,
+        )
         ..setupBuildConfig(linkingEnabled: false, dryRun: dryRun)
         ..setupCodeConfig(
-            targetArchitecture: dryRun ? null : Architecture.current,
-            linkModePreference: LinkModePreference.dynamic,
-            cCompilerConfig: dryRun ? null : cCompiler);
+          targetArchitecture: dryRun ? null : Architecture.current,
+          linkModePreference: LinkModePreference.dynamic,
+          cCompilerConfig: dryRun ? null : cCompiler,
+        );
 
       final buildConfigUri = testTempUri.resolve('build_config.json');
       await File.fromUri(buildConfigUri)

diff --git a/pkgs/native_toolchain_c/CHANGELOG.md b/pkgs/native_toolchain_c/CHANGELOG.md
@@ -4,6 +4,7 @@
   https://github.com/dart-lang/native/issues/1611
 - Make optimization level configurable. Defaults to `-3s` and `/O3`.
   https://github.com/dart-lang/native/issues/1267
+- Add `libraries` and `libraryDirectories` to `CTool`.
 
 ## 0.6.0
 

diff --git a/pkgs/native_toolchain_c/lib/src/cbuilder/cbuilder.dart b/pkgs/native_toolchain_c/lib/src/cbuilder/cbuilder.dart
@@ -53,6 +53,8 @@ class CBuilder extends CTool implements Builder {
     super.sources = const [],
     super.includes = const [],
     super.frameworks = CTool.defaultFrameworks,
+    super.libraries = const [],
+    super.libraryDirectories = CTool.defaultLibraryDirectories,
     @Deprecated(
       'Newer Dart and Flutter SDKs automatically add the Dart hook '
       'sources as dependencies.',
@@ -76,6 +78,8 @@ class CBuilder extends CTool implements Builder {
     super.sources = const [],
     super.includes = const [],
     super.frameworks = CTool.defaultFrameworks,
+    super.libraries = const [],
+    super.libraryDirectories = CTool.defaultLibraryDirectories,
     @Deprecated(
       'Newer Dart and Flutter SDKs automatically add the Dart hook '
       'sources as dependencies.',
@@ -132,6 +136,10 @@ class CBuilder extends CTool implements Builder {
       // ignore: deprecated_member_use_from_same_package
       for (final source in this.dartBuildFiles) packageRoot.resolve(source),
     ];
+    final libraryDirectories = [
+      for (final directory in this.libraryDirectories)
+        outDir.resolveUri(Uri.file(directory)),
+    ];
     // ignore: deprecated_member_use
     if (!config.dryRun) {
       final task = RunCBuilder(
@@ -141,6 +149,8 @@ class CBuilder extends CTool implements Builder {
         sources: sources,
         includes: includes,
         frameworks: frameworks,
+        libraries: libraries,
+        libraryDirectories: libraryDirectories,
         dynamicLibrary:
             type == OutputType.library && linkMode == DynamicLoadingBundled()
                 ? libUri

diff --git a/pkgs/native_toolchain_c/lib/src/cbuilder/clinker.dart b/pkgs/native_toolchain_c/lib/src/cbuilder/clinker.dart
@@ -29,6 +29,8 @@ class CLinker extends CTool implements Linker {
     super.sources = const [],
     super.includes = const [],
     super.frameworks = CTool.defaultFrameworks,
+    super.libraries = const [],
+    super.libraryDirectories = CTool.defaultLibraryDirectories,
     @visibleForTesting super.installName,
     super.flags = const [],
     super.defines = const {},
@@ -68,6 +70,10 @@ class CLinker extends CTool implements Linker {
       for (final directory in this.includes)
         packageRoot.resolveUri(Uri.file(directory)),
     ];
+    final libraryDirectories = [
+      for (final directory in this.libraryDirectories)
+        outDir.resolveUri(Uri.file(directory)),
+    ];
     final task = RunCBuilder(
       config: config,
       codeConfig: config.codeConfig,
@@ -76,6 +82,8 @@ class CLinker extends CTool implements Linker {
       sources: sources,
       includes: includes,
       frameworks: frameworks,
+      libraries: libraries,
+      libraryDirectories: libraryDirectories,
       dynamicLibrary: linkMode == DynamicLoadingBundled() ? libUri : null,
       staticLibrary: linkMode == StaticLinking() ? libUri : null,
       // ignore: invalid_use_of_visible_for_testing_member

diff --git a/pkgs/native_toolchain_c/lib/src/cbuilder/ctool.dart b/pkgs/native_toolchain_c/lib/src/cbuilder/ctool.dart
@@ -6,15 +6,17 @@ import 'package:meta/meta.dart';
 import 'package:native_assets_cli/code_assets.dart';
 
 import 'cbuilder.dart';
+import 'clinker.dart';
 import 'language.dart';
 import 'optimization_level.dart';
 import 'output_type.dart';
 
+/// Common options for [CBuilder] and [CLinker].
 abstract class CTool {
   /// What kind of artifact to build.
   final OutputType type;
 
-  /// Name of the library or executable to linkg.
+  /// Name of the library or executable to build or link.
   ///
   /// The filename will be decided by [LinkConfig.targetOS] and
   /// [OSLibraryNaming.libraryFileName] or
@@ -50,16 +52,39 @@ abstract class CTool {
   ///
   /// Defaults to `['Foundation']`.
   ///
-  /// Framworks will not be automatically reported as dependencies of the hook.
+  /// Frameworks will not be automatically reported as dependencies of the hook.
   /// Frameworks can be mentioned by name if they are available on the system,
   /// so the file path is not known. If you're depending on your own frameworks
   /// report them as dependencies of the hook by calling
   /// [BuildOutputBuilder.addDependency] / [LinkOutputBuilder.addDependency]
   /// manually.
   final List<String> frameworks;
 
+  /// The default [frameworks].
   static const List<String> defaultFrameworks = ['Foundation'];
 
+  /// Libraries to link to.
+  ///
+  /// In addition to the system default directories, libraries will be searched
+  /// for in [libraryDirectories].
+  ///
+  /// If you want to link to a library that was built by another [CBuilder] or
+  /// [CLinker], either leave the default [libraryDirectories] or include `'.'`
+  /// in the list.
+  final List<String> libraries;
+
+  /// Directories to search for [libraries], in addition to the system default
+  /// directories.
+  ///
+  /// Resolved against [LinkConfig.outputDirectory].
+  ///
+  /// Defaults to `['.']`, which means the [LinkConfig.outputDirectory] will be
+  /// searched for libraries.
+  final List<String> libraryDirectories;
+
+  /// The default [libraryDirectories].
+  static const List<String> defaultLibraryDirectories = ['.'];
+
   /// TODO(https://github.com/dart-lang/native/issues/54): Move to [LinkConfig]
   /// or hide in public API.
   @visibleForTesting
@@ -130,6 +155,8 @@ abstract class CTool {
     required this.sources,
     required this.includes,
     required this.frameworks,
+    required this.libraries,
+    required this.libraryDirectories,
     required this.installName,
     required this.flags,
     required this.defines,

diff --git a/pkgs/native_toolchain_c/lib/src/cbuilder/run_cbuilder.dart b/pkgs/native_toolchain_c/lib/src/cbuilder/run_cbuilder.dart
@@ -28,6 +28,8 @@ class RunCBuilder {
   final List<Uri> sources;
   final List<Uri> includes;
   final List<String> frameworks;
+  final List<String> libraries;
+  final List<Uri> libraryDirectories;
   final Uri? executable;
   final Uri? dynamicLibrary;
   final Uri? staticLibrary;
@@ -56,6 +58,8 @@ class RunCBuilder {
     this.sources = const [],
     this.includes = const [],
     required this.frameworks,
+    this.libraries = const [],
+    this.libraryDirectories = const [],
     this.executable,
     this.dynamicLibrary,
     this.staticLibrary,
@@ -298,8 +302,7 @@ class RunCBuilder {
         if (executable != null) ...[
           '-o',
           outDir.resolveUri(executable!).toFilePath(),
-        ],
-        if (dynamicLibrary != null) ...[
+        ] else if (dynamicLibrary != null) ...[
           '--shared',
           '-o',
           outFile!.toFilePath(),
@@ -310,6 +313,19 @@ class RunCBuilder {
         ],
         ...linkerOptions?.postSourcesFlags(toolInstance.tool, sourceFiles) ??
             [],
+        if (executable != null || dynamicLibrary != null) ...[
+          if (config.targetOS case OS.android || OS.linux)
+            // During bundling code assets are all placed in the same directory.
+            // Setting this rpath allows the binary to find other code assets
+            // it is linked against.
+            if (linkerOptions != null)
+              '-rpath=\$ORIGIN'
+            else
+              '-Wl,-rpath=\$ORIGIN',
+          for (final directory in libraryDirectories)
+            '-L${directory.toFilePath()}',
+          for (final library in libraries) '-l$library',
+        ],
       ],
       logger: logger,
       captureOutput: false,
@@ -344,17 +360,20 @@ class RunCBuilder {
           ...sources.map((e) => e.toFilePath()),
           '/link',
           '/out:${outDir.resolveUri(executable!).toFilePath()}',
-        ],
-        if (dynamicLibrary != null) ...[
+        ] else if (dynamicLibrary != null) ...[
           ...sources.map((e) => e.toFilePath()),
           '/link',
           '/DLL',
           '/out:${outDir.resolveUri(dynamicLibrary!).toFilePath()}',
-        ],
-        if (staticLibrary != null) ...[
+        ] else if (staticLibrary != null) ...[
           '/c',
           ...sources.map((e) => e.toFilePath()),
         ],
+        if (executable != null || dynamicLibrary != null) ...[
+          for (final directory in libraryDirectories)
+            '/LIBPATH:${directory.toFilePath()}',
+          for (final library in libraries) '$library.lib',
+        ],
       ],
       workingDirectory: outDir,
       environment: environment,