Skip to content
This repository was archived by the owner on Feb 22, 2023. It is now read-only.
Merged
Next Next commit
Update tool to set macOS deployment target to 10.15.
This change is necessary for #6517.
  • Loading branch information
IVLIVS-III committed Oct 26, 2022
commit ee0aebe2f33c234d0ec9dfd55f36148822d3b27e
82 changes: 81 additions & 1 deletion script/tool/lib/src/create_all_plugins_app_command.dart
Original file line number Diff line number Diff line change
Expand Up @@ -61,10 +61,19 @@ class CreateAllPluginsAppCommand extends PackageCommand {
print('');
}

await _genPubspecWithAllPlugins();

/// Run `flutter pub get` to generate all native build files for macOS.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: Remove the "for macOS" here since it's all platform (even if we only happen to be using it for macOS right now).

final int genNativeBuildFilesExitCode = await _genNativeBuildFiles();
if (genNativeBuildFilesExitCode != 0) {
throw ToolExit(genNativeBuildFilesExitCode);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should instead printError a message (e.g., "Failed to generate native build files"), then throw a new defined error like the ones you added above. We don't actually need the underlying exit code.

}

await Future.wait(<Future<void>>[
_genPubspecWithAllPlugins(),
_updateAppGradle(),
_updateManifest(),
_updateMacosPodfile(),
_updateMacosPbxproj(),
]);
}

Expand Down Expand Up @@ -259,4 +268,75 @@ dev_dependencies:${_pubspecMapString(pubspec.devDependencies)}

return buffer.toString();
}

Future<int> _genNativeBuildFiles() async {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be a boolean.

// Only run on macOS.
// Other platforms don't need generation of additional files.
if (!io.Platform.isMacOS) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We may need this for other platforms in the future, and pub get will be part of building the app anyway, so we can just do this unconditionally and keep the codepaths the same on all platforms.

return 0;
}

final io.ProcessResult result = io.Process.runSync(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can simplify this method by using processRunner.runAndStream, which will automatically print all of the output.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh neat, didn't know this existed.

flutterCommand,
<String>[
'pub',
'get',
],
workingDirectory: _appDirectory.path,
);

print(result.stdout);
print(result.stderr);
return result.exitCode;
}

Future<void> _updateMacosPodfile() async {
// Only change the macOS deployment target if the host platform is macOS.
if (!io.Platform.isMacOS) {
return;
}

final File podfileFile =
app.platformDirectory(FlutterPlatform.macos).childFile('Podfile');
if (!podfileFile.existsSync()) {
throw ToolExit(64);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder why 64 is the ToolExit code. Could that be documented here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Initially it was suggested to me, to look at the implementation of _updateAppGradle to see how changes to project configuration files could be made (see this comment).

This would be done in script/tool/lib/src/create_all_plugins_app_command.dart. See _updateAppGradle for an example of making a similar change to some Android values.

There, this (to me arbitrary) exit code 64 was thrown in case the relevant android config file didn't exist.
I don't know the meaning of this value either. Looking back at git history, the value was introduced two years ago in #3544.
The value itself seems to date back even further tho, since the PR mentioned above only moved some files around.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This does look it was arbitrary. The way we're handling error codes for any current code is to define file-level constants starting with 3 (since 1 and 2 are global errors) for each failure, and using those, so that it's self-documenting. Let's do that here; it's fine that it's inconsistent with the older code in the file since those values aren't meaningful anyway.

}

final StringBuffer newPodfile = StringBuffer();
for (final String line in podfileFile.readAsLinesSync()) {
if (line.contains('platform :osx')) {
// macOS 10.15 is required by in_app_purchase.
newPodfile.writeln("platform :osx, '10.15'");
} else {
newPodfile.writeln(line);
}
}
podfileFile.writeAsStringSync(newPodfile.toString());
}

Future<void> _updateMacosPbxproj() async {
// Only change the macOS deployment target if the host platform is macOS.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same; this can be unconditional.

if (!io.Platform.isMacOS) {
return;
}

final File pbxprojFile = app
.platformDirectory(FlutterPlatform.macos)
.childDirectory('Runner.xcodeproj')
.childFile('project.pbxproj');
if (!pbxprojFile.existsSync()) {
throw ToolExit(64);
}

final StringBuffer newPbxproj = StringBuffer();
for (final String line in pbxprojFile.readAsLinesSync()) {
if (line.contains('MACOSX_DEPLOYMENT_TARGET')) {
// macOS 10.15 is required by in_app_purchase.
newPbxproj.writeln(' MACOSX_DEPLOYMENT_TARGET = 10.15;');
} else {
newPbxproj.writeln(line);
}
}
pbxprojFile.writeAsStringSync(newPbxproj.toString());
}
}