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

[native_toolchain_c] Add CBuilder.dynamicallyLinkTo option #1423

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/native.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ jobs:

- run: dart --enable-experiment=native-assets test
working-directory: pkgs/${{ matrix.package }}/example/build/native_dynamic_linking/
if: ${{ matrix.package == 'native_assets_cli' && matrix.sdk == 'dev' && !matrix.breaking-chang && matrix.os != 'windows' }}
if: ${{ matrix.package == 'native_assets_cli' && matrix.sdk == 'dev' && !matrix.breaking-change }}

- run: dart --enable-experiment=native-assets test
working-directory: pkgs/${{ matrix.package }}/example/build/native_add_app/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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'),
dynamicallyLinkTo: ['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'),
dynamicallyLinkTo: ['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(
Expand All @@ -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'),
};
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,6 @@
'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.
@TestOn('!windows')
library;

import 'dart:io';
Expand Down
36 changes: 35 additions & 1 deletion pkgs/native_toolchain_c/lib/src/cbuilder/cbuilder.dart
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,15 @@ class CBuilder extends CTool implements Builder {
/// Defaults to `true`.
final bool ndebugDefine;

/// Libraries to dynamically link to.
///
/// The libraries are expected to be at the root of the output directory of
/// the build hook invocation.
///
/// When using this option ensure that the builders producing the libraries
/// to link to are run before this builder.
final List<String> dynamicallyLinkTo;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Are these strings names or file paths? Please document. Also, why prefer names over paths or vice versa? (Should probably be documented in the PR description as design choice.)


CBuilder.library({
required super.name,
super.assetName,
Expand All @@ -62,6 +71,7 @@ class CBuilder extends CTool implements Builder {
super.defines = const {},
this.buildModeDefine = true,
this.ndebugDefine = true,
this.dynamicallyLinkTo = const [],
super.pic = true,
super.std,
super.language = Language.c,
Expand All @@ -83,6 +93,7 @@ class CBuilder extends CTool implements Builder {
super.defines = const {},
this.buildModeDefine = true,
this.ndebugDefine = true,
this.dynamicallyLinkTo = const [],
bool? pie = false,
super.std,
super.language = Language.c,
Expand Down Expand Up @@ -146,7 +157,10 @@ class CBuilder extends CTool implements Builder {
executable: type == OutputType.executable ? exeUri : null,
// ignore: invalid_use_of_visible_for_testing_member
installName: installName,
flags: flags,
flags: [
...flags,
...config.dynamicLinkingFlags(dynamicallyLinkTo),
],
defines: {
...defines,
if (buildModeDefine) config.buildMode.name.toUpperCase(): null,
Expand Down Expand Up @@ -195,3 +209,23 @@ class CBuilder extends CTool implements Builder {
}
}
}

extension on BuildConfig {
List<String> dynamicLinkingFlags(List<String> libraries) =>
switch (targetOS) {
OS.macOS || OS.iOS => [
'-L${outputDirectory.toFilePath()}',
for (final library in libraries) '-l$library',
],
OS.linux || OS.android => [
'-Wl,-rpath=\$ORIGIN/.',
'-L${outputDirectory.toFilePath()}',
for (final library in libraries) '-l$library',
],
OS.windows => [
for (final library in libraries)
outputDirectory.resolve('$library.lib').toFilePath(),
Copy link
Collaborator

Choose a reason for hiding this comment

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

Users might output their libs somewhere else (a subdir for example), so maybe it's better to have List<Uri> libraries and be explicit about file paths.

If we go for file paths, then we might need to look up the name of the library for macOS/iOS/Linux/Android, unless the compiler allows file paths as well (as opposed to directory and library name.) But for those ditto, if the user compiles dylibs into a subdirectory of the target dir, the linking fails.

],
_ => throw UnimplementedError('Unsupported OS: $targetOS'),
};
}
63 changes: 63 additions & 0 deletions pkgs/native_toolchain_c/test/cbuilder/cbuilder_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -502,6 +502,69 @@ void main() {
expect(compilerInvocation, contains('-l stdc++'));
}
});

test('CBuilder dynamicallyLinkTo', () async {
final tempUri = await tempDirForTest();
final dynamicallyLinkedCUri = packageUri.resolve(
'test/cbuilder/testfiles/dynamically_linked/src/dynamically_linked.c');
final addSrcUri = packageUri.resolve('test/cbuilder/testfiles/add/src/');
final addCUri = addSrcUri.resolve('add.c');

if (!await File.fromUri(dynamicallyLinkedCUri).exists()) {
throw Exception('Run the test from the root directory.');
}
const name = 'dynamically_linked';

final logMessages = <String>[];
final logger = createCapturingLogger(logMessages);

final buildConfig = BuildConfig.build(
outputDirectory: tempUri,
packageName: name,
packageRoot: tempUri,
targetArchitecture: Architecture.current,
targetOS: OS.current,
buildMode: BuildMode.release,
// Ignored by executables.
linkModePreference: LinkModePreference.dynamic,
cCompiler: CCompilerConfig(
compiler: cc,
envScript: envScript,
envScriptArgs: envScriptArgs,
),
linkingEnabled: false,
);
final buildOutput = BuildOutput();

final builders = [
CBuilder.library(
name: 'add',
assetName: 'add',
sources: [addCUri.toFilePath()],
),
CBuilder.executable(
name: name,
includes: [addSrcUri.toFilePath()],
sources: [dynamicallyLinkedCUri.toFilePath()],
dynamicallyLinkTo: ['add'],
)
];
for (final builder in builders) {
await builder.run(
config: buildConfig,
output: buildOutput,
logger: logger,
);
}

final executableUri = tempUri.resolve(OS.current.executableFileName(name));
expect(await File.fromUri(executableUri).exists(), true);
final result = await runProcess(
executable: executableUri,
logger: logger,
);
expect(result.exitCode, 3);
});
}

Future<void> testDefines({
Expand Down
12 changes: 1 addition & 11 deletions pkgs/native_toolchain_c/test/cbuilder/testfiles/add/src/add.c
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,7 @@
// 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.

#include <stdint.h>

#ifdef DEBUG
#include <stdio.h>
#endif

#if _WIN32
#define FFI_EXPORT __declspec(dllexport)
#else
#define FFI_EXPORT
#endif
#include "add.h"

FFI_EXPORT int32_t add(int32_t a, int32_t b) {
#ifdef DEBUG
Expand Down
13 changes: 13 additions & 0 deletions pkgs/native_toolchain_c/test/cbuilder/testfiles/add/src/add.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
// Copyright (c) 2024, 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.

#include <stdint.h>

#if _WIN32
#define FFI_EXPORT __declspec(dllexport)
#else
#define FFI_EXPORT
#endif

FFI_EXPORT int32_t add(int32_t a, int32_t b);
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
// Copyright (c) 2024, 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.

#include "add.h"

int main() {
return add(1, 2);
}
Loading