Unverified Commit b82cf76f authored by Jenn Magder's avatar Jenn Magder Committed by GitHub

Return void from project migrate() (#112897)

parent 69fc4fb6
......@@ -16,8 +16,7 @@ abstract class ProjectMigrator {
@protected
final Logger logger;
/// Returns whether migration was successful or was skipped.
bool migrate();
void migrate();
/// Return null if the line should be deleted.
@protected
......@@ -80,15 +79,9 @@ class ProjectMigration {
final List<ProjectMigrator> migrators;
bool run() {
void run() {
for (final ProjectMigrator migrator in migrators) {
if (!migrator.migrate()) {
// Migration failures should be more robust, with transactions and fallbacks.
// See https://github.com/flutter/flutter/issues/12573 and
// https://github.com/flutter/flutter/issues/40460
return false;
}
migrator.migrate();
}
return true;
}
}
......@@ -133,9 +133,7 @@ Future<XcodeBuildResult> buildXcodeProject({
];
final ProjectMigration migration = ProjectMigration(migrators);
if (!migration.run()) {
return XcodeBuildResult(success: false);
}
migration.run();
if (!_checkXcodeVersion()) {
return XcodeBuildResult(success: false);
......
......@@ -19,14 +19,13 @@ class HostAppInfoPlistMigration extends ProjectMigrator {
final File _infoPlist;
@override
bool migrate() {
void migrate() {
if (!_infoPlist.existsSync()) {
logger.printTrace('Info.plist not found, skipping host app Info.plist migration.');
return true;
return;
}
processFileLines(_infoPlist);
return true;
}
@override
......
......@@ -20,7 +20,7 @@ class IOSDeploymentTargetMigration extends ProjectMigrator {
final File _appFrameworkInfoPlist;
@override
bool migrate() {
void migrate() {
if (_xcodeProjectInfoFile.existsSync()) {
processFileLines(_xcodeProjectInfoFile);
} else {
......@@ -38,8 +38,6 @@ class IOSDeploymentTargetMigration extends ProjectMigrator {
} else {
logger.printTrace('Podfile not found, skipping global platform iOS version migration.');
}
return true;
}
@override
......
......@@ -16,10 +16,10 @@ class ProjectBaseConfigurationMigration extends ProjectMigrator {
final File _xcodeProjectInfoFile;
@override
bool migrate() {
void migrate() {
if (!_xcodeProjectInfoFile.existsSync()) {
logger.printTrace('Xcode project not found, skipping Runner project build settings and configuration migration');
return true;
return;
}
final String originalProjectContents = _xcodeProjectInfoFile.readAsStringSync();
......@@ -84,6 +84,5 @@ class ProjectBaseConfigurationMigration extends ProjectMigrator {
logger.printStatus('Project base configurations detected, removing.');
_xcodeProjectInfoFile.writeAsStringSync(newProjectContents);
}
return true;
}
}
......@@ -16,14 +16,13 @@ class ProjectBuildLocationMigration extends ProjectMigrator {
final File _xcodeProjectWorkspaceData;
@override
bool migrate() {
void migrate() {
if (!_xcodeProjectWorkspaceData.existsSync()) {
logger.printTrace('Xcode project workspace data not found, skipping build location migration.');
return true;
return;
}
processFileLines(_xcodeProjectWorkspaceData);
return true;
}
@override
......
......@@ -23,15 +23,13 @@ class RemoveFrameworkLinkAndEmbeddingMigration extends ProjectMigrator {
final Usage _usage;
@override
bool migrate() {
void migrate() {
if (!_xcodeProjectInfoFile.existsSync()) {
logger.printTrace('Xcode project not found, skipping framework link and embedding migration');
return true;
return;
}
processFileLines(_xcodeProjectInfoFile);
return true;
}
@override
......
......@@ -18,10 +18,10 @@ class XcodeBuildSystemMigration extends ProjectMigrator {
final File _xcodeWorkspaceSharedSettings;
@override
bool migrate() {
void migrate() {
if (!_xcodeWorkspaceSharedSettings.existsSync()) {
logger.printTrace('Xcode workspace settings not found, skipping build system migration');
return true;
return;
}
final String contents = _xcodeWorkspaceSharedSettings.readAsStringSync();
......@@ -36,7 +36,5 @@ class XcodeBuildSystemMigration extends ProjectMigrator {
logger.printStatus('Legacy build system detected, removing ${_xcodeWorkspaceSharedSettings.path}');
_xcodeWorkspaceSharedSettings.deleteSync();
}
return true;
}
}
......@@ -47,9 +47,7 @@ Future<void> buildLinux(
];
final ProjectMigration migration = ProjectMigration(migrators);
if (!migration.run()) {
throwToolExit('Unable to migrate project files');
}
migration.run();
// Build the environment that needs to be set for the re-entrant flutter build
// step.
......
......@@ -54,9 +54,7 @@ Future<void> buildMacOS({
];
final ProjectMigration migration = ProjectMigration(migrators);
if (!migration.run()) {
throwToolExit('Could not migrate project file');
}
migration.run();
final Directory flutterBuildDir = globals.fs.directory(getMacOSBuildDirectory());
if (!flutterBuildDir.existsSync()) {
......
......@@ -18,7 +18,7 @@ class MacOSDeploymentTargetMigration extends ProjectMigrator {
final File _podfile;
@override
bool migrate() {
void migrate() {
if (_xcodeProjectInfoFile.existsSync()) {
processFileLines(_xcodeProjectInfoFile);
} else {
......@@ -30,8 +30,6 @@ class MacOSDeploymentTargetMigration extends ProjectMigrator {
} else {
logger.printTrace('Podfile not found, skipping global platform macOS version migration.');
}
return true;
}
@override
......
......@@ -21,16 +21,14 @@ class RemoveMacOSFrameworkLinkAndEmbeddingMigration extends ProjectMigrator {
final Usage _usage;
@override
bool migrate() {
void migrate() {
if (!_xcodeProjectInfoFile.existsSync()) {
logger.printTrace(
'Xcode project not found, skipping framework link and embedding migration');
return true;
return;
}
processFileLines(_xcodeProjectInfoFile);
return true;
}
@override
......
......@@ -16,10 +16,10 @@ class CmakeCustomCommandMigration extends ProjectMigrator {
final File _cmakeFile;
@override
bool migrate() {
void migrate() {
if (!_cmakeFile.existsSync()) {
logger.printTrace('CMake project not found, skipping add_custom_command() VERBATIM migration');
return true;
return;
}
final String originalProjectContents = _cmakeFile.readAsStringSync();
......@@ -68,6 +68,5 @@ class CmakeCustomCommandMigration extends ProjectMigrator {
logger.printStatus('add_custom_command() missing VERBATIM or FLUTTER_TARGET_PLATFORM, updating.');
_cmakeFile.writeAsStringSync(newProjectContents);
}
return true;
}
}
......@@ -18,7 +18,7 @@ class XcodeProjectObjectVersionMigration extends ProjectMigrator {
final File _xcodeProjectSchemeFile;
@override
bool migrate() {
void migrate() {
if (_xcodeProjectInfoFile.existsSync()) {
processFileLines(_xcodeProjectInfoFile);
} else {
......@@ -29,8 +29,6 @@ class XcodeProjectObjectVersionMigration extends ProjectMigrator {
} else {
logger.printTrace('Runner scheme not found, skipping Xcode compatibility migration.');
}
return true;
}
@override
......
......@@ -15,10 +15,10 @@ class XcodeScriptBuildPhaseMigration extends ProjectMigrator {
final File _xcodeProjectInfoFile;
@override
bool migrate() {
void migrate() {
if (!_xcodeProjectInfoFile.existsSync()) {
logger.printTrace('Xcode project not found, skipping script build phase dependency analysis removal.');
return true;
return;
}
final String originalProjectContents = _xcodeProjectInfoFile.readAsStringSync();
......@@ -56,6 +56,5 @@ class XcodeScriptBuildPhaseMigration extends ProjectMigrator {
logger.printStatus('Removing script build phase dependency analysis.');
_xcodeProjectInfoFile.writeAsStringSync(newProjectContents);
}
return true;
}
}
......@@ -40,9 +40,7 @@ Future<void> buildWeb(
];
final ProjectMigration migration = ProjectMigration(migrators);
if (!migration.run()) {
throwToolExit('Failed to run all web migrations.');
}
migration.run();
final Status status = globals.logger.startProgress('Compiling $target for the Web...');
final Stopwatch sw = Stopwatch()..start();
......
......@@ -18,17 +18,16 @@ class ScrubGeneratedPluginRegistrant extends ProjectMigrator {
final Logger _logger;
@override
bool migrate() {
void migrate() {
final File registrant = _project.libDirectory.childFile('generated_plugin_registrant.dart');
final File gitignore = _project.parent.directory.childFile('.gitignore');
if (!removeFile(registrant)) {
return false;
return;
}
if (gitignore.existsSync()) {
processFileLines(gitignore);
}
return true;
}
// Cleans up the .gitignore by removing the line that mentions generated_plugin_registrant.
......
......@@ -54,9 +54,7 @@ Future<void> buildWindows(WindowsProject windowsProject, BuildInfo buildInfo, {
];
final ProjectMigration migration = ProjectMigration(migrators);
if (!migration.run()) {
throwToolExit('Unable to migrate project files');
}
migration.run();
// Ensure that necessary ephemeral files are generated and up to date.
_writeGeneratedFlutterConfig(windowsProject, buildInfo, target);
......
......@@ -30,15 +30,9 @@ void main () {
});
testWithoutContext('migrators succeed', () {
final FakeIOSMigrator fakeIOSMigrator = FakeIOSMigrator(succeeds: true);
final FakeIOSMigrator fakeIOSMigrator = FakeIOSMigrator();
final ProjectMigration migration = ProjectMigration(<ProjectMigrator>[fakeIOSMigrator]);
expect(migration.run(), isTrue);
});
testWithoutContext('migrators fail', () {
final FakeIOSMigrator fakeIOSMigrator = FakeIOSMigrator(succeeds: false);
final ProjectMigration migration = ProjectMigration(<ProjectMigrator>[fakeIOSMigrator]);
expect(migration.run(), isFalse);
migration.run();
});
group('remove framework linking and embedding migration', () {
......@@ -61,7 +55,7 @@ void main () {
testLogger,
testUsage
);
expect(iosProjectMigration.migrate(), isTrue);
iosProjectMigration.migrate();
expect(testUsage.events, isEmpty);
expect(xcodeProjectInfoFile.existsSync(), isFalse);
......@@ -80,7 +74,7 @@ void main () {
testLogger,
testUsage,
);
expect(iosProjectMigration.migrate(), isTrue);
iosProjectMigration.migrate();
expect(testUsage.events, isEmpty);
expect(xcodeProjectInfoFile.lastModifiedSync(), projectLastModified);
......@@ -100,7 +94,7 @@ shellScript = "/bin/sh \"$FLUTTER_ROOT/packages/flutter_tools/bin/xcode_backend.
testLogger,
testUsage,
);
expect(iosProjectMigration.migrate(), isTrue);
iosProjectMigration.migrate();
expect(xcodeProjectInfoFile.readAsStringSync(), contents);
expect(testLogger.statusText, isEmpty);
});
......@@ -127,7 +121,7 @@ keep this 2
testLogger,
testUsage,
);
expect(iosProjectMigration.migrate(), isTrue);
iosProjectMigration.migrate();
expect(testUsage.events, isEmpty);
expect(xcodeProjectInfoFile.readAsStringSync(), r'''
......@@ -207,7 +201,7 @@ keep this 2
project,
testLogger,
);
expect(iosProjectMigration.migrate(), isTrue);
iosProjectMigration.migrate();
expect(xcodeWorkspaceSharedSettings.existsSync(), isFalse);
expect(testLogger.traceText, contains('Xcode workspace settings not found, skipping build system migration'));
......@@ -230,7 +224,7 @@ keep this 2
project,
testLogger,
);
expect(iosProjectMigration.migrate(), isTrue);
iosProjectMigration.migrate();
expect(xcodeWorkspaceSharedSettings.existsSync(), isTrue);
expect(testLogger.statusText, isEmpty);
});
......@@ -253,7 +247,7 @@ keep this 2
project,
testLogger,
);
expect(iosProjectMigration.migrate(), isTrue);
iosProjectMigration.migrate();
expect(xcodeWorkspaceSharedSettings.existsSync(), isFalse);
expect(testLogger.statusText, contains('Legacy build system detected, removing'));
......@@ -279,7 +273,7 @@ keep this 2
project,
testLogger,
);
expect(iosProjectMigration.migrate(), isTrue);
iosProjectMigration.migrate();
expect(xcodeProjectWorkspaceData.existsSync(), isFalse);
expect(testLogger.traceText, contains('Xcode project workspace data not found, skipping build location migration.'));
......@@ -301,7 +295,7 @@ keep this 2
project,
testLogger,
);
expect(iosProjectMigration.migrate(), isTrue);
iosProjectMigration.migrate();
expect(xcodeProjectWorkspaceData.existsSync(), isTrue);
expect(testLogger.statusText, isEmpty);
});
......@@ -325,7 +319,7 @@ keep this 2
project,
testLogger,
);
expect(iosProjectMigration.migrate(), isTrue);
iosProjectMigration.migrate();
expect(xcodeProjectWorkspaceData.readAsStringSync(), '''
<?xml version="1.0" encoding="UTF-8"?>
<Workspace
......@@ -358,7 +352,7 @@ keep this 2
project,
testLogger,
);
expect(iosProjectMigration.migrate(), isTrue);
iosProjectMigration.migrate();
expect(xcodeProjectInfoFile.existsSync(), isFalse);
expect(testLogger.traceText, contains('Xcode project not found, skipping Runner project build settings and configuration migration'));
......@@ -374,7 +368,7 @@ keep this 2
project,
testLogger,
);
expect(iosProjectMigration.migrate(), isTrue);
iosProjectMigration.migrate();
expect(xcodeProjectInfoFile.lastModifiedSync(), projectLastModified);
expect(xcodeProjectInfoFile.readAsStringSync(), contents);
......@@ -402,7 +396,7 @@ keep this 3
project,
testLogger,
);
expect(iosProjectMigration.migrate(), isTrue);
iosProjectMigration.migrate();
expect(xcodeProjectInfoFile.readAsStringSync(), '''
97C147031CF9000F007C117D /* Debug */ = {
......@@ -457,7 +451,7 @@ keep this 3
project,
testLogger,
);
expect(iosProjectMigration.migrate(), isTrue);
iosProjectMigration.migrate();
expect(xcodeProjectInfoFile.readAsStringSync(), '''
97C147031CF9000F007C1171 /* Debug */ = {
......@@ -520,7 +514,7 @@ keep this 3
project,
testLogger,
);
expect(iosProjectMigration.migrate(), isTrue);
iosProjectMigration.migrate();
expect(xcodeProjectInfoFile.existsSync(), isFalse);
expect(appFrameworkInfoPlist.existsSync(), isFalse);
expect(podfile.existsSync(), isFalse);
......@@ -551,7 +545,7 @@ keep this 3
project,
testLogger,
);
expect(iosProjectMigration.migrate(), isTrue);
iosProjectMigration.migrate();
expect(xcodeProjectInfoFile.lastModifiedSync(), projectLastModified);
expect(xcodeProjectInfoFile.readAsStringSync(), xcodeProjectInfoFileContents);
......@@ -597,7 +591,7 @@ platform :ios, '9.0'
project,
testLogger,
);
expect(iosProjectMigration.migrate(), isTrue);
iosProjectMigration.migrate();
expect(xcodeProjectInfoFile.readAsStringSync(), '''
GCC_WARN_UNUSED_VARIABLE = YES;
......@@ -656,7 +650,7 @@ platform :ios, '11.0'
project,
testLogger,
);
expect(iosProjectMigration.migrate(), isTrue);
iosProjectMigration.migrate();
expect(xcodeProjectInfoFile.existsSync(), isFalse);
expect(xcodeProjectSchemeFile.existsSync(), isFalse);
......@@ -688,7 +682,7 @@ platform :ios, '11.0'
project,
testLogger,
);
expect(iosProjectMigration.migrate(), isTrue);
iosProjectMigration.migrate();
expect(xcodeProjectInfoFile.lastModifiedSync(), projectLastModified);
expect(xcodeProjectInfoFile.readAsStringSync(), xcodeProjectInfoFileContents);
......@@ -718,7 +712,7 @@ platform :ios, '11.0'
project,
testLogger,
);
expect(iosProjectMigration.migrate(), isTrue);
iosProjectMigration.migrate();
expect(xcodeProjectInfoFile.readAsStringSync(), '''
classes = {
......@@ -759,7 +753,7 @@ platform :ios, '11.0'
project,
testLogger,
);
expect(iosProjectMigration.migrate(), isTrue);
iosProjectMigration.migrate();
expect(infoPlistFile.existsSync(), isFalse);
expect(testLogger.traceText, contains('Info.plist not found, skipping host app Info.plist migration.'));
......@@ -786,7 +780,7 @@ platform :ios, '11.0'
testLogger,
);
final DateTime infoPlistFileLastModified = infoPlistFile.lastModifiedSync();
expect(iosProjectMigration.migrate(), isTrue);
iosProjectMigration.migrate();
expect(infoPlistFile.lastModifiedSync(), infoPlistFileLastModified);
expect(testLogger.statusText, isEmpty);
......@@ -807,7 +801,7 @@ platform :ios, '11.0'
project,
testLogger,
);
expect(iosProjectMigration.migrate(), isTrue);
iosProjectMigration.migrate();
expect(infoPlistFile.readAsStringSync(), equals('''
<?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">
......@@ -913,7 +907,7 @@ platform :ios, '11.0'
project,
testLogger,
);
expect(iosProjectMigration.migrate(), isTrue);
iosProjectMigration.migrate();
expect(xcodeProjectInfoFile.existsSync(), isFalse);
expect(testLogger.traceText, contains('Xcode project not found, skipping script build phase dependency analysis removal'));
......@@ -939,7 +933,7 @@ platform :ios, '11.0'
project,
testLogger,
);
expect(iosProjectMigration.migrate(), isTrue);
iosProjectMigration.migrate();
expect(xcodeProjectInfoFile.lastModifiedSync(), projectLastModified);
expect(xcodeProjectInfoFile.readAsStringSync(), xcodeProjectInfoFileContents);
......@@ -970,7 +964,7 @@ platform :ios, '11.0'
project,
testLogger,
);
expect(iosProjectMigration.migrate(), isTrue);
iosProjectMigration.migrate();
expect(xcodeProjectInfoFile.readAsStringSync(), '''
/* Begin PBXShellScriptBuildPhase section */
......@@ -1020,15 +1014,11 @@ class FakeIosProject extends Fake implements IosProject {
}
class FakeIOSMigrator extends ProjectMigrator {
FakeIOSMigrator({required this.succeeds})
FakeIOSMigrator()
: super(BufferLogger.test());
final bool succeeds;
@override
bool migrate() {
return succeeds;
}
void migrate() {}
@override
String migrateLine(String line) {
......
......@@ -37,7 +37,7 @@ void main() {
testLogger,
testUsage,
);
expect(macosProjectMigration.migrate(), isTrue);
macosProjectMigration.migrate();
expect(testUsage.events, isEmpty);
expect(xcodeProjectInfoFile.existsSync(), isFalse);
......@@ -61,7 +61,7 @@ void main() {
testLogger,
testUsage,
);
expect(macosProjectMigration.migrate(), isTrue);
macosProjectMigration.migrate();
expect(testUsage.events, isEmpty);
expect(xcodeProjectInfoFile.lastModifiedSync(), projectLastModified);
......@@ -82,7 +82,7 @@ shellScript = "echo \"$PRODUCT_NAME.app\" > \"$PROJECT_DIR\"/Flutter/ephemeral/.
testLogger,
testUsage,
);
expect(macosProjectMigration.migrate(), isTrue);
macosProjectMigration.migrate();
expect(xcodeProjectInfoFile.readAsStringSync(), contents);
expect(testLogger.statusText, isEmpty);
});
......@@ -105,7 +105,7 @@ keep this 2
testLogger,
testUsage,
);
expect(macosProjectMigration.migrate(), isTrue);
macosProjectMigration.migrate();
expect(testUsage.events, isEmpty);
expect(xcodeProjectInfoFile.readAsStringSync(), r'''
......@@ -178,7 +178,7 @@ keep this 2
project,
testLogger,
);
expect(macOSProjectMigration.migrate(), isTrue);
macOSProjectMigration.migrate();
expect(xcodeProjectInfoFile.existsSync(), isFalse);
expect(podfile.existsSync(), isFalse);
......@@ -201,7 +201,7 @@ keep this 2
project,
testLogger,
);
expect(macOSProjectMigration.migrate(), isTrue);
macOSProjectMigration.migrate();
expect(xcodeProjectInfoFile.lastModifiedSync(), projectLastModified);
expect(xcodeProjectInfoFile.readAsStringSync(), xcodeProjectInfoFileContents);
......@@ -227,7 +227,7 @@ platform :osx, '10.11'
project,
testLogger,
);
expect(macOSProjectMigration.migrate(), isTrue);
macOSProjectMigration.migrate();
expect(xcodeProjectInfoFile.readAsStringSync(), '''
GCC_WARN_UNUSED_VARIABLE = YES;
......
......@@ -15,18 +15,6 @@ import '../../src/common.dart';
void main () {
group('CMake project migration', () {
testWithoutContext('migrators succeed', () {
final FakeCmakeMigrator fakeCmakeMigrator = FakeCmakeMigrator(succeeds: true);
final ProjectMigration migration = ProjectMigration(<ProjectMigrator>[fakeCmakeMigrator]);
expect(migration.run(), isTrue);
});
testWithoutContext('migrators fail', () {
final FakeCmakeMigrator fakeCmakeMigrator = FakeCmakeMigrator(succeeds: false);
final ProjectMigration migration = ProjectMigration(<ProjectMigrator>[fakeCmakeMigrator]);
expect(migration.run(), isFalse);
});
group('migrate add_custom_command() to use VERBATIM', () {
late MemoryFileSystem memoryFileSystem;
late BufferLogger testLogger;
......@@ -50,7 +38,7 @@ void main () {
mockCmakeProject,
testLogger,
);
expect(cmakeProjectMigration.migrate(), isTrue);
cmakeProjectMigration.migrate();
expect(managedCmakeFile.existsSync(), isFalse);
expect(testLogger.traceText, contains('CMake project not found, skipping add_custom_command() VERBATIM migration'));
......@@ -66,7 +54,7 @@ void main () {
mockCmakeProject,
testLogger,
);
expect(cmakeProjectMigration.migrate(), isTrue);
cmakeProjectMigration.migrate();
expect(managedCmakeFile.lastModifiedSync(), projectLastModified);
expect(managedCmakeFile.readAsStringSync(), contents);
......@@ -93,7 +81,7 @@ add_custom_command(
mockCmakeProject,
testLogger,
);
expect(cmakeProjectMigration.migrate(), isTrue);
cmakeProjectMigration.migrate();
expect(managedCmakeFile.lastModifiedSync(), projectLastModified);
expect(managedCmakeFile.readAsStringSync(), contents);
......@@ -117,7 +105,7 @@ add_custom_command(
mockCmakeProject,
testLogger,
);
expect(cmakeProjectMigration.migrate(), isTrue);
cmakeProjectMigration.migrate();
expect(managedCmakeFile.readAsStringSync(), r'''
add_custom_command(
......@@ -151,7 +139,7 @@ add_custom_command(
mockCmakeProject,
testLogger,
);
expect(cmakeProjectMigration.migrate(), isTrue);
cmakeProjectMigration.migrate();
expect(managedCmakeFile.readAsStringSync(), r'''
add_custom_command(
......@@ -179,15 +167,11 @@ class FakeCmakeProject extends Fake implements CmakeBasedProject {
}
class FakeCmakeMigrator extends ProjectMigrator {
FakeCmakeMigrator({required this.succeeds})
FakeCmakeMigrator()
: super(BufferLogger.test());
final bool succeeds;
@override
bool migrate() {
return succeeds;
}
void migrate() { }
@override
String migrateLine(String line) {
......
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