fix: remove custom export options plist as it no longer seems to be needed (#2620)

This commit is contained in:
Bryan Oltman
2024-11-14 14:07:07 -05:00
committed by GitHub
parent 0533806458
commit 0c74e93d0f
12 changed files with 74 additions and 396 deletions
@@ -259,7 +259,6 @@ class ArtifactBuilder {
/// an .xcarchive and _not_ an .ipa.
Future<IpaBuildResult> buildIpa({
bool codesign = true,
File? exportOptionsPlist,
String? flavor,
String? target,
List<String> args = const [],
@@ -268,8 +267,6 @@ class ArtifactBuilder {
String? appDillPath;
await _runShorebirdBuildCommand(() async {
const executable = 'flutter';
final exportOptionsPlistPath =
(exportOptionsPlist ?? ios.createExportOptionsPlist()).path;
final arguments = [
'build',
'ipa',
@@ -277,7 +274,6 @@ class ArtifactBuilder {
if (flavor != null) '--flavor=$flavor',
if (target != null) '--target=$target',
if (!codesign) '--no-codesign',
if (codesign) '''--export-options-plist=$exportOptionsPlistPath''',
...args,
];
@@ -156,14 +156,6 @@ This may indicate that the patch contains native changes, which cannot be applie
@override
Future<File> buildPatchArtifact({String? releaseVersion}) async {
final File exportOptionsPlist;
try {
exportOptionsPlist = ios.exportOptionsPlistFromArgs(argResults);
} catch (error) {
logger.err('$error');
throw ProcessExit(ExitCode.usage.code);
}
try {
final shouldCodesign = argResults['codesign'] == true;
final (flutterVersionAndRevision, flutterVersion) = await (
@@ -190,7 +182,6 @@ For more information see: ${supportedFlutterVersionsUrl.toLink()}''',
// release was, we will erroneously report native diffs.
ipaBuildResult = await artifactBuilder.buildIpa(
codesign: shouldCodesign,
exportOptionsPlist: exportOptionsPlist,
flavor: flavor,
target: target,
args: argResults.forwardedArgs +
@@ -105,7 +105,6 @@ of the iOS app that is using this module.''',
)
..addOption(
CommonArguments.exportMethodArg.name,
defaultsTo: ExportMethod.appStore.argName,
allowed: ExportMethod.values.map((e) => e.argName),
help: CommonArguments.exportMethodArg.description,
allowedHelp: {
@@ -106,14 +106,6 @@ For more information see: ${supportedFlutterVersionsUrl.toLink()}''',
);
}
final File exportOptionsPlist;
try {
exportOptionsPlist = ios.exportOptionsPlistFromArgs(argResults);
} catch (error) {
logger.err('$error');
throw ProcessExit(ExitCode.usage.code);
}
final flutterVersionString = await shorebirdFlutter.getVersionAndRevision();
final buildProgress =
logger.progress('Building ipa with Flutter $flutterVersionString');
@@ -121,7 +113,6 @@ For more information see: ${supportedFlutterVersionsUrl.toLink()}''',
try {
await artifactBuilder.buildIpa(
codesign: codesign,
exportOptionsPlist: exportOptionsPlist,
flavor: flavor,
target: target,
args: argResults.forwardedArgs,
@@ -78,7 +78,6 @@ class ReleaseCommand extends ShorebirdCommand {
)
..addOption(
CommonArguments.exportMethodArg.name,
defaultsTo: ExportMethod.appStore.argName,
allowed: ExportMethod.values.map((e) => e.argName),
help: CommonArguments.exportMethodArg.description,
allowedHelp: {
@@ -138,6 +138,8 @@ extension ForwardedArgs on ArgResults {
..._argsNamed(CommonArguments.buildNameArg.name),
..._argsNamed(CommonArguments.buildNumberArg.name),
..._argsNamed(CommonArguments.splitDebugInfoArg.name),
..._argsNamed(CommonArguments.exportMethodArg.name),
..._argsNamed(CommonArguments.exportOptionsPlistArg.name),
],
);
@@ -2,13 +2,10 @@
import 'dart:io';
import 'package:args/args.dart';
import 'package:collection/collection.dart';
import 'package:path/path.dart' as p;
import 'package:pub_semver/pub_semver.dart';
import 'package:scoped_deps/scoped_deps.dart';
import 'package:shorebird_cli/src/archive_analysis/archive_analysis.dart';
import 'package:shorebird_cli/src/common_arguments.dart';
import 'package:shorebird_cli/src/shorebird_env.dart';
import 'package:xml/xml.dart';
@@ -31,7 +28,8 @@ To add iOS, run "flutter create . --platforms ios"''';
}
/// {@template export_method}
/// The method used to export the IPA.
/// The method used to export the IPA. This is passed to the Flutter tool.
/// Acceptable values can be found by running `flutter build ipa -h`.
/// {@endtemplate}
enum ExportMethod {
appStore('app-store', 'Upload to the App Store'),
@@ -84,43 +82,6 @@ final iosRef = create(Ios.new);
Ios get ios => read(iosRef);
class Ios {
File exportOptionsPlistFromArgs(ArgResults results) {
final exportPlistArg =
results[CommonArguments.exportOptionsPlistArg.name] as String?;
final exportMethodArgExists =
results.options.contains(CommonArguments.exportMethodArg.name);
if (exportPlistArg != null &&
exportMethodArgExists &&
results.wasParsed(CommonArguments.exportMethodArg.name)) {
throw ArgumentError(
'''Cannot specify both --${CommonArguments.exportMethodArg.name} and --${CommonArguments.exportOptionsPlistArg.name}.''',
);
}
final File? exportOptionsPlist;
if (exportPlistArg != null) {
exportOptionsPlist = File(exportPlistArg);
_validateExportOptionsPlist(exportOptionsPlist);
return exportOptionsPlist;
}
final ExportMethod? exportMethod;
if (exportMethodArgExists &&
results.wasParsed(CommonArguments.exportMethodArg.name)) {
exportMethod = ExportMethod.values.firstWhere(
(element) =>
element.argName ==
results[CommonArguments.exportMethodArg.name] as String,
);
} else {
exportMethod = null;
}
return createExportOptionsPlist(
exportMethod: exportMethod ?? ExportMethod.appStore,
);
}
/// Returns the set of flavors for the iOS project, if the project has an
/// iOS platform configured.
Set<String>? flavors() {
@@ -181,60 +142,4 @@ class Ios {
(e) => e.localName == 'wasCreatedForAppExtension' && e.value == 'YES',
);
}
/// Creates an ExportOptions.plist file, which is used to tell xcodebuild to
/// not manage the app version and build number. If we don't do this, then
/// xcodebuild will increment the build number if it detects an App Store
/// Connect build with the same version and build number. This is a problem
/// for us when patching, as patches need to have the same version and build
/// number as the release they are patching.
/// See
/// https://developer.apple.com/forums/thread/690647?answerId=689925022#689925022
File createExportOptionsPlist({
ExportMethod? exportMethod,
}) {
exportMethod ??= ExportMethod.appStore;
final plistContents = '''
<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
<plist version="1.0">
<dict>
<key>manageAppVersionAndBuildNumber</key>
<false/>
<key>signingStyle</key>
<string>automatic</string>
<key>uploadBitcode</key>
<false/>
<key>method</key>
<string>${exportMethod.argName}</string>
</dict>
</plist>
''';
final tempDir = Directory.systemTemp.createTempSync();
final exportPlistFile = File(p.join(tempDir.path, 'ExportOptions.plist'))
..createSync(recursive: true)
..writeAsStringSync(plistContents);
return exportPlistFile;
}
/// Verifies that [exportOptionsPlistFile] exists and sets
/// manageAppVersionAndBuildNumber to false, which prevents Xcode from
/// changing the version number out from under us.
///
/// Throws an exception if validation fails, exits normally if validation
/// succeeds.
void _validateExportOptionsPlist(File exportOptionsPlistFile) {
if (!exportOptionsPlistFile.existsSync()) {
throw FileSystemException(
'''Export options plist file ${exportOptionsPlistFile.path} does not exist''',
);
}
final plist = Plist(file: exportOptionsPlistFile);
if (plist.properties['manageAppVersionAndBuildNumber'] != false) {
throw InvalidExportOptionsPlistException(
'''Export options plist ${exportOptionsPlistFile.path} does not set manageAppVersionAndBuildNumber to false. This is required for shorebird to work.''',
);
}
}
}
@@ -3,7 +3,6 @@ import 'dart:io';
import 'package:mason_logger/mason_logger.dart';
import 'package:mocktail/mocktail.dart';
import 'package:path/path.dart' as p;
import 'package:scoped_deps/scoped_deps.dart';
import 'package:shorebird_cli/src/artifact_builder.dart';
import 'package:shorebird_cli/src/logging/logging.dart';
@@ -710,15 +709,6 @@ Either run `flutter pub get` manually, or follow the steps in ${cannotRunInVSCod
group(
'buildIpa',
() {
late File exportOptionsPlist;
setUp(() {
final tempDir = Directory.systemTemp.createTempSync();
exportOptionsPlist =
File(p.join(tempDir.path, 'exportoptions.plist'));
when(ios.createExportOptionsPlist).thenReturn(exportOptionsPlist);
});
group('with default arguments', () {
test('invokes flutter build with an export options plist', () async {
final result = await runWithOverrides(builder.buildIpa);
@@ -730,7 +720,6 @@ Either run `flutter pub get` manually, or follow the steps in ${cannotRunInVSCod
'build',
'ipa',
'--release',
'--export-options-plist=${exportOptionsPlist.path}',
],
runInShell: true,
environment: any(named: 'environment'),
@@ -751,7 +740,6 @@ Either run `flutter pub get` manually, or follow the steps in ${cannotRunInVSCod
'build',
'ipa',
'--release',
'--export-options-plist=${exportOptionsPlist.path}',
],
runInShell: any(named: 'runInShell'),
environment: {
@@ -775,7 +763,6 @@ Either run `flutter pub get` manually, or follow the steps in ${cannotRunInVSCod
'build',
'ipa',
'--release',
'--export-options-plist=${exportOptionsPlist.path}',
],
runInShell: any(named: 'runInShell'),
environment: {
@@ -786,58 +773,10 @@ Either run `flutter pub get` manually, or follow the steps in ${cannotRunInVSCod
});
});
group('when export options plist is provided', () {
test('forwards to flutter build', () async {
await runWithOverrides(
() => builder.buildIpa(
exportOptionsPlist: File('custom_exportoptions.plist'),
),
);
verify(
() => shorebirdProcess.run(
'flutter',
[
'build',
'ipa',
'--release',
'--export-options-plist=custom_exportoptions.plist',
],
runInShell: any(named: 'runInShell'),
),
).called(1);
});
});
test('does not provide export options plist without codesigning',
() async {
await runWithOverrides(
() => builder.buildIpa(
codesign: false,
exportOptionsPlist: File('exportOptionsPlist.plist'),
),
);
verify(
() => shorebirdProcess.run(
'flutter',
[
'build',
'ipa',
'--release',
'--no-codesign',
],
runInShell: any(named: 'runInShell'),
environment: any(named: 'environment'),
),
).called(1);
});
test('forwards extra arguments to flutter build', () async {
await runWithOverrides(
() => builder.buildIpa(
codesign: false,
exportOptionsPlist: File('exportOptionsPlist.plist'),
flavor: 'flavor',
target: 'target.dart',
args: ['--foo', 'bar'],
@@ -142,8 +142,6 @@ void main() {
() => shorebirdEnv.getShorebirdProjectRoot(),
).thenReturn(projectRoot);
when(() => ios.exportOptionsPlistFromArgs(any())).thenReturn(File(''));
when(aotTools.isLinkDebugInfoSupported).thenAnswer((_) async => false);
patcher = IosPatcher(
@@ -533,26 +531,10 @@ For more information see: ${supportedFlutterVersionsUrl.toLink()}''',
});
});
group('when exportOptionsPlist fails', () {
setUp(() {
when(() => ios.exportOptionsPlistFromArgs(any())).thenThrow(
const FileSystemException('error'),
);
});
test('logs error and exits with code 70', () async {
await expectLater(
() => runWithOverrides(patcher.buildPatchArtifact),
exitsWithCode(ExitCode.usage),
);
});
});
group('when build fails with ProcessException', () {
setUp(() {
when(
() => artifactBuilder.buildIpa(
exportOptionsPlist: any(named: 'exportOptionsPlist'),
codesign: any(named: 'codesign'),
args: any(named: 'args'),
flavor: any(named: 'flavor'),
@@ -581,7 +563,6 @@ For more information see: ${supportedFlutterVersionsUrl.toLink()}''',
setUp(() {
when(
() => artifactBuilder.buildIpa(
exportOptionsPlist: any(named: 'exportOptionsPlist'),
codesign: any(named: 'codesign'),
args: any(named: 'args'),
flavor: any(named: 'flavor'),
@@ -606,7 +587,6 @@ For more information see: ${supportedFlutterVersionsUrl.toLink()}''',
setUp(() {
when(
() => artifactBuilder.buildIpa(
exportOptionsPlist: any(named: 'exportOptionsPlist'),
codesign: any(named: 'codesign'),
args: any(named: 'args'),
flavor: any(named: 'flavor'),
@@ -644,7 +624,6 @@ For more information see: ${supportedFlutterVersionsUrl.toLink()}''',
)..createSync(recursive: true);
when(
() => artifactBuilder.buildIpa(
exportOptionsPlist: any(named: 'exportOptionsPlist'),
codesign: any(named: 'codesign'),
args: any(named: 'args'),
flavor: any(named: 'flavor'),
@@ -723,7 +702,6 @@ For more information see: ${supportedFlutterVersionsUrl.toLink()}''',
verify(
() => artifactBuilder.buildIpa(
flavor: any(named: 'flavor'),
exportOptionsPlist: any(named: 'exportOptionsPlist'),
codesign: any(named: 'codesign'),
target: any(named: 'target'),
args: any(
@@ -749,7 +727,6 @@ For more information see: ${supportedFlutterVersionsUrl.toLink()}''',
expect(p.basename(artifact.path), endsWith('.zip'));
verify(
() => artifactBuilder.buildIpa(
exportOptionsPlist: any(named: 'exportOptionsPlist'),
codesign: any(named: 'codesign'),
args: ['--verbose'],
),
@@ -782,7 +759,6 @@ For more information see: ${supportedFlutterVersionsUrl.toLink()}''',
verify(
() => artifactBuilder.buildIpa(
exportOptionsPlist: any(named: 'exportOptionsPlist'),
codesign: any(named: 'codesign'),
args: any(named: 'args'),
flavor: any(named: 'flavor'),
@@ -332,7 +332,6 @@ To change the version of this release, change your app's version in your pubspec
when(
() => artifactBuilder.buildIpa(
codesign: any(named: 'codesign'),
exportOptionsPlist: any(named: 'exportOptionsPlist'),
flavor: any(named: 'flavor'),
target: any(named: 'target'),
args: any(named: 'args'),
@@ -354,10 +353,6 @@ To change the version of this release, change your app's version in your pubspec
when(() => codeSigner.base64PublicKey(any()))
.thenReturn(base64PublicKey);
when(
() => ios.exportOptionsPlistFromArgs(argResults),
).thenReturn(File(''));
when(() => shorebirdEnv.getShorebirdProjectRoot())
.thenReturn(projectRoot);
when(
@@ -379,7 +374,6 @@ To change the version of this release, change your app's version in your pubspec
when(
() => artifactBuilder.buildIpa(
codesign: any(named: 'codesign'),
exportOptionsPlist: any(named: 'exportOptionsPlist'),
flavor: any(named: 'flavor'),
target: any(named: 'target'),
args: any(named: 'args'),
@@ -402,7 +396,6 @@ To change the version of this release, change your app's version in your pubspec
verify(
() => artifactBuilder.buildIpa(
codesign: any(named: 'codesign'),
exportOptionsPlist: any(named: 'exportOptionsPlist'),
flavor: any(named: 'flavor'),
target: any(named: 'target'),
args: any(named: 'args'),
@@ -434,32 +427,11 @@ To change the version of this release, change your app's version in your pubspec
});
});
group('when export options plist fails to generate', () {
const error = 'error';
setUp(() {
when(
() => ios.exportOptionsPlistFromArgs(argResults),
).thenThrow(error);
});
test('logs error and exits with code 64', () async {
await expectLater(
() => runWithOverrides(iosReleaser.buildReleaseArtifacts),
exitsWithCode(ExitCode.usage),
);
verify(
() => logger.err(error),
).called(1);
});
});
group('when build fails', () {
setUp(() {
when(
() => artifactBuilder.buildIpa(
codesign: any(named: 'codesign'),
exportOptionsPlist: any(named: 'exportOptionsPlist'),
flavor: any(named: 'flavor'),
target: any(named: 'target'),
args: any(named: 'args'),
@@ -501,7 +473,6 @@ To change the version of this release, change your app's version in your pubspec
).called(1);
verify(
() => artifactBuilder.buildIpa(
exportOptionsPlist: any(named: 'exportOptionsPlist'),
args: ['--verbose'],
),
).called(1);
@@ -111,6 +111,14 @@ void main() {
CommonArguments.splitDebugInfoArg.name,
help: CommonArguments.splitDebugInfoArg.description,
)
..addOption(
CommonArguments.exportMethodArg.name,
help: CommonArguments.exportMethodArg.description,
)
..addOption(
CommonArguments.exportOptionsPlistArg.name,
help: CommonArguments.exportOptionsPlistArg.description,
)
..addMultiOption(
'platforms',
allowed: ReleaseType.values.map((e) => e.cliName),
@@ -265,5 +273,67 @@ void main() {
);
});
});
group('when export method is provided before the --', () {
test('forwards it', () {
final args = [
'--verbose',
'--export-method=development',
];
final result = parser.parse(args);
expect(result.forwardedArgs, hasLength(1));
expect(
result.forwardedArgs,
contains('--export-method=development'),
);
});
});
group('when export method is provided after the --', () {
test('forwards it', () {
final args = [
'--verbose',
'--',
'--export-method=development',
];
final result = parser.parse(args);
expect(result.forwardedArgs, hasLength(1));
expect(
result.forwardedArgs,
contains('--export-method=development'),
);
});
});
group('when export options plist is provided before the --', () {
test('forwards it', () {
final args = [
'--verbose',
'--export-options-plist=build/ExportOptions.plist',
];
final result = parser.parse(args);
expect(result.forwardedArgs, hasLength(1));
expect(
result.forwardedArgs,
contains('--export-options-plist=build/ExportOptions.plist'),
);
});
});
group('when export options plist is provided after the --', () {
test('forwards it', () {
final args = [
'--verbose',
'--',
'--export-options-plist=build/ExportOptions.plist',
];
final result = parser.parse(args);
expect(result.forwardedArgs, hasLength(1));
expect(
result.forwardedArgs,
contains('--export-options-plist=build/ExportOptions.plist'),
);
});
});
});
}
@@ -1,12 +1,8 @@
import 'dart:io';
import 'package:args/args.dart';
import 'package:mocktail/mocktail.dart';
import 'package:path/path.dart' as p;
import 'package:propertylistserialization/propertylistserialization.dart';
import 'package:scoped_deps/scoped_deps.dart';
import 'package:shorebird_cli/src/archive_analysis/archive_analysis.dart';
import 'package:shorebird_cli/src/common_arguments.dart';
import 'package:shorebird_cli/src/platform/platform.dart';
import 'package:shorebird_cli/src/shorebird_env.dart';
import 'package:test/test.dart';
@@ -51,163 +47,6 @@ To add iOS, run "flutter create . --platforms ios"''',
});
});
group('exportOptionsPlistFromArgs', () {
late ArgResults argResults;
setUp(() {
argResults = MockArgResults();
when(() => argResults.wasParsed(any())).thenReturn(false);
when(() => argResults.options).thenReturn([
CommonArguments.exportMethodArg.name,
CommonArguments.exportOptionsPlistArg.name,
]);
});
group('when both export-method and export-options-plist are provided',
() {
setUp(() {
when(
() => argResults.wasParsed(CommonArguments.exportMethodArg.name),
).thenReturn(true);
when(
() => argResults[CommonArguments.exportOptionsPlistArg.name],
).thenReturn('/path/to/export.plist');
});
test('throws ArgumentError', () {
expect(
() => ios.exportOptionsPlistFromArgs(argResults),
throwsArgumentError,
);
});
});
group('when export-method is provided', () {
setUp(() {
when(() => argResults.wasParsed(CommonArguments.exportMethodArg.name))
.thenReturn(true);
when(() => argResults[CommonArguments.exportMethodArg.name])
.thenReturn(ExportMethod.adHoc.argName);
when(() => argResults[CommonArguments.exportOptionsPlistArg.name])
.thenReturn(null);
});
test('generates an export options plist with that export method',
() async {
final exportOptionsPlistFile = ios.exportOptionsPlistFromArgs(
argResults,
);
final exportOptionsPlist = Plist(file: exportOptionsPlistFile);
expect(
exportOptionsPlist.properties['method'],
ExportMethod.adHoc.argName,
);
});
});
group('when export-options-plist is provided', () {
group('when file does not exist', () {
setUp(() {
when(
() => argResults[CommonArguments.exportOptionsPlistArg.name],
).thenReturn('/does/not/exist');
});
test('throws a FileSystemException', () async {
expect(
() => ios.exportOptionsPlistFromArgs(argResults),
throwsA(
isA<FileSystemException>().having(
(e) => e.message,
'message',
'''Export options plist file /does/not/exist does not exist''',
),
),
);
});
});
group('when manageAppVersionAndBuildNumber is not set to false', () {
const exportPlistContent = '''
<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
<plist version="1.0">
<dict>
</dict>
</plist>
''';
test('throws InvalidExportOptionsPlistException', () async {
final tmpDir = Directory.systemTemp.createTempSync();
final exportPlistFile = File(
p.join(tmpDir.path, 'export.plist'),
)..writeAsStringSync(exportPlistContent);
when(
() => argResults[CommonArguments.exportOptionsPlistArg.name],
).thenReturn(exportPlistFile.path);
expect(
() => ios.exportOptionsPlistFromArgs(argResults),
throwsA(
isA<InvalidExportOptionsPlistException>().having(
(e) => e.message,
'message',
'''Export options plist ${exportPlistFile.path} does not set manageAppVersionAndBuildNumber to false. This is required for shorebird to work.''',
),
),
);
});
});
});
group('when neither export-method nor export-options-plist is provided',
() {
setUp(() {
when(() => argResults.wasParsed(CommonArguments.exportMethodArg.name))
.thenReturn(false);
when(() => argResults[CommonArguments.exportOptionsPlistArg.name])
.thenReturn(null);
});
test('generates an export options plist with app-store export method',
() async {
final exportOptionsPlistFile =
ios.exportOptionsPlistFromArgs(argResults);
final exportOptionsPlist = Plist(file: exportOptionsPlistFile);
expect(
exportOptionsPlist.properties['method'],
ExportMethod.appStore.argName,
);
final exportOptionsPlistMap =
PropertyListSerialization.propertyListWithString(
exportOptionsPlistFile.readAsStringSync(),
) as Map<String, Object>;
expect(
exportOptionsPlistMap['manageAppVersionAndBuildNumber'],
isFalse,
);
expect(exportOptionsPlistMap['signingStyle'], 'automatic');
expect(exportOptionsPlistMap['uploadBitcode'], isFalse);
expect(exportOptionsPlistMap['method'], 'app-store');
});
});
group('when export-method option does not exist', () {
setUp(() {
when(() => argResults.options)
.thenReturn([CommonArguments.exportOptionsPlistArg.name]);
});
test('does not check whether export-method was parsed', () {
ios.exportOptionsPlistFromArgs(argResults);
verifyNever(
() => argResults.wasParsed(CommonArguments.exportMethodArg.name),
);
});
});
});
group('flavors', () {
final schemesPath = p.join(
'ios',