Unverified Commit 0a18aa46 authored by Daco Harkes's avatar Daco Harkes Committed by GitHub

Default `NativeAssets` Darwin and IOS target archs if missing (#136948)

Make the `NativeAssets` target consistent with `build_info.dart`'s
documentation on missing `IosArchs` or `DarwinArchs`.

Bug:

* https://github.com/flutter/flutter/issues/136931

Also, updates the doc to reflect that MacOS is by default built for both
x64 and arm64. The PR making universal binaries the default didn't
update the doc comment.

* https://github.com/flutter/flutter/pull/100271

Please note that these defines are handled inconsistently in
`flutter_tools`. In some places they default to what's specified in the
doc-comment. In other places a `MissingDefineException` is thrown. I
believe the code around `build_info.dart` and the `environment` could
benefit from a wrapping so that defaults or missing definitions are
handled consistently in the code base.

## Pre-launch Checklist

- [x] I read the [Contributor Guide] and followed the process outlined
there for submitting PRs.
- [x] I read the [Tree Hygiene] wiki page, which explains my
responsibilities.
- [x] I read and followed the [Flutter Style Guide], including [Features
we expect every widget to implement].
- [x] I signed the [CLA].
- [x] I listed at least one issue that this PR fixes in the description
above.
- [x] I updated/added relevant documentation (doc comments with `///`).
- [ ] I added new tests to check the change I am making, or this PR is
[test-exempt].
- [ ] All existing and new tests are passing.

<!-- Links -->
[Contributor Guide]:
https://github.com/flutter/flutter/wiki/Tree-hygiene#overview
[Tree Hygiene]: https://github.com/flutter/flutter/wiki/Tree-hygiene
[test-exempt]:
https://github.com/flutter/flutter/wiki/Tree-hygiene#tests
[Flutter Style Guide]:
https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo
[Features we expect every widget to implement]:
https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo#features-we-expect-every-widget-to-implement
[CLA]: https://cla.developers.google.com/
[flutter/tests]: https://github.com/flutter/tests
[breaking change policy]:
https://github.com/flutter/flutter/wiki/Tree-hygiene#handling-breaking-changes
[Discord]: https://github.com/flutter/flutter/wiki/Chat
parent ddbf09c2
......@@ -920,7 +920,7 @@ const String kIosArchs = 'IosArchs';
/// The define to control what macOS architectures are built for.
///
/// This is expected to be a space-delimited list of architectures. If not
/// provided, defaults to x86_64.
/// provided, defaults to x86_64 and arm64.
///
/// Supported values are x86_64 and arm64.
const String kDarwinArchs = 'DarwinArchs';
......
......@@ -81,11 +81,12 @@ class NativeAssets extends Target {
switch (targetPlatform) {
case TargetPlatform.ios:
final String? iosArchsEnvironment = environment.defines[kIosArchs];
if (iosArchsEnvironment == null) {
throw MissingDefineException(kIosArchs, name);
}
final List<DarwinArch> iosArchs = iosArchsEnvironment.split(' ').map(getDarwinArchForName).toList();
final List<DarwinArch> iosArchs =
_emptyToNull(environment.defines[kIosArchs])
?.split(' ')
.map(getIOSArchForName)
.toList()
?? <DarwinArch>[DarwinArch.arm64];
final String? environmentBuildMode = environment.defines[kBuildMode];
if (environmentBuildMode == null) {
throw MissingDefineException(kBuildMode, name);
......@@ -107,11 +108,12 @@ class NativeAssets extends Target {
yamlParentDirectory: environment.buildDir.uri,
);
case TargetPlatform.darwin:
final String? darwinArchsEnvironment = environment.defines[kDarwinArchs];
if (darwinArchsEnvironment == null) {
throw MissingDefineException(kDarwinArchs, name);
}
final List<DarwinArch> darwinArchs = darwinArchsEnvironment.split(' ').map(getDarwinArchForName).toList();
final List<DarwinArch> darwinArchs =
_emptyToNull(environment.defines[kDarwinArchs])
?.split(' ')
.map(getDarwinArchForName)
.toList()
?? <DarwinArch>[DarwinArch.x86_64, DarwinArch.arm64];
final String? environmentBuildMode = environment.defines[kBuildMode];
if (environmentBuildMode == null) {
throw MissingDefineException(kBuildMode, name);
......@@ -249,3 +251,10 @@ class NativeAssets extends Target {
Source.pattern('{BUILD_DIR}/native_assets.yaml'),
];
}
String? _emptyToNull(String? input) {
if (input == null || input.isEmpty) {
return null;
}
return input;
}
......@@ -55,11 +55,19 @@ void main() {
expect(const NativeAssets().build(iosEnvironment), throwsA(isA<MissingDefineException>()));
});
testUsingContext('NativeAssets throws error if missing ios archs', () async {
testUsingContext('NativeAssets defaults to ios archs if missing', () async {
await createPackageConfig(iosEnvironment);
iosEnvironment.defines.remove(kIosArchs);
expect(const NativeAssets().build(iosEnvironment), throwsA(isA<MissingDefineException>()));
final NativeAssetsBuildRunner buildRunner = FakeNativeAssetsBuildRunner();
await NativeAssets(buildRunner: buildRunner).build(iosEnvironment);
final File nativeAssetsYaml =
iosEnvironment.buildDir.childFile('native_assets.yaml');
final File depsFile = iosEnvironment.buildDir.childFile('native_assets.d');
expect(depsFile, exists);
expect(nativeAssetsYaml, exists);
});
testUsingContext('NativeAssets throws error if missing sdk root', () async {
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment