From 2a4d56340e6eac705b15e5389f270169cf937509 Mon Sep 17 00:00:00 2001 From: Daco Harkes Date: Fri, 20 Dec 2024 13:37:53 +0100 Subject: [PATCH] [native_toolchain_c] Skip building dylib if code asset type not requested (#1843) Follow up of: * https://github.com/dart-lang/native/pull/1786 Hooks should not fail early if an asset type is not requested to build. In order to avoid hook writers having to wrap the `Builder`s for all the various asset types with statements checking that those asset types are requested, the builders themselves just no-op if the asset type is not requested. --- .../lib/src/cbuilder/cbuilder.dart | 5 + .../test/cbuilder/cbuilder_test.dart | 149 +++++++++--------- 2 files changed, 83 insertions(+), 71 deletions(-) diff --git a/pkgs/native_toolchain_c/lib/src/cbuilder/cbuilder.dart b/pkgs/native_toolchain_c/lib/src/cbuilder/cbuilder.dart index c2e3c0a34..5f0038277 100644 --- a/pkgs/native_toolchain_c/lib/src/cbuilder/cbuilder.dart +++ b/pkgs/native_toolchain_c/lib/src/cbuilder/cbuilder.dart @@ -117,6 +117,11 @@ class CBuilder extends CTool implements Builder { required Logger? logger, String? linkInPackage, }) async { + if (!config.buildAssetTypes.contains(CodeAsset.type)) { + logger?.info('buildAssetTypes did not contain "${CodeAsset.type}", ' + 'skipping CodeAsset $assetName build.'); + return; + } assert( config.linkingEnabled || linkInPackage == null, 'linkInPackage can only be provided if config.linkingEnabled is true.', diff --git a/pkgs/native_toolchain_c/test/cbuilder/cbuilder_test.dart b/pkgs/native_toolchain_c/test/cbuilder/cbuilder_test.dart index 2d826a2d3..e33c18afa 100644 --- a/pkgs/native_toolchain_c/test/cbuilder/cbuilder_test.dart +++ b/pkgs/native_toolchain_c/test/cbuilder/cbuilder_test.dart @@ -18,7 +18,7 @@ import 'package:test/test.dart'; import '../helpers.dart'; void main() { - test('Langauge.toString', () { + test('Language.toString', () { expect(Language.c.toString(), 'c'); expect(Language.cpp.toString(), 'c++'); }); @@ -118,80 +118,87 @@ void main() { } for (final dryRun in [true, false]) { - final suffix = testSuffix([if (dryRun) 'dry_run', picTag]); - - test('CBuilder dylib$suffix', () async { - final tempUri = await tempDirForTest(); - final tempUri2 = await tempDirForTest(); - final addCUri = - packageUri.resolve('test/cbuilder/testfiles/add/src/add.c'); - const name = 'add'; - - final logMessages = []; - final logger = createCapturingLogger(logMessages); - - final buildConfigBuilder = BuildConfigBuilder() - ..setupHookConfig( - buildAssetTypes: [CodeAsset.type], - packageName: name, - packageRoot: tempUri, - ) - ..setupBuildConfig( - linkingEnabled: false, - dryRun: dryRun, - ) - ..setupCodeConfig( - targetOS: targetOS, - macOSConfig: macOSConfig, - targetArchitecture: Architecture.current, - linkModePreference: LinkModePreference.dynamic, - cCompilerConfig: dryRun ? null : cCompiler, + for (final buildCodeAssets in [true, if (!dryRun) false]) { + final suffix = testSuffix([if (dryRun) 'dry_run', picTag]); + + test('CBuilder dylib$suffix', () async { + final tempUri = await tempDirForTest(); + final tempUri2 = await tempDirForTest(); + final addCUri = + packageUri.resolve('test/cbuilder/testfiles/add/src/add.c'); + const name = 'add'; + + final logMessages = []; + final logger = createCapturingLogger(logMessages); + + final buildConfigBuilder = BuildConfigBuilder() + ..setupHookConfig( + buildAssetTypes: [if (buildCodeAssets) CodeAsset.type], + packageName: name, + packageRoot: tempUri, + ) + ..setupBuildConfig( + linkingEnabled: false, + dryRun: dryRun, + ); + if (buildCodeAssets) { + buildConfigBuilder.setupCodeConfig( + targetOS: targetOS, + macOSConfig: macOSConfig, + targetArchitecture: Architecture.current, + linkModePreference: LinkModePreference.dynamic, + cCompilerConfig: dryRun ? null : cCompiler, + ); + } + buildConfigBuilder.setupBuildRunConfig( + outputDirectory: tempUri, + outputDirectoryShared: tempUri2, + ); + final buildConfig = BuildConfig(buildConfigBuilder.json); + final buildOutput = BuildOutputBuilder(); + + final cbuilder = CBuilder.library( + sources: [addCUri.toFilePath()], + name: name, + assetName: name, + pic: pic, + buildMode: BuildMode.release, + ); + await cbuilder.run( + config: buildConfig, + output: buildOutput, + logger: logger, ); - buildConfigBuilder.setupBuildRunConfig( - outputDirectory: tempUri, - outputDirectoryShared: tempUri2, - ); - final buildConfig = BuildConfig(buildConfigBuilder.json); - final buildOutput = BuildOutputBuilder(); - - final cbuilder = CBuilder.library( - sources: [addCUri.toFilePath()], - name: name, - assetName: name, - pic: pic, - buildMode: BuildMode.release, - ); - await cbuilder.run( - config: buildConfig, - output: buildOutput, - logger: logger, - ); - - final dylibUri = tempUri.resolve(OS.current.dylibFileName(name)); - expect(await File.fromUri(dylibUri).exists(), !dryRun); - if (!dryRun) { - final dylib = openDynamicLibraryForTest(dylibUri.toFilePath()); - final add = dylib.lookupFunction('add'); - expect(add(1, 2), 3); - final compilerInvocation = logMessages.singleWhere( - (message) => message.contains(addCUri.toFilePath()), + final dylibUri = tempUri.resolve(OS.current.dylibFileName(name)); + expect( + await File.fromUri(dylibUri).exists(), + equals(!dryRun && buildCodeAssets), ); - switch ((buildConfig.codeConfig.targetOS, pic)) { - case (OS.windows, _) || (_, null): - expect(compilerInvocation, isNot(contains('-fPIC'))); - expect(compilerInvocation, isNot(contains('-fPIE'))); - expect(compilerInvocation, isNot(contains('-fno-PIC'))); - expect(compilerInvocation, isNot(contains('-fno-PIE'))); - case (_, true): - expect(compilerInvocation, contains('-fPIC')); - case (_, false): - expect(compilerInvocation, contains('-fno-PIC')); - expect(compilerInvocation, contains('-fno-PIE')); + if (!dryRun && buildCodeAssets) { + final dylib = openDynamicLibraryForTest(dylibUri.toFilePath()); + final add = dylib.lookupFunction('add'); + expect(add(1, 2), 3); + + final compilerInvocation = logMessages.singleWhere( + (message) => message.contains(addCUri.toFilePath()), + ); + switch ((buildConfig.codeConfig.targetOS, pic)) { + case (OS.windows, _) || (_, null): + expect(compilerInvocation, isNot(contains('-fPIC'))); + expect(compilerInvocation, isNot(contains('-fPIE'))); + expect(compilerInvocation, isNot(contains('-fno-PIC'))); + expect(compilerInvocation, isNot(contains('-fno-PIE'))); + case (_, true): + expect(compilerInvocation, contains('-fPIC')); + case (_, false): + expect(compilerInvocation, contains('-fno-PIC')); + expect(compilerInvocation, contains('-fno-PIE')); + } } - } - }); + }); + } } }