[native_assets_builder] build.dart -> hook/build.dart (#1018)

* #823

Notes:

* Should keep existing toplevel `build.dart` scripts working by trying both `hook/build.dart` and `build.dart`.
* Does not migrate the examples yet, so that we don't have to do `breaking-change` is true in the ci.
   * Will do this after rolling into Dart.
* `native_toolchain_c` had a `this.dartBuildFiles = const ['build.dart'],`. Updating that to `hook/build.dart` would not work for unmigrated scripts, because files listed as dependencies that don't exist will also trigger a rebuild, e.g. it is considered deleting a file. It would also break on any kind of invocation from `hook/link.dart`. So, I've removed the default value for now. It's not pretty though. Ideas @mosuem?

pkgs/native_assets_builder/CHANGELOG.md

@@ -1,6 +1,9 @@
 ## 0.6.0-wip
 
 - **Breaking change** Completely rewritten API in `native_assets_cli`.
+- **Breaking change** Move `build.dart` to `hook/build.dart`.
+  https://github.com/dart-lang/native/issues/823
+  (Backwards compatibility, fallback to toplevel `build.dart`.)
 
 ## 0.5.0
 

pkgs/native_assets_builder/lib/src/build_runner/build_runner.dart

@@ -259,7 +259,8 @@ class NativeAssetsBuildRunner {
   }) async {
     final outDir = config.outputDirectory;
     final configFile = outDir.resolve('../config.yaml');
-    final buildDotDart = config.packageRoot.resolve('build.dart');
+    final buildHook = config.packageRoot.resolve('hook/').resolve('build.dart');
+    final buildHookLegacy = config.packageRoot.resolve('build.dart');
     final configFileContents = config.toYamlString();
     logger.info('config.yaml contents: $configFileContents');
     await File.fromUri(configFile).writeAsString(configFileContents);
@@ -269,9 +270,13 @@ class NativeAssetsBuildRunner {
       // Ensure we'll never read outdated build results.
       await buildOutputFile.delete();
     }
+
     final arguments = [
       '--packages=${packageConfigUri.toFilePath()}',
-      buildDotDart.toFilePath(),
+      if (await File.fromUri(buildHook).exists())
+        buildHook.toFilePath()
+      else
+        buildHookLegacy.toFilePath(),
       '--config=${configFile.toFilePath()}',
     ];
     final result = await runProcess(

pkgs/native_assets_builder/lib/src/package_layout/package_layout.dart

@@ -86,15 +86,23 @@ class PackageLayout {
   /// All packages in [packageConfig] with native assets.
   ///
   /// Whether a package has native assets is defined by whether it contains
-  /// a `build.dart`.
+  /// a `hook/build.dart`.
   ///
-  /// `package:native` itself is excluded.
+  /// For backwards compatibility, a toplevel `build.dart` is also supported.
+  // TODO(https://github.com/dart-lang/native/issues/823): Remove fallback when
+  // everyone has migrated. (Probably once we stop backwards compatibility of
+  // the protocol version pre 1.2.0 on some future version.)
   late final Future<List<Package>> packagesWithNativeAssets = () async {
     final result = <Package>[];
     for (final package in packageConfig.packages) {
       final packageRoot = package.root;
       if (packageRoot.scheme == 'file') {
-        if (await File.fromUri(packageRoot.resolve('build.dart')).exists()) {
+        if (await File.fromUri(
+              packageRoot.resolve('hook/').resolve('build.dart'),
+            ).exists() ||
+            await File.fromUri(
+              packageRoot.resolve('build.dart'),
+            ).exists()) {
           result.add(package);
         }
       }

pkgs/native_assets_builder/pubspec.yaml

@@ -1,6 +1,6 @@
 name: native_assets_builder
 description: >-
-  This package is the backend that invokes top-level `build.dart` scripts.
+  This package is the backend that invokes `build.dart` hooks.
 version: 0.6.0-wip
 repository: https://github.com/dart-lang/native/tree/main/pkgs/native_assets_builder
 

pkgs/native_assets_builder/test/build_runner/build_dependencies_test.dart

@@ -32,18 +32,20 @@ void main() async {
           logMessages.join('\n'),
           stringContainsInOrder(
             [
-              'native_add${Platform.pathSeparator}build.dart',
-              'native_subtract${Platform.pathSeparator}build.dart'
+              'native_add${Platform.pathSeparator}hook'
+                  '${Platform.pathSeparator}build.dart',
+              'native_subtract${Platform.pathSeparator}hook'
+                  '${Platform.pathSeparator}build.dart',
             ],
           ),
         );
         expect(result.assets.length, 2);
         expect(
           result.dependencies,
           [
-            tempUri.resolve('native_add/').resolve('build.dart'),
+            tempUri.resolve('native_add/').resolve('hook/build.dart'),
             tempUri.resolve('native_add/').resolve('src/native_add.c'),
-            tempUri.resolve('native_subtract/').resolve('build.dart'),
+            tempUri.resolve('native_subtract/').resolve('hook/build.dart'),
             tempUri
                 .resolve('native_subtract/')
                 .resolve('src/native_subtract.c'),

pkgs/native_assets_builder/test/build_runner/build_runner_caching_test.dart

@@ -29,12 +29,15 @@ void main() async {
             capturedLogs: logMessages);
         expect(
           logMessages.join('\n'),
-          contains('native_add${Platform.pathSeparator}build.dart'),
+          contains(
+            'native_add${Platform.pathSeparator}hook'
+            '${Platform.pathSeparator}build.dart',
+          ),
         );
         expect(
           result.dependencies,
           [
-            packageUri.resolve('build.dart'),
+            packageUri.resolve('hook/build.dart'),
             packageUri.resolve('src/native_add.c'),
           ],
         );
@@ -50,12 +53,15 @@ void main() async {
         );
         expect(
           logMessages.join('\n'),
-          isNot(contains('native_add${Platform.pathSeparator}build.dart')),
+          isNot(contains(
+            'native_add${Platform.pathSeparator}hook'
+            '${Platform.pathSeparator}build.dart',
+          )),
         );
         expect(
           result.dependencies,
           [
-            packageUri.resolve('build.dart'),
+            packageUri.resolve('hook/build.dart'),
             packageUri.resolve('src/native_add.c'),
           ],
         );

pkgs/native_assets_builder/test/build_runner/build_runner_failure_test.dart

@@ -33,7 +33,7 @@ void main() async {
         expect(
           result.dependencies,
           [
-            packageUri.resolve('build.dart'),
+            packageUri.resolve('hook/build.dart'),
             packageUri.resolve('src/native_add.c'),
           ],
         );
@@ -78,7 +78,7 @@ void main() async {
         expect(
           result.dependencies,
           [
-            packageUri.resolve('build.dart'),
+            packageUri.resolve('hook/build.dart'),
             packageUri.resolve('src/native_add.c'),
           ],
         );

pkgs/native_assets_builder/test/build_runner/build_runner_non_root_package_test.dart

@@ -48,13 +48,16 @@ void main() async {
         expect(
           result.dependencies,
           [
-            packageUri.resolve('build.dart'),
+            packageUri.resolve('hook/build.dart'),
             packageUri.resolve('src/native_add.c'),
           ],
         );
         expect(
           logMessages.join('\n'),
-          contains('native_add${Platform.pathSeparator}build.dart'),
+          contains(
+            'native_add${Platform.pathSeparator}hook'
+            '${Platform.pathSeparator}build.dart',
+          ),
         );
       }
     });

pkgs/native_assets_builder/test/build_runner/build_runner_test.dart

@@ -31,8 +31,10 @@ void main() async {
             capturedLogs: logMessages);
         expect(
             logMessages.join('\n'),
-            stringContainsInOrder(
-                ['native_add${Platform.pathSeparator}build.dart']));
+            stringContainsInOrder([
+              'native_add${Platform.pathSeparator}hook'
+                  '${Platform.pathSeparator}build.dart',
+            ]));
         expect(result.assets.length, 1);
       }
 
@@ -52,9 +54,10 @@ void main() async {
         );
         expect(
           false,
-          logMessages
-              .join('\n')
-              .contains('native_add${Platform.pathSeparator}build.dart'),
+          logMessages.join('\n').contains(
+                'native_add${Platform.pathSeparator}hook'
+                '${Platform.pathSeparator}build.dart',
+              ),
         );
         expect(result.assets.length, 1);
       }

pkgs/native_assets_builder/test/build_runner/metadata_test.dart

@@ -31,8 +31,10 @@ void main() async {
         expect(
             logMessages.join('\n'),
             stringContainsInOrder([
-              'package_with_metadata${Platform.pathSeparator}build.dart',
-              'package_reading_metadata${Platform.pathSeparator}build.dart',
+              'package_with_metadata${Platform.pathSeparator}hook'
+                  '${Platform.pathSeparator}build.dart',
+              'package_reading_metadata${Platform.pathSeparator}hook'
+                  '${Platform.pathSeparator}build.dart',
               '{some_int: 3, some_key: some_value}',
             ]));
       }

pkgs/native_assets_builder/test_data/manifest.yaml

@@ -1,39 +1,39 @@
 # The list of files to copy to a temporary folder to ensure running tests from
 # a completely clean setup.
-- cyclic_package_1/build.dart
+- cyclic_package_1/hook/build.dart
 - cyclic_package_1/pubspec.yaml
-- cyclic_package_2/build.dart
+- cyclic_package_2/hook/build.dart
 - cyclic_package_2/pubspec.yaml
 - dart_app/bin/dart_app.dart
 - dart_app/pubspec.yaml
-- native_add/build.dart
 - native_add/ffigen.yaml
+- native_add/hook/build.dart
 - native_add/lib/native_add.dart
 - native_add/lib/src/native_add_bindings_generated.dart
 - native_add/lib/src/native_add.dart
 - native_add/pubspec.yaml
 - native_add/src/native_add.c
 - native_add/src/native_add.h
 - native_add/test/native_add_test.dart
-- native_subtract/build.dart
 - native_subtract/ffigen.yaml
+- native_subtract/hook/build.dart
 - native_subtract/lib/native_subtract.dart
 - native_subtract/lib/src/native_subtract_bindings_generated.dart
 - native_subtract/lib/src/native_subtract.dart
 - native_subtract/pubspec.yaml
 - native_subtract/src/native_subtract.c
 - native_subtract/src/native_subtract.h
-- package_reading_metadata/build.dart
+- package_reading_metadata/hook/build.dart
 - package_reading_metadata/pubspec.yaml
-- package_with_metadata/build.dart
+- package_with_metadata/hook/build.dart
 - package_with_metadata/pubspec.yaml
 - some_dev_dep/bin/some_dev_dep.dart
 - some_dev_dep/pubspec.yaml
-- wrong_build_output/build.dart
-- wrong_build_output/pubspec.yaml
-- wrong_build_output_2/build.dart
+- wrong_build_output_2/hook/build.dart
 - wrong_build_output_2/pubspec.yaml
-- wrong_build_output_3/build.dart
+- wrong_build_output_3/hook/build.dart
 - wrong_build_output_3/pubspec.yaml
-- wrong_namespace_asset/build.dart
+- wrong_build_output/hook/build.dart
+- wrong_build_output/pubspec.yaml
+- wrong_namespace_asset/hook/build.dart
 - wrong_namespace_asset/pubspec.yaml

pkgs/native_assets_builder/test_data/native_add/build.dart → pkgs/native_assets_builder/test_data/native_add/hook/build.dart

@@ -16,6 +16,7 @@ void main(List<String> arguments) async {
       sources: [
         'src/$packageName.c',
       ],
+      dartBuildFiles: ['hook/build.dart'],
     );
     await cbuilder.run(
       buildConfig: config,

pkgs/native_assets_builder/test_data/native_add/manifest.yaml

@@ -1,5 +1,5 @@
-- build.dart
 - ffigen.yaml
+- hook/build.dart
 - lib/native_add.dart
 - lib/src/native_add_bindings_generated.dart
 - lib/src/native_add.dart

pkgs/native_assets_builder/test_data/native_add_add_source/build.dart → pkgs/native_assets_builder/test_data/native_add_add_source/hook/build.dart

@@ -17,6 +17,7 @@ void main(List<String> arguments) async {
         'src/$packageName.c',
         'src/native_multiply.c',
       ],
+      dartBuildFiles: ['hook/build.dart'],
     );
     await cbuilder.run(
       buildConfig: config,

pkgs/native_assets_builder/test_data/native_add_add_source/manifest.yaml

@@ -1,3 +1,3 @@
-- build.dart
+- hook/build.dart
 - src/native_multiply.c
 - src/native_multiply.h

pkgs/native_assets_builder/test_data/native_subtract/build.dart → pkgs/native_assets_builder/test_data/native_subtract/hook/build.dart

@@ -16,6 +16,7 @@ void main(List<String> arguments) async {
       sources: [
         'src/$packageName.c',
       ],
+      dartBuildFiles: ['hook/build.dart'],
     );
     await cbuilder.run(
       buildConfig: config,

pkgs/native_assets_builder/test_data/package_reading_metadata/pubspec.yaml

@@ -1,5 +1,5 @@
 name: package_reading_metadata
-description: Reads some metadata in its build.dart
+description: Reads some metadata in its hook/build.dart
 version: 0.1.0
 
 publish_to: none

pkgs/native_assets_builder/test_data/package_with_metadata/pubspec.yaml

@@ -1,5 +1,5 @@
 name: package_with_metadata
-description: Sets some metadata in its build.dart
+description: Sets some metadata in its hook/build.dart
 version: 0.1.0
 
 publish_to: none

pkgs/native_assets_cli/CHANGELOG.md

@@ -2,6 +2,8 @@
 
 - **Breaking change** Completely rewritten API.
   https://github.com/dart-lang/native/pull/946
+- **Breaking change** Move `build.dart` to `hook/build.dart`.
+  https://github.com/dart-lang/native/issues/823
 - Bump examples dependencies to path dependencies.
 
 ## 0.4.2

pkgs/native_assets_cli/example/local_asset/build.dart

@@ -27,6 +27,8 @@ void main(List<String> args) async {
 
       output.addDependencies([
         assetSourcePath,
+        // TODO(https://github.com/dart-lang/native/issues/823): Update after
+        // change is rolled into Dart SDK.
         config.packageRoot.resolve('build.dart'),
       ]);
     }

pkgs/native_assets_cli/example/native_add_library/build.dart

@@ -15,6 +15,9 @@ void main(List<String> args) async {
       sources: [
         'src/$packageName.c',
       ],
+      // TODO(https://github.com/dart-lang/native/issues/823): Update after
+      // change is rolled into Dart SDK.
+      dartBuildFiles: ['build.dart'],
     );
     await cbuilder.run(
       buildConfig: config,

pkgs/native_assets_cli/example/use_dart_api/build.dart

@@ -17,6 +17,9 @@ void main(List<String> arguments) async {
         'src/$packageName.c',
         'src/dart_api_dl.c',
       ],
+      // TODO(https://github.com/dart-lang/native/issues/823): Update after
+      // change is rolled into Dart SDK.
+      dartBuildFiles: ['build.dart'],
     );
     await cbuilder.run(
       buildConfig: config,

pkgs/native_assets_cli/example/use_dart_api/manifest.yaml

@@ -1,4 +1,4 @@
-- build.dart
+- hook/build.dart
 - ffigen.yaml
 - lib/native_add.dart
 - lib/src/native_add_bindings_generated.dart

pkgs/native_assets_cli/lib/src/api/build.dart

@@ -26,6 +26,7 @@ import 'build_output.dart';
 ///       sources: [
 ///         'src/$packageName.c',
 ///       ],
+///       dartBuildFiles: ['hook/build.dart'],
 ///     );
 ///     await cbuilder.run(
 ///       buildConfig: config,
@@ -66,7 +67,7 @@ import 'build_output.dart';
 ///
 ///       output.addDependencies([
 ///         assetSourcePath,
-///         config.packageRoot.resolve('build.dart'),
+///         config.packageRoot.resolve('hook/build.dart'),
 ///       ]);
 ///     }
 ///

pkgs/native_assets_cli/test/example/local_asset_test.dart

@@ -38,6 +38,8 @@ void main() async {
       final processResult = await Process.run(
         dartUri.toFilePath(),
         [
+          // TODO(https://github.com/dart-lang/native/issues/823): Update after
+          // change is rolled into Dart SDK.
           'build.dart',
           '-Dout_dir=${tempUri.toFilePath()}',
           '-Dpackage_name=$name',
@@ -83,6 +85,8 @@ void main() async {
           dependencies,
           [
             testPackageUri.resolve('data/asset.txt'),
+            // TODO(https://github.com/dart-lang/native/issues/823): Update after
+            // change is rolled into Dart SDK.
             testPackageUri.resolve('build.dart'),
           ],
         );