From 489f7be55aaa25e5f64d69d8e38694e489979978 Mon Sep 17 00:00:00 2001 From: Eric Seidel Date: Wed, 26 Mar 2025 08:44:53 -0700 Subject: [PATCH] feat: support field_table data in link supplement (#3008) --- bin/internal/flutter.version | 2 +- cspell.config.yaml | 1 + .../commands/patch/ios_framework_patcher.dart | 62 +++-------------- .../lib/src/commands/patch/ios_patcher.dart | 61 +++------------- .../shorebird_cli/lib/src/platform/apple.dart | 55 +++++++++++++++ .../shorebird_cli/lib/src/shorebird_env.dart | 5 ++ .../patch/ios_framework_patcher_test.dart | 69 +------------------ .../src/commands/patch/ios_patcher_test.dart | 69 +------------------ .../test/src/platform/apple_test.dart | 59 ++++++++++++++++ .../test/src/shorebird_env_test.dart | 18 +++++ 10 files changed, 162 insertions(+), 239 deletions(-) diff --git a/bin/internal/flutter.version b/bin/internal/flutter.version index fbaf98e7..42c73c64 100644 --- a/bin/internal/flutter.version +++ b/bin/internal/flutter.version @@ -1 +1 @@ -3f9cefb45389b72ff073ddf305fe0939f822143b +b14323b05f28b2c72b34f57c8059ec3001d65712 diff --git a/cspell.config.yaml b/cspell.config.yaml index b22425d6..450891fb 100644 --- a/cspell.config.yaml +++ b/cspell.config.yaml @@ -35,6 +35,7 @@ words: - dylib - endtemplate - Entra + - eseidel - exitcode - exportoptions - felangel diff --git a/packages/shorebird_cli/lib/src/commands/patch/ios_framework_patcher.dart b/packages/shorebird_cli/lib/src/commands/patch/ios_framework_patcher.dart index 5d092dea..564d199e 100644 --- a/packages/shorebird_cli/lib/src/commands/patch/ios_framework_patcher.dart +++ b/packages/shorebird_cli/lib/src/commands/patch/ios_framework_patcher.dart @@ -43,20 +43,6 @@ class IosFrameworkPatcher extends Patcher { String get _aotOutputPath => p.join(shorebirdEnv.buildDirectory.path, 'out.aot'); - String get _patchClassTableLinkInfoFile => p.join( - shorebirdEnv.buildDirectory.path, - 'ios', - 'shorebird', - 'App.ct.link', - ); - - String get _patchClassTableLinkDebugInfoPath => p.join( - shorebirdEnv.buildDirectory.path, - 'ios', - 'shorebird', - 'App.class_table.json', - ); - String get _vmcodeOutputPath => p.join(shorebirdEnv.buildDirectory.path, 'out.vmcode'); @@ -161,27 +147,12 @@ class IosFrameworkPatcher extends Patcher { releaseXcframeworkPath = tempDir.path; } - File? releaseClassTableLinkInfoFile; - File? releaseClassTableLinkDebugInfoFile; + final releaseSupplementDir = Directory.systemTemp.createTempSync(); if (supplementArtifact != null) { - final tempDir = Directory.systemTemp.createTempSync(); await artifactManager.extractZip( zipFile: supplementArtifact, - outputDirectory: tempDir, + outputDirectory: releaseSupplementDir, ); - releaseClassTableLinkInfoFile = File(p.join(tempDir.path, 'App.ct.link')); - if (!releaseClassTableLinkInfoFile.existsSync()) { - logger.err('Unable to find class table link info file'); - throw ProcessExit(ExitCode.software.code); - } - - releaseClassTableLinkDebugInfoFile = File( - p.join(tempDir.path, 'App.class_table.json'), - ); - if (!releaseClassTableLinkDebugInfoFile.existsSync()) { - logger.err('Unable to find class table link debug info file'); - throw ProcessExit(ExitCode.software.code); - } } unzipProgress.complete( @@ -194,30 +165,15 @@ class IosFrameworkPatcher extends Patcher { final aotSnapshotFile = File( p.join(shorebirdEnv.getShorebirdProjectRoot()!.path, 'build', 'out.aot'), ); + // TODO(eseidel): Drop support for builds before the linker. final useLinker = AotTools.usesLinker(shorebirdEnv.flutterRevision); if (useLinker) { - // If we're using a newer version of the linker, we need to also copy the - // necessary class table link information alongside the snapshots. - if (releaseClassTableLinkInfoFile != null && - releaseClassTableLinkDebugInfoFile != null) { - // Copy the release's class table link info file next to the release - // snapshot so that it can be used to generate a patch. - releaseClassTableLinkInfoFile.copySync( - p.join(releaseArtifactFile.parent.path, 'App.ct.link'), - ); - releaseClassTableLinkDebugInfoFile.copySync( - p.join(releaseArtifactFile.parent.path, 'App.class_table.json'), - ); - - // Copy the patch's class table link info file to the build directory - // so that it can be used to generate a patch. - File( - _patchClassTableLinkInfoFile, - ).copySync(p.join(shorebirdEnv.buildDirectory.path, 'out.ct.link')); - File(_patchClassTableLinkDebugInfoPath).copySync( - p.join(shorebirdEnv.buildDirectory.path, 'out.class_table.json'), - ); - } + apple.copySupplementFilesToSnapshotDirs( + releaseSupplementDir: releaseSupplementDir, + releaseSnapshotDir: releaseArtifactFile.parent, + patchSupplementDir: shorebirdEnv.iosSupplementDirectory, + patchSnapshotDir: shorebirdEnv.buildDirectory, + ); await apple.runLinker( kernelFile: File(_appDillCopyPath), diff --git a/packages/shorebird_cli/lib/src/commands/patch/ios_patcher.dart b/packages/shorebird_cli/lib/src/commands/patch/ios_patcher.dart index 5e1cb52b..872f0bf5 100644 --- a/packages/shorebird_cli/lib/src/commands/patch/ios_patcher.dart +++ b/packages/shorebird_cli/lib/src/commands/patch/ios_patcher.dart @@ -43,20 +43,6 @@ class IosPatcher extends Patcher { required super.target, }); - String get _patchClassTableLinkInfoPath => p.join( - shorebirdEnv.buildDirectory.path, - 'ios', - 'shorebird', - 'App.ct.link', - ); - - String get _patchClassTableLinkDebugInfoPath => p.join( - shorebirdEnv.buildDirectory.path, - 'ios', - 'shorebird', - 'App.class_table.json', - ); - String get _aotOutputPath => p.join(shorebirdEnv.buildDirectory.path, 'out.aot'); @@ -243,27 +229,12 @@ For more information see: ${supportedFlutterVersionsUrl.toLink()}'''); releaseXcarchivePath = tempDir.path; } - File? releaseClassTableLinkInfoFile; - File? releaseClassTableLinkDebugInfoFile; + final releaseSupplementDir = Directory.systemTemp.createTempSync(); if (supplementArtifact != null) { - final tempDir = Directory.systemTemp.createTempSync(); await artifactManager.extractZip( zipFile: supplementArtifact, - outputDirectory: tempDir, + outputDirectory: releaseSupplementDir, ); - releaseClassTableLinkInfoFile = File(p.join(tempDir.path, 'App.ct.link')); - if (!releaseClassTableLinkInfoFile.existsSync()) { - logger.err('Unable to find class table link info file'); - throw ProcessExit(ExitCode.software.code); - } - - releaseClassTableLinkDebugInfoFile = File( - p.join(tempDir.path, 'App.class_table.json'), - ); - if (!releaseClassTableLinkDebugInfoFile.existsSync()) { - logger.err('Unable to find class table link debug info file'); - throw ProcessExit(ExitCode.software.code); - } } unzipProgress.complete(); @@ -280,28 +251,12 @@ For more information see: ${supportedFlutterVersionsUrl.toLink()}'''); final useLinker = AotTools.usesLinker(shorebirdEnv.flutterRevision); if (useLinker) { - // If we're using a newer version of the linker, we need to also copy the - // necessary class table link information alongside the snapshots. - if (releaseClassTableLinkInfoFile != null && - releaseClassTableLinkDebugInfoFile != null) { - // Copy the release's class table link info file next to the release - // snapshot so that it can be used to generate a patch. - releaseClassTableLinkInfoFile.copySync( - p.join(releaseArtifactFile.parent.path, 'App.ct.link'), - ); - releaseClassTableLinkDebugInfoFile.copySync( - p.join(releaseArtifactFile.parent.path, 'App.class_table.json'), - ); - - // Copy the patch's class table link info file to the build directory - // so that it can be used to generate a patch. - File( - _patchClassTableLinkInfoPath, - ).copySync(p.join(shorebirdEnv.buildDirectory.path, 'out.ct.link')); - File(_patchClassTableLinkDebugInfoPath).copySync( - p.join(shorebirdEnv.buildDirectory.path, 'out.class_table.json'), - ); - } + apple.copySupplementFilesToSnapshotDirs( + releaseSupplementDir: releaseSupplementDir, + releaseSnapshotDir: releaseArtifactFile.parent, + patchSupplementDir: shorebirdEnv.iosSupplementDirectory, + patchSnapshotDir: shorebirdEnv.buildDirectory, + ); final (:exitCode, :linkPercentage) = await apple.runLinker( kernelFile: File(_appDillCopyPath), diff --git a/packages/shorebird_cli/lib/src/platform/apple.dart b/packages/shorebird_cli/lib/src/platform/apple.dart index d57bc374..7a54f6c9 100644 --- a/packages/shorebird_cli/lib/src/platform/apple.dart +++ b/packages/shorebird_cli/lib/src/platform/apple.dart @@ -110,6 +110,60 @@ Apple get apple => read(appleRef); /// A class that provides information about the iOS platform. class Apple { + /// Copies the supplement files into the build directory. + /// Currently we run gen_snapshot from `flutter`, both for the release and + /// patch builds. Both times it produces supplement files in a directory. + /// In the release case, these files are zipped up and stored as an artifact + /// on our servers for later use. In the patch case, they were created on + /// disk just before this call. + /// In both cases we need to copy the supplement files from these directories + /// to right next to where the snapshot files are before calling into + /// `aot_tools` to link the two snapshots together. + // TODO(eseidel): We should pass the entire supplement directories to + // `aot_tools` rather than having to know the contents within `shorebird`. + void copySupplementFilesToSnapshotDirs({ + required Directory releaseSupplementDir, + required Directory releaseSnapshotDir, + required Directory patchSupplementDir, + required Directory patchSnapshotDir, + }) { + // All known supplement files names seen across all Flutter versions. + final supplementFileNames = [ + 'App.ct.link', + 'App.class_table.json', + 'App.ft.link', + 'App.field_table.json', + ]; + + // This uses maybeCopy because not all versions of gen_snapshot/aot_tools + // use the same supplement files. At the `shorebird` level we don't know + // which files should be present, so we just try to copy all. + void maybeCopy(File file, Directory destDir, {String? newBaseName}) { + if (!file.existsSync()) return; + final baseName = p.basename(file.path); + final destName = + newBaseName != null + ? baseName.replaceFirst('App', newBaseName) + : baseName; + file.copySync(p.join(destDir.path, destName)); + } + + final releaseSupplementFiles = supplementFileNames.map( + (name) => File(p.join(releaseSupplementDir.path, name)), + ); + for (final file in releaseSupplementFiles) { + maybeCopy(file, releaseSnapshotDir); + } + + final patchSupplementFiles = supplementFileNames.map( + (name) => File(p.join(patchSupplementDir.path, name)), + ); + const patchSnapshotBaseName = 'out'; + for (final file in patchSupplementFiles) { + maybeCopy(file, patchSnapshotDir, newBaseName: patchSnapshotBaseName); + } + } + /// Returns the set of flavors for the Xcode project associated with /// [platform], if this project has that platform configured. Set? flavors({required ApplePlatform platform}) { @@ -155,6 +209,7 @@ class Apple { .toSet(); } + // TODO(eseidel): Move this into a "linker" class rather than Apple. /// Runs the linking step to minimize differences between patch and release /// and maximize code that can be executed on the CPU. Future runLinker({ diff --git a/packages/shorebird_cli/lib/src/shorebird_env.dart b/packages/shorebird_cli/lib/src/shorebird_env.dart index 111490e2..65581830 100644 --- a/packages/shorebird_cli/lib/src/shorebird_env.dart +++ b/packages/shorebird_cli/lib/src/shorebird_env.dart @@ -98,6 +98,11 @@ class ShorebirdEnv { return Directory(p.join(getFlutterProjectRoot()!.path, 'build')); } + /// Where the link supplement files are stored. + // TODO(eseidel): Make this not iOS specific. + Directory get iosSupplementDirectory => + Directory(p.join(buildDirectory.path, 'ios', 'shorebird')); + /// The `shorebird.yaml` file for this project. File getShorebirdYamlFile({required Directory cwd}) { return File(p.join(cwd.path, 'shorebird.yaml')); diff --git a/packages/shorebird_cli/test/src/commands/patch/ios_framework_patcher_test.dart b/packages/shorebird_cli/test/src/commands/patch/ios_framework_patcher_test.dart index 6a4cba00..09e2d716 100644 --- a/packages/shorebird_cli/test/src/commands/patch/ios_framework_patcher_test.dart +++ b/packages/shorebird_cli/test/src/commands/patch/ios_framework_patcher_test.dart @@ -135,6 +135,9 @@ void main() { when( () => shorebirdEnv.buildDirectory, ).thenReturn(Directory(p.join(projectRoot.path, 'build'))); + when(() => shorebirdEnv.iosSupplementDirectory).thenReturn( + Directory(p.join(projectRoot.path, 'build', 'shorebird', 'ios')), + ); when( () => shorebirdEnv.getShorebirdProjectRoot(), ).thenReturn(projectRoot); @@ -803,72 +806,6 @@ void main() { ); }); - group('when class table link info is not present', () { - setUp(() { - when( - () => artifactManager.extractZip( - zipFile: supplementArtifactFile, - outputDirectory: any(named: 'outputDirectory'), - ), - ).thenAnswer((invocation) async {}); - }); - - test('exits with code 70', () async { - await expectLater( - () => runWithOverrides( - () => patcher.createPatchArtifacts( - appId: appId, - releaseId: releaseId, - releaseArtifact: releaseArtifactFile, - supplementArtifact: supplementArtifactFile, - ), - ), - exitsWithCode(ExitCode.software), - ); - - verify( - () => logger.err('Unable to find class table link info file'), - ).called(1); - }); - }); - - group('when debug info is missing', () { - setUp(() { - when( - () => artifactManager.extractZip( - zipFile: supplementArtifactFile, - outputDirectory: any(named: 'outputDirectory'), - ), - ).thenAnswer((invocation) async { - final outDir = - invocation.namedArguments[#outputDirectory] as Directory; - File( - p.join(outDir.path, 'App.ct.link'), - ).createSync(recursive: true); - }); - }); - - test('exits with code 70', () async { - await expectLater( - () => runWithOverrides( - () => patcher.createPatchArtifacts( - appId: appId, - releaseId: releaseId, - releaseArtifact: releaseArtifactFile, - supplementArtifact: supplementArtifactFile, - ), - ), - exitsWithCode(ExitCode.software), - ); - - verify( - () => logger.err( - 'Unable to find class table link debug info file', - ), - ).called(1); - }); - }); - group('when class table link info & debug info are present', () { setUp(() { when( diff --git a/packages/shorebird_cli/test/src/commands/patch/ios_patcher_test.dart b/packages/shorebird_cli/test/src/commands/patch/ios_patcher_test.dart index 35266dec..ed28588c 100644 --- a/packages/shorebird_cli/test/src/commands/patch/ios_patcher_test.dart +++ b/packages/shorebird_cli/test/src/commands/patch/ios_patcher_test.dart @@ -143,6 +143,9 @@ void main() { when( () => shorebirdEnv.buildDirectory, ).thenReturn(Directory(p.join(projectRoot.path, 'build'))); + when(() => shorebirdEnv.iosSupplementDirectory).thenReturn( + Directory(p.join(projectRoot.path, 'build', 'shorebird', 'ios')), + ); when(aotTools.isLinkDebugInfoSupported).thenAnswer((_) async => false); @@ -1126,72 +1129,6 @@ For more information see: ${supportedFlutterVersionsUrl.toLink()}'''), ); }); - group('when class table link info is not present', () { - setUp(() { - when( - () => artifactManager.extractZip( - zipFile: supplementArtifactFile, - outputDirectory: any(named: 'outputDirectory'), - ), - ).thenAnswer((invocation) async {}); - }); - - test('exits with code 70', () async { - await expectLater( - () => runWithOverrides( - () => patcher.createPatchArtifacts( - appId: appId, - releaseId: releaseId, - releaseArtifact: releaseArtifactFile, - supplementArtifact: supplementArtifactFile, - ), - ), - exitsWithCode(ExitCode.software), - ); - - verify( - () => logger.err('Unable to find class table link info file'), - ).called(1); - }); - }); - - group('when debug info is missing', () { - setUp(() { - when( - () => artifactManager.extractZip( - zipFile: supplementArtifactFile, - outputDirectory: any(named: 'outputDirectory'), - ), - ).thenAnswer((invocation) async { - final outDir = - invocation.namedArguments[#outputDirectory] as Directory; - File( - p.join(outDir.path, 'App.ct.link'), - ).createSync(recursive: true); - }); - }); - - test('exits with code 70', () async { - await expectLater( - () => runWithOverrides( - () => patcher.createPatchArtifacts( - appId: appId, - releaseId: releaseId, - releaseArtifact: releaseArtifactFile, - supplementArtifact: supplementArtifactFile, - ), - ), - exitsWithCode(ExitCode.software), - ); - - verify( - () => logger.err( - 'Unable to find class table link debug info file', - ), - ).called(1); - }); - }); - group('when class table link info & debug info are present', () { setUp(() { when( diff --git a/packages/shorebird_cli/test/src/platform/apple_test.dart b/packages/shorebird_cli/test/src/platform/apple_test.dart index f9481f35..ba11df69 100644 --- a/packages/shorebird_cli/test/src/platform/apple_test.dart +++ b/packages/shorebird_cli/test/src/platform/apple_test.dart @@ -92,6 +92,65 @@ To add macOS, run "flutter create . --platforms macos"'''); ).thenReturn(projectRoot); }); + group('copySupplementFilesToSnapshotDirs', () { + test('copies all files next to snapshots', () { + final names = [ + 'App.class_table.json', + 'App.field_table.json', + 'App.ct.link', + 'App.ft.link', + ]; + + void createFiles(Directory dir) { + for (final name in names) { + File(p.join(dir.path, name)).createSync(); + } + } + + final releaseSupplementDir = Directory.systemTemp.createTempSync(); + final patchSupplementDir = Directory.systemTemp.createTempSync(); + createFiles(releaseSupplementDir); + createFiles(patchSupplementDir); + + final releaseSnapshotDir = Directory.systemTemp.createTempSync(); + final patchSnapshotDir = Directory.systemTemp.createTempSync(); + apple.copySupplementFilesToSnapshotDirs( + releaseSupplementDir: releaseSupplementDir, + releaseSnapshotDir: releaseSnapshotDir, + patchSupplementDir: patchSupplementDir, + patchSnapshotDir: patchSnapshotDir, + ); + expect(Directory(releaseSnapshotDir.path).listSync(), hasLength(4)); + expect(Directory(patchSnapshotDir.path).listSync(), hasLength(4)); + }); + + test('copies only some files next to snapshots', () { + final names = ['App.class_table.json', 'App.ct.link', 'ignored.txt']; + + void createFiles(Directory dir) { + for (final name in names) { + File(p.join(dir.path, name)).createSync(); + } + } + + final releaseSupplementDir = Directory.systemTemp.createTempSync(); + final patchSupplementDir = Directory.systemTemp.createTempSync(); + createFiles(releaseSupplementDir); + createFiles(patchSupplementDir); + + final releaseSnapshotDir = Directory.systemTemp.createTempSync(); + final patchSnapshotDir = Directory.systemTemp.createTempSync(); + apple.copySupplementFilesToSnapshotDirs( + releaseSupplementDir: releaseSupplementDir, + releaseSnapshotDir: releaseSnapshotDir, + patchSupplementDir: patchSupplementDir, + patchSnapshotDir: patchSnapshotDir, + ); + expect(Directory(releaseSnapshotDir.path).listSync(), hasLength(2)); + expect(Directory(patchSnapshotDir.path).listSync(), hasLength(2)); + }); + }); + group('ios', () { final schemesPath = p.join( 'ios', diff --git a/packages/shorebird_cli/test/src/shorebird_env_test.dart b/packages/shorebird_cli/test/src/shorebird_env_test.dart index bf1b4b79..e0a47b58 100644 --- a/packages/shorebird_cli/test/src/shorebird_env_test.dart +++ b/packages/shorebird_cli/test/src/shorebird_env_test.dart @@ -240,6 +240,24 @@ void main() { }); }); + group('iosSupplementDirectory', () { + test('returns correct path', () { + final tempDir = Directory.systemTemp.createTempSync(); + File(p.join(tempDir.path, 'pubspec.yaml')).createSync(recursive: true); + Directory( + p.join(tempDir.path, 'build', 'ios', 'shorebird'), + ).createSync(recursive: true); + final supplementDirectory = IOOverrides.runZoned( + () => runWithOverrides(() => shorebirdEnv.iosSupplementDirectory), + getCurrentDirectory: () => tempDir, + ); + expect( + supplementDirectory.path, + equals(p.join(tempDir.path, 'build', 'ios', 'shorebird')), + ); + }); + }); + group('macosPodfileLockFile', () { test('returns correct path', () { final tempDir = Directory.systemTemp.createTempSync();