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_assets_cli] Redesign API step 1 #946

Merged
merged 77 commits into from
Mar 14, 2024
Merged
Show file tree
Hide file tree
Changes from 58 commits
Commits
Show all changes
77 commits
Select commit Hold shift + click to select a range
adbdd0f
[native_assets_cli] Redesign API
dcharkes Jan 18, 2024
6533972
Surpress deprecation warnings to appease CI
dcharkes Jan 22, 2024
cbdbc1e
changelog entries
dcharkes Jan 22, 2024
30964ac
Merge branch 'main' into native-assets-cli-build-output
dcharkes Jan 22, 2024
499feae
Address comments
dcharkes Jan 22, 2024
027fece
Remove `BuildState`
dcharkes Jan 23, 2024
e4d9b01
Sketch an `AssetDownloader`
dcharkes Jan 24, 2024
17d40dc
Merge branch 'main' into native-assets-cli-build-output
dcharkes Feb 15, 2024
5deb29c
Remove _2 fields
dcharkes Feb 15, 2024
a580dfb
changelogs
dcharkes Feb 15, 2024
b0d4cdc
cleanup
dcharkes Feb 15, 2024
12108ad
fix wrong deps version
dcharkes Feb 15, 2024
b0bc231
Add local asset example
dcharkes Feb 15, 2024
37de6cf
Add both singular and plural API
dcharkes Feb 15, 2024
cef032f
update examples
dcharkes Feb 16, 2024
09e5a3e
Use parts file for API
dcharkes Feb 16, 2024
7af60c0
Rename `Asset` to `CCodeAsset`
dcharkes Feb 16, 2024
e6794fb
Move c code assets
dcharkes Feb 16, 2024
e1e4f22
Add asset type to protocol
dcharkes Feb 16, 2024
8915b77
Parse assets based on asset type
dcharkes Feb 16, 2024
bee8f2d
Introduce `Asset` with `id` and `file`.
dcharkes Feb 19, 2024
9d9f894
Split `CCodeAsset.target` in `os` and `architecture`.
dcharkes Feb 19, 2024
624383f
Make build runner expand the architectures for CCodeAssets in dry run
dcharkes Feb 19, 2024
c772761
Rename `AssetPathType` to `DynamicLinking`
dcharkes Feb 19, 2024
6c5003b
Add a supported_asset_types to BuildConfig
dcharkes Feb 19, 2024
564eeee
Output older BuildOutput format if old BuildConfig was passed
dcharkes Feb 20, 2024
1f1509a
bump changelog
dcharkes Feb 20, 2024
dcd2165
Bump SDK constraint to use field promotion
dcharkes Feb 20, 2024
752dee9
Fix test on Windows
dcharkes Feb 20, 2024
ba4a81f
Make `BuildConfig.targetArchitecture` nullable instead of throw
dcharkes Feb 20, 2024
7a0c668
Increase test coverage
dcharkes Feb 20, 2024
e9a49a0
Add `DataAsset`s
dcharkes Feb 20, 2024
8170ef8
Fix windows test
dcharkes Feb 20, 2024
b8822ef
Fix test try 2
dcharkes Feb 20, 2024
81b7260
Iterate on documentation in public API
dcharkes Feb 21, 2024
8d5da09
Move Architecture and OS to their own files
dcharkes Feb 21, 2024
f539b85
Move c compiler config into its own file
dcharkes Feb 21, 2024
f1544e3
Mark all classes in the API final
dcharkes Feb 21, 2024
2958a2a
Another documentation round
dcharkes Feb 21, 2024
622b798
Cleanup Target.current use
dcharkes Feb 21, 2024
e488607
Migrate away from Target
dcharkes Feb 21, 2024
1154c95
Rename `TargetImpl` to `Target`
dcharkes Feb 21, 2024
51e71e6
Add changelog entries and bump version
dcharkes Feb 21, 2024
7ac882f
Use Dart casing `targetOs` -> `targetOS`
dcharkes Feb 21, 2024
300c206
data asset test coverage
dcharkes Feb 21, 2024
43ce175
Address comment
dcharkes Feb 23, 2024
f697827
Address comment
dcharkes Feb 23, 2024
5ba7e77
Address comments
dcharkes Feb 26, 2024
9992ce2
address comment
dcharkes Feb 27, 2024
21d5f83
address comment
dcharkes Feb 27, 2024
aa4d83e
Improve docs
dcharkes Feb 27, 2024
0ffe6e4
Make all asset ids use a `package:...`
dcharkes Feb 27, 2024
653dd16
cleanup
dcharkes Feb 27, 2024
a1a3554
address comments
dcharkes Feb 28, 2024
d82edcd
address comment
dcharkes Feb 28, 2024
84d77b4
Add Argument/State errors to DynamicLoading with LinkMode.static
dcharkes Feb 28, 2024
818ac15
More documentation for CCodeAssets and more errors
dcharkes Feb 28, 2024
03c44d2
Fix comment
dcharkes Feb 28, 2024
bc77f0d
Split up asset id into package and name in constructors
dcharkes Mar 4, 2024
4116376
Address comments
dcharkes Mar 4, 2024
ea8e581
Fix test
dcharkes Mar 4, 2024
9ed573f
Fix more tests
dcharkes Mar 4, 2024
230be89
address comment
dcharkes Mar 4, 2024
ab85d47
add test
dcharkes Mar 4, 2024
0ef0dcc
Remove `AssetDownloader` for now
dcharkes Mar 12, 2024
c66a963
Make BuildConfig loading sync
dcharkes Mar 12, 2024
7dab2e0
Remove unused imports
dcharkes Mar 12, 2024
8fa2b2e
Make `BuildConfig.fromArguments` the default constructor
dcharkes Mar 12, 2024
491cc25
Rename `CCodeAsset` to `NativeCodeAsset`
dcharkes Mar 12, 2024
0411dd9
Fix analysis
dcharkes Mar 12, 2024
88863be
Remove link between `LinkModePreference` and `LinkMode`
dcharkes Mar 12, 2024
05e473e
Rename `LinkMode.dynamic` -> `LinkMode.dynamicLoading`
dcharkes Mar 12, 2024
8134f46
Merge `DynamicLoadingMode` into `LinkMode`
dcharkes Mar 13, 2024
acd8a22
address comments
dcharkes Mar 14, 2024
81353f0
Pipe through `supportedAssetTypes`
dcharkes Mar 14, 2024
7e996d3
update example
dcharkes Mar 14, 2024
8dc1ef7
undo copyright comment change
dcharkes Mar 14, 2024
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
4 changes: 4 additions & 0 deletions pkgs/native_assets_builder/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
## 0.6.0-wip

- **Breaking change** Completely rewritten API in `native_assets_cli`.

## 0.5.0

- **Breaking change**: Hide implementation of `KernelAssets`.
Expand Down
112 changes: 66 additions & 46 deletions pkgs/native_assets_builder/lib/src/build_runner/build_runner.dart
Original file line number Diff line number Diff line change
Expand Up @@ -36,12 +36,12 @@ class NativeAssetsBuildRunner {
/// If provided, only native assets of all transitive dependencies of
/// [runPackageName] are built.
Future<BuildResult> build({
required LinkModePreference linkModePreference,
required LinkModePreferenceImpl linkModePreference,
required Target target,
required Uri workingDirectory,
required BuildMode buildMode,
CCompilerConfig? cCompilerConfig,
IOSSdk? targetIOSSdk,
required BuildModeImpl buildMode,
CCompilerConfigImpl? cCompilerConfig,
IOSSdkImpl? targetIOSSdk,
int? targetAndroidNdkApi,
required bool includeParentEnvironment,
PackageLayout? packageLayout,
Expand Down Expand Up @@ -77,7 +77,7 @@ class NativeAssetsBuildRunner {
buildPlan = plan;
packageGraph = planner.packageGraph;
}
final assets = <Asset>[];
final assets = <AssetImpl>[];
final dependencies = <Uri>[];
final metadata = <String, Metadata>{};
var success = true;
Expand Down Expand Up @@ -132,8 +132,8 @@ class NativeAssetsBuildRunner {
/// If provided, only native assets of all transitive dependencies of
/// [runPackageName] are built.
Future<DryRunResult> dryRun({
required LinkModePreference linkModePreference,
required OS targetOs,
required LinkModePreferenceImpl linkModePreference,
required OSImpl targetOS,
required Uri workingDirectory,
required bool includeParentEnvironment,
PackageLayout? packageLayout,
Expand Down Expand Up @@ -163,13 +163,13 @@ class NativeAssetsBuildRunner {
}
buildPlan = plan;
}
final assets = <Asset>[];
final assets = <AssetImpl>[];
var success = true;
for (final package in buildPlan) {
final config = await _cliConfigDryRun(
packageName: package.name,
packageRoot: packageLayout.packageRoot(package.name),
targetOs: targetOs,
targetOS: targetOS,
linkMode: linkModePreference,
buildParentDir: packageLayout.dartToolNativeAssetsBuilder,
);
Expand All @@ -180,7 +180,26 @@ class NativeAssetsBuildRunner {
includeParentEnvironment,
dryRun: true,
);
assets.addAll(packageAssets);
for (final asset in packageAssets) {
switch (asset) {
case CCodeAssetImpl _:
if (asset.architecture != null) {
// Backwards compatibility, if an architecture is provided use it.
assets.add(asset);
} else {
// Dry run does not report architecture. Dart VM branches on OS
// and Target when looking up assets, so populate assets for all
// architectures.
for (final architecture in asset.os.architectures) {
assets.add(asset.copyWith(
architecture: architecture,
));
}
}
case DataAssetImpl _:
assets.add(asset);
}
}
success &= packageSuccess;
}
return _DryRunResultImpl(
Expand All @@ -190,21 +209,21 @@ class NativeAssetsBuildRunner {
}

Future<_PackageBuildRecord> _buildPackageCached(
BuildConfig config,
BuildConfigImpl config,
Uri packageConfigUri,
Uri workingDirectory,
bool includeParentEnvironment,
) async {
final packageName = config.packageName;
final outDir = config.outDir;
final outDir = config.outputDirectory;
if (!await Directory.fromUri(outDir).exists()) {
await Directory.fromUri(outDir).create(recursive: true);
}

final buildOutput = await BuildOutput.readFromFile(outDir: outDir);
final buildOutput = await BuildOutputImpl.readFromFile(outDir: outDir);
final lastBuilt = buildOutput?.timestamp.roundDownToSeconds() ??
DateTime.fromMillisecondsSinceEpoch(0);
final dependencies = buildOutput?.dependencies;
final dependencies = buildOutput?.dependenciesModel;
final lastChange = await dependencies?.lastModified() ?? DateTime.now();

if (lastBuilt.isAfter(lastChange)) {
Expand All @@ -213,8 +232,8 @@ class NativeAssetsBuildRunner {
// All build flags go into [outDir]. Therefore we do not have to check
// here whether the config is equal.
final assets = buildOutput!.assets;
final dependencies = buildOutput.dependencies.dependencies;
final metadata = buildOutput.metadata;
final dependencies = buildOutput.dependencies;
final metadata = buildOutput.metadataModel;
return (assets, dependencies, metadata, true);
}

Expand All @@ -228,19 +247,20 @@ class NativeAssetsBuildRunner {
}

Future<_PackageBuildRecord> _buildPackage(
BuildConfig config,
BuildConfigImpl config,
Uri packageConfigUri,
Uri workingDirectory,
bool includeParentEnvironment, {
required bool dryRun,
}) async {
final outDir = config.outDir;
final outDir = config.outputDirectory;
final configFile = outDir.resolve('../config.yaml');
final buildDotDart = config.packageRoot.resolve('build.dart');
final configFileContents = config.toYamlString();
logger.info('config.yaml contents: $configFileContents');
await File.fromUri(configFile).writeAsString(configFileContents);
final buildOutputFile = File.fromUri(outDir.resolve(BuildOutput.fileName));
final buildOutputFile =
File.fromUri(outDir.resolve(BuildOutputImpl.fileName));
if (await buildOutputFile.exists()) {
// Ensure we'll never read outdated build results.
await buildOutputFile.delete();
Expand Down Expand Up @@ -282,11 +302,11 @@ ${result.stdout}
}

try {
final buildOutput = await BuildOutput.readFromFile(outDir: outDir);
final buildOutput = await BuildOutputImpl.readFromFile(outDir: outDir);
final assets = buildOutput?.assets ?? [];
success &= validateAssetsPackage(assets, config.packageName);
final dependencies = buildOutput?.dependencies.dependencies ?? [];
final metadata = dryRun ? null : buildOutput?.metadata;
final dependencies = buildOutput?.dependencies ?? [];
final metadata = dryRun ? null : buildOutput?.metadataModel;
return (assets, dependencies, metadata, success);
} on FormatException catch (e) {
logger.severe('''
Expand All @@ -295,7 +315,7 @@ build_output.yaml contained a format error.
${e.message}
''');
success = false;
return (<Asset>[], <Uri>[], const Metadata({}), false);
return (<CCodeAssetImpl>[], <Uri>[], const Metadata({}), false);
// TODO(https://github.com/dart-lang/native/issues/109): Stop throwing
// type errors in native_assets_cli, release a new version of that package
// and then remove this.
Expand All @@ -306,34 +326,34 @@ Building native assets for package:${config.packageName} failed.
build_output.yaml contained a format error.
''');
success = false;
return (<Asset>[], <Uri>[], const Metadata({}), false);
return (<CCodeAssetImpl>[], <Uri>[], const Metadata({}), false);
} finally {
if (!success) {
final buildOutputFile =
File.fromUri(outDir.resolve(BuildOutput.fileName));
File.fromUri(outDir.resolve(BuildOutputImpl.fileName));
if (await buildOutputFile.exists()) {
await buildOutputFile.delete();
}
}
}
}

static Future<BuildConfig> _cliConfig({
static Future<BuildConfigImpl> _cliConfig({
required String packageName,
required Uri packageRoot,
required Target target,
IOSSdk? targetIOSSdk,
IOSSdkImpl? targetIOSSdk,
int? targetAndroidNdkApi,
required BuildMode buildMode,
required LinkModePreference linkMode,
required BuildModeImpl buildMode,
required LinkModePreferenceImpl linkMode,
required Uri buildParentDir,
CCompilerConfig? cCompilerConfig,
CCompilerConfigImpl? cCompilerConfig,
DependencyMetadata? dependencyMetadata,
}) async {
final buildDirName = BuildConfig.checksum(
final buildDirName = BuildConfigImpl.checksum(
packageName: packageName,
packageRoot: packageRoot,
targetOs: target.os,
targetOS: target.os,
targetArchitecture: target.architecture,
buildMode: buildMode,
linkModePreference: linkMode,
Expand All @@ -348,11 +368,11 @@ build_output.yaml contained a format error.
// TODO(https://dartbug.com/50565): Purge old or unused folders.
await outDir.create(recursive: true);
}
return BuildConfig(
return BuildConfigImpl(
outDir: outDirUri,
packageName: packageName,
packageRoot: packageRoot,
targetOs: target.os,
targetOS: target.os,
targetArchitecture: target.architecture,
buildMode: buildMode,
linkModePreference: linkMode,
Expand All @@ -363,24 +383,24 @@ build_output.yaml contained a format error.
);
}

static Future<BuildConfig> _cliConfigDryRun({
static Future<BuildConfigImpl> _cliConfigDryRun({
required String packageName,
required Uri packageRoot,
required OS targetOs,
required LinkModePreference linkMode,
required OSImpl targetOS,
required LinkModePreferenceImpl linkMode,
required Uri buildParentDir,
}) async {
final buildDirName = 'dry_run_${targetOs}_$linkMode';
final buildDirName = 'dry_run_${targetOS}_$linkMode';
final outDirUri = buildParentDir.resolve('$buildDirName/out/');
final outDir = Directory.fromUri(outDirUri);
if (!await outDir.exists()) {
await outDir.create(recursive: true);
}
return BuildConfig.dryRun(
return BuildConfigImpl.dryRun(
outDir: outDirUri,
packageName: packageName,
packageRoot: packageRoot,
targetOs: targetOs,
targetOS: targetOS,
linkModePreference: linkMode,
);
}
Expand All @@ -400,7 +420,7 @@ build_output.yaml contained a format error.
};
}

bool validateAssetsPackage(List<Asset> assets, String packageName) {
bool validateAssetsPackage(Iterable<AssetImpl> assets, String packageName) {
final invalidAssetIds = assets
.map((a) => a.id)
.where((n) => !n.startsWith('package:$packageName/'))
Expand All @@ -419,16 +439,16 @@ build_output.yaml contained a format error.
}

typedef _PackageBuildRecord = (
List<Asset>,
List<Uri> dependencies,
Iterable<AssetImpl>,
Iterable<Uri> dependencies,
Metadata?,
bool success,
);

/// The result from a [NativeAssetsBuildRunner.dryRun].
abstract interface class DryRunResult {
/// The native assets for all [Target]s for the build or dry run.
List<Asset> get assets;
List<AssetImpl> get assets;

/// Whether all builds completed without errors.
///
Expand All @@ -438,7 +458,7 @@ abstract interface class DryRunResult {

final class _DryRunResultImpl implements DryRunResult {
@override
final List<Asset> assets;
final List<AssetImpl> assets;

@override
final bool success;
Expand All @@ -462,7 +482,7 @@ abstract class BuildResult implements DryRunResult {

final class _BuildResultImpl implements BuildResult {
@override
final List<Asset> assets;
final List<AssetImpl> assets;

@override
final List<Uri> dependencies;
Expand Down
12 changes: 7 additions & 5 deletions pkgs/native_assets_builder/pubspec.yaml
Original file line number Diff line number Diff line change
@@ -1,18 +1,20 @@
name: native_assets_builder
description: >-
This package is the backend that invokes top-level `build.dart` scripts.
version: 0.5.0
version: 0.6.0-wip
repository: https://github.com/dart-lang/native/tree/main/pkgs/native_assets_builder

environment:
sdk: '>=3.0.0 <4.0.0'
sdk: '>=3.3.0 <4.0.0'

publish_to: none

dependencies:
graphs: ^2.3.1
logging: ^1.2.0
native_assets_cli: ^0.4.2
# native_assets_cli:
# path: ../native_assets_cli/
# native_assets_cli: ^0.4.2
native_assets_cli:
path: ../native_assets_cli/
package_config: ^2.1.0
yaml: ^3.1.2
yaml_edit: ^2.1.0
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

import 'dart:io';

import 'package:native_assets_cli/native_assets_cli_internal.dart';
import 'package:test/test.dart';

import '../helpers.dart';
Expand Down Expand Up @@ -74,7 +75,8 @@ void main() async {

{
final result = await build(packageUri, logger, dartExecutable);
await expectSymbols(asset: result.assets.single, symbols: ['add']);
await expectSymbols(
asset: result.assets.single as CCodeAssetImpl, symbols: ['add']);
}

await copyTestProjects(
Expand All @@ -85,7 +87,7 @@ void main() async {
{
final result = await build(packageUri, logger, dartExecutable);
await expectSymbols(
asset: result.assets.single,
asset: result.assets.single as CCodeAssetImpl,
symbols: ['add', 'subtract'],
);
}
Expand All @@ -101,7 +103,8 @@ void main() async {

{
final result = await build(packageUri, logger, dartExecutable);
await expectSymbols(asset: result.assets.single, symbols: ['add']);
await expectSymbols(
asset: result.assets.single as CCodeAssetImpl, symbols: ['add']);
}

await copyTestProjects(
Expand All @@ -111,7 +114,8 @@ void main() async {
{
final result = await build(packageUri, logger, dartExecutable);
await expectSymbols(
asset: result.assets.single, symbols: ['add', 'multiply']);
asset: result.assets.single as CCodeAssetImpl,
symbols: ['add', 'multiply']);
}
});
});
Expand Down
Loading
Loading