[native_toolchain_c] Skip building dylib if code asset type not reque…

…sted (#1843) Follow up of: * #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.
dart-lang · Dec 20, 2024 · 2a4d563 · 2a4d563
1 parent 9596c1f
commit 2a4d563
Show file tree

Hide file tree

Showing 2 changed files with 83 additions and 71 deletions.
diff --git 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
@@ -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 = <String>[];
-        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 = <String>[];
+          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<Int32 Function(Int32, Int32),
-              int Function(int, int)>('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<Int32 Function(Int32, Int32),
+                int Function(int, int)>('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'));
+            }
           }
-        }
-      });
+        });
+      }
     }
   }