feat: add support for releasing and patching macOS apps #2644
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅ 📢 Thoughts on this report? Let us know! |
packages/shorebird_cli/lib/src/commands/patch/macos_patcher.dart
Outdated
Show resolved
Hide resolved
| .sorted( | ||
| (a, b) => b.statSync().modified.compareTo(a.statSync().modified), | ||
| ) | ||
| .firstWhereOrNull((directory) => directory.path.endsWith('.app')); |
There was a problem hiding this comment.
I thought in the iOS case we'd move to reading the path from the logs? But maybe that was a different kind of path?
There was a problem hiding this comment.
You might be thinking of the app.dill?
| }); | ||
|
|
||
| String get _patchClassTableLinkInfoPath => | ||
| p.join(buildDirectory.path, 'macos', 'shorebird', 'App.ct.link'); |
There was a problem hiding this comment.
I probably would have written myself a little _buildPath('macos', 'shorebird','App.ct.link') helper. 🤷
packages/shorebird_cli/lib/src/commands/patch/macos_patcher.dart
Outdated
Show resolved
Hide resolved
| outputDirectory: tempDir, | ||
| ); | ||
| releaseClassTableLinkInfoFile = File(p.join(tempDir.path, 'App.ct.link')); | ||
| if (!releaseClassTableLinkInfoFile.existsSync()) { |
There was a problem hiding this comment.
I probably would have had an _existsOrExit(file, 'class table link debug info') if I'm doing this more often. 🤷
There was a problem hiding this comment.
I'm planning to refactor all this and extract the shared logic in a subsequent PR.
| 'cache', | ||
| 'artifacts', | ||
| 'engine', | ||
| 'darwin-x64-release', |
There was a problem hiding this comment.
We're using the enum to differentiate based on Target, but we may also need to differentiate based on Host
There was a problem hiding this comment.
Yep, we'll probably want to refactor ShorebirdArtifacts to accept host and/or target in the future.
| @@ -652,6 +714,7 @@ aar artifact already exists, continuing...''', | |||
|
|
|||
| final zippedRunner = await Directory(runnerPath).zipToTempFile(); | |||
| try { | |||
| logger.detail('[archive] zipped runner.app to ${zippedRunner.path}'); | |||
There was a problem hiding this comment.
Do we want this log? We don't have an equivalent for other platforms so just wondering if this was left in intentionally.
There was a problem hiding this comment.
I don't feel strongly. Users will only see it when running with the -v flag, and there's so much other output that I'm not sure it matters much one way or the other.
packages/shorebird_cli/lib/src/commands/patch/macos_patcher.dart
Outdated
Show resolved
Hide resolved
packages/shorebird_cli/lib/src/commands/patch/macos_patcher.dart
Outdated
Show resolved
Hide resolved
| ), | ||
| ); | ||
|
|
||
| Future<_LinkResult> _runLinker({ |
There was a problem hiding this comment.
We should probably create a Linker class that contains this shared logic instead of duplicating on both iOS and MacOS.
There was a problem hiding this comment.
Yep, I have a TODO earlier in this file on line 33 for the _LinkResult typedef.
packages/shorebird_cli/lib/src/commands/release/macos_releaser.dart
Outdated
Show resolved
Hide resolved
Description
Adds
shorebird release macosandshorebird patch macosRemaining work
Note: These should probably be moved to issues, as they aren't prereqs for landing this PR, but for announcing beta mac support.
Fixes #2654
Type of Change