Skip to content

Commit

Permalink
[native_assets_cli] Nest CodeConfig OS-specific config (#1824)
Browse files Browse the repository at this point in the history 
Dart and Flutter have been passing the minimum OS SDK versions for a while (in non-dry-run), which means we can mark these fields non-optional if the target OS is used.

This PR changes the `CodeConfig` to nest the OS versions (and iOS target SDK: device or simulator) to be nested under the OS and required.

Other side effects of this PR:

* Many unit tests where missing these (now) mandatory fields
* `native_toolchain_c` can no longer test without the minimum OS versions, which changed the iOS test `otool` output.

Addressing:

* #1738 (comment)
  • Loading branch information
@dcharkes
dcharkes authored Dec 18, 2024
1 parent acc5534 commit 8c16b6c
Show file tree
Hide file tree
Showing 29 changed files with 403 additions and 165 deletions.
5 changes: 5 additions & 0 deletions pkgs/native_assets_builder/test/build_runner/build_runner_reusability_test.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,15 @@ void main() async {
dartExecutable: dartExecutable,
);

final targetOS = OS.current;
const defaultMacOSVersion = 13;
BuildConfigBuilder configCreator() => BuildConfigBuilder()
..setupCodeConfig(
targetArchitecture: Architecture.current,
targetOS: OS.current,
macOSConfig: targetOS == OS.macOS
? MacOSConfig(targetVersion: defaultMacOSVersion)
: null,
linkModePreference: LinkModePreference.dynamic,
);

Expand Down
8 changes: 7 additions & 1 deletion pkgs/native_assets_builder/test/build_runner/concurrency_shared_test_helper.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ void main(List<String> args) async {
..level = Level.ALL
..onRecord.listen((event) => print(event.message));

final targetOS = target.os;
final result = await NativeAssetsBuildRunner(
logger: logger,
dartExecutable: dartExecutable,
Expand All @@ -25,7 +26,10 @@ void main(List<String> args) async {
configCreator: () => BuildConfigBuilder()
..setupCodeConfig(
targetArchitecture: target.architecture,
targetOS: target.os,
targetOS: targetOS,
macOSConfig: targetOS == OS.macOS
? MacOSConfig(targetVersion: defaultMacOSVersion)
: null,
linkModePreference: LinkModePreference.dynamic,
),
workingDirectory: packageUri,
Expand All @@ -41,3 +45,5 @@ void main(List<String> args) async {
}
print('done');
}

int defaultMacOSVersion = 13;
7 changes: 5 additions & 2 deletions pkgs/native_assets_builder/test/build_runner/concurrency_test_helper.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ void main(List<String> args) async {
..level = Level.ALL
..onRecord.listen((event) => print(event.message));

final targetOS = OS.current;
final result = await NativeAssetsBuildRunner(
logger: logger,
dartExecutable: dartExecutable,
Expand All @@ -28,10 +29,12 @@ void main(List<String> args) async {
configCreator: () => BuildConfigBuilder()
..setupCodeConfig(
targetArchitecture: Architecture.current,
targetOS: OS.current,
targetOS: targetOS,
linkModePreference: LinkModePreference.dynamic,
cCompilerConfig: dartCICompilerConfig,
targetMacOSVersion: OS.current == OS.macOS ? defaultMacOSVersion : null,
macOSConfig: targetOS == OS.macOS
? MacOSConfig(targetVersion: defaultMacOSVersion)
: null,
),
workingDirectory: packageUri,
linkingEnabled: false,
Expand Down
75 changes: 55 additions & 20 deletions pkgs/native_assets_builder/test/build_runner/helpers.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,11 +62,19 @@ Future<BuildResult?> build(
targetOS: targetOS,
linkModePreference: linkModePreference,
cCompilerConfig: cCompilerConfig ?? dartCICompilerConfig,
targetIOSSdk: targetIOSSdk,
targetIOSVersion: targetIOSVersion,
targetMacOSVersion: targetMacOSVersion ??
(targetOS == OS.macOS ? defaultMacOSVersion : null),
targetAndroidNdkApi: targetAndroidNdkApi,
iOSConfig: targetOS == OS.iOS
? IOSConfig(
targetSdk: targetIOSSdk!,
targetVersion: targetIOSVersion!,
)
: null,
macOSConfig: targetOS == OS.macOS
? MacOSConfig(
targetVersion: targetMacOSVersion ?? defaultMacOSVersion)
: null,
androidConfig: targetOS == OS.android
? AndroidConfig(targetNdkApi: targetAndroidNdkApi!)
: null,
);
}
return configBuilder;
Expand Down Expand Up @@ -127,11 +135,19 @@ Future<LinkResult?> link(
targetOS: target?.os ?? OS.current,
linkModePreference: linkModePreference,
cCompilerConfig: cCompilerConfig ?? dartCICompilerConfig,
targetIOSSdk: targetIOSSdk,
targetIOSVersion: targetIOSVersion,
targetMacOSVersion: targetMacOSVersion ??
(targetOS == OS.macOS ? defaultMacOSVersion : null),
targetAndroidNdkApi: targetAndroidNdkApi,
iOSConfig: targetOS == OS.iOS
? IOSConfig(
targetSdk: targetIOSSdk!,
targetVersion: targetIOSVersion!,
)
: null,
macOSConfig: targetOS == OS.macOS
? MacOSConfig(
targetVersion: targetMacOSVersion ?? defaultMacOSVersion)
: null,
androidConfig: targetOS == OS.android
? AndroidConfig(targetNdkApi: targetAndroidNdkApi!)
: null,
);
}
return configBuilder;
Expand Down Expand Up @@ -181,17 +197,27 @@ Future<(BuildResult?, LinkResult?)> buildAndLink(
logger: logger,
dartExecutable: dartExecutable,
);
final targetOS = target?.os ?? OS.current;
final buildResult = await buildRunner.build(
configCreator: () => BuildConfigBuilder()
..setupCodeConfig(
targetArchitecture: target?.architecture ?? Architecture.current,
targetOS: target?.os ?? OS.current,
targetOS: targetOS,
linkModePreference: linkModePreference,
cCompilerConfig: cCompilerConfig ?? dartCICompilerConfig,
targetIOSSdk: targetIOSSdk,
targetIOSVersion: targetIOSVersion,
targetMacOSVersion: targetMacOSVersion,
targetAndroidNdkApi: targetAndroidNdkApi,
iOSConfig: targetOS == OS.iOS
? IOSConfig(
targetSdk: targetIOSSdk!,
targetVersion: targetIOSVersion!,
)
: null,
macOSConfig: targetOS == OS.macOS
? MacOSConfig(
targetVersion: targetMacOSVersion ?? defaultMacOSVersion)
: null,
androidConfig: targetOS == OS.android
? AndroidConfig(targetNdkApi: targetAndroidNdkApi!)
: null,
),
configValidator: buildConfigValidator,
workingDirectory: packageUri,
Expand All @@ -217,13 +243,22 @@ Future<(BuildResult?, LinkResult?)> buildAndLink(
configCreator: () => LinkConfigBuilder()
..setupCodeConfig(
targetArchitecture: target?.architecture ?? Architecture.current,
targetOS: target?.os ?? OS.current,
targetOS: targetOS,
linkModePreference: linkModePreference,
cCompilerConfig: cCompilerConfig,
targetIOSSdk: targetIOSSdk,
targetIOSVersion: targetIOSVersion,
targetMacOSVersion: targetMacOSVersion,
targetAndroidNdkApi: targetAndroidNdkApi,
iOSConfig: targetOS == OS.iOS
? IOSConfig(
targetSdk: targetIOSSdk!,
targetVersion: targetIOSVersion!,
)
: null,
macOSConfig: targetOS == OS.macOS
? MacOSConfig(
targetVersion: targetMacOSVersion ?? defaultMacOSVersion)
: null,
androidConfig: targetOS == OS.android
? AndroidConfig(targetNdkApi: targetAndroidNdkApi!)
: null,
),
configValidator: linkConfigValidator,
workingDirectory: packageUri,
Expand Down
8 changes: 7 additions & 1 deletion pkgs/native_assets_builder/test/test_data/native_dynamic_linking_test.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ void main() async {
final testPackageUri = testDataUri.resolve('$name/');
final dartUri = Uri.file(Platform.resolvedExecutable);

final targetOS = OS.current;
final configBuilder = BuildConfigBuilder()
..setupHookConfig(
packageName: name,
Expand All @@ -43,7 +44,10 @@ void main() async {
)
..setupCodeConfig(
targetArchitecture: Architecture.current,
targetOS: OS.current,
targetOS: targetOS,
macOSConfig: targetOS == OS.macOS
? MacOSConfig(targetVersion: defaultMacOSVersion)
: null,
linkModePreference: LinkModePreference.dynamic,
cCompilerConfig: cCompiler,
);
Expand Down Expand Up @@ -125,3 +129,5 @@ void main() async {
}),
);
}

int defaultMacOSVersion = 13;
6 changes: 5 additions & 1 deletion pkgs/native_assets_builder/test/test_data/transformer_test.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ void main() async {
late String stdout;
late BuildOutput output;

final targetOS = OS.current;
Future<void> runBuild(Architecture architecture) async {
final configBuilder = BuildConfigBuilder()
..setupHookConfig(
Expand All @@ -58,7 +59,10 @@ void main() async {
)
..setupCodeConfig(
targetArchitecture: architecture,
targetOS: OS.current,
targetOS: targetOS,
macOSConfig: targetOS == OS.macOS
? MacOSConfig(targetVersion: defaultMacOSVersion)
: null,
linkModePreference: LinkModePreference.dynamic,
);

Expand Down
11 changes: 5 additions & 6 deletions pkgs/native_assets_builder/test_data/fail_on_os_sdk_version/hook/build.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,27 +15,26 @@ const minMacOSVersionForThisPackage = 13;
void main(List<String> arguments) async {
await build(arguments, (config, output) async {
if (config.codeConfig.targetOS == OS.android) {
if (config.codeConfig.targetAndroidNdkApi! <
if (config.codeConfig.androidConfig.targetNdkApi <
minNdkApiVersionForThisPackage) {
throw UnsupportedError(
'The native assets for this package require at '
'least Android NDK API level $minNdkApiVersionForThisPackage.',
);
}
} else if (config.codeConfig.targetOS == OS.iOS) {
final iosVersion = config.codeConfig.targetIOSVersion;
final iosVersion = config.codeConfig.iOSConfig.targetVersion;
// iosVersion is nullable to deal with version skew.
if (iosVersion != null && iosVersion < minIosVersionForThisPackage) {
if (iosVersion < minIosVersionForThisPackage) {
throw UnsupportedError(
'The native assets for this package require at '
'least iOS version $minIosVersionForThisPackage.',
);
}
} else if (config.codeConfig.targetOS == OS.macOS) {
final macosVersion = config.codeConfig.targetMacOSVersion;
final macosVersion = config.codeConfig.macOSConfig.targetVersion;
// macosVersion is nullable to deal with version skew.
if (macosVersion != null &&
macosVersion < minMacOSVersionForThisPackage) {
if (macosVersion < minMacOSVersionForThisPackage) {
throw UnsupportedError(
'The native assets for this package require at '
'least MacOS version $minMacOSVersionForThisPackage.',
Expand Down
11 changes: 5 additions & 6 deletions pkgs/native_assets_builder/test_data/fail_on_os_sdk_version_linker/hook/link.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,27 +15,26 @@ const minMacOSVersionForThisPackage = 13;
void main(List<String> arguments) async {
await link(arguments, (config, output) async {
if (config.codeConfig.targetOS == OS.android) {
if (config.codeConfig.targetAndroidNdkApi! <
if (config.codeConfig.androidConfig.targetNdkApi <
minNdkApiVersionForThisPackage) {
throw UnsupportedError(
'The native assets for this package require at '
'least Android NDK API level $minNdkApiVersionForThisPackage.',
);
}
} else if (config.codeConfig.targetOS == OS.iOS) {
final iosVersion = config.codeConfig.targetIOSVersion;
final iosVersion = config.codeConfig.iOSConfig.targetVersion;
// iosVersion is nullable to deal with version skew.
if (iosVersion != null && iosVersion < minIosVersionForThisPackage) {
if (iosVersion < minIosVersionForThisPackage) {
throw UnsupportedError(
'The native assets for this package require at '
'least iOS version $minIosVersionForThisPackage.',
);
}
} else if (config.codeConfig.targetOS == OS.macOS) {
final macosVersion = config.codeConfig.targetMacOSVersion;
final macosVersion = config.codeConfig.macOSConfig.targetVersion;
// macosVersion is nullable to deal with version skew.
if (macosVersion != null &&
macosVersion < minMacOSVersionForThisPackage) {
if (macosVersion < minMacOSVersionForThisPackage) {
throw UnsupportedError(
'The native assets for this package require at '
'least MacOS version $minMacOSVersionForThisPackage.',
Expand Down
5 changes: 4 additions & 1 deletion pkgs/native_assets_cli/lib/code_assets.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,16 @@ export 'src/code_assets/c_compiler_config.dart' show CCompilerConfig;
export 'src/code_assets/code_asset.dart' show CodeAsset, OSLibraryNaming;
export 'src/code_assets/config.dart'
show
AndroidConfig,
CodeAssetBuildConfig,
CodeAssetBuildOutputBuilder,
CodeAssetBuildOutputBuilderAdd,
CodeAssetLinkConfig,
CodeAssetLinkOutputBuilder,
CodeAssetLinkOutputBuilderAdd,
CodeConfig;
CodeConfig,
IOSConfig,
MacOSConfig;
export 'src/code_assets/ios_sdk.dart' show IOSSdk;
export 'src/code_assets/link_mode.dart'
show
Expand Down
Loading

0 comments on commit 8c16b6c

Please sign in to comment.