Skip to content

Commit

Permalink
fix(shorebird_cli): fix an issue where ios releases could not properl…
Browse files Browse the repository at this point in the history
…y parse a flutter hash (#2467)
  • Loading branch information
bryanoltman authored Sep 4, 2024
1 parent bfbd3c7 commit 4b95ae7
Show file tree
Hide file tree
Showing 6 changed files with 162 additions and 37 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import 'package:crypto/crypto.dart';
import 'package:mason_logger/mason_logger.dart';
import 'package:path/path.dart' as p;
import 'package:platform/platform.dart';
import 'package:pub_semver/pub_semver.dart';
import 'package:shorebird_cli/src/archive_analysis/plist.dart';
import 'package:shorebird_cli/src/artifact_builder.dart';
import 'package:shorebird_cli/src/artifact_manager.dart';
Expand Down Expand Up @@ -82,8 +81,9 @@ To change the version of this release, change your app's version in your pubspec

final flutterVersionArg = argResults['flutter-version'] as String?;
if (flutterVersionArg != null) {
if (Version.parse(flutterVersionArg) <
minimumSupportedIosFlutterVersion) {
final version =
await shorebirdFlutter.resolveFlutterVersion(flutterVersionArg);
if (version != null && version < minimumSupportedIosFlutterVersion) {
logger.err(
'''
iOS releases are not supported with Flutter versions older than $minimumSupportedIosFlutterVersion.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import 'package:shorebird_cli/src/commands/release/release.dart';
import 'package:shorebird_cli/src/common_arguments.dart';
import 'package:shorebird_cli/src/config/config.dart';
import 'package:shorebird_cli/src/extensions/arg_results.dart';
import 'package:shorebird_cli/src/extensions/version.dart';
import 'package:shorebird_cli/src/logger.dart';
import 'package:shorebird_cli/src/metadata/metadata.dart';
import 'package:shorebird_cli/src/platform.dart';
Expand Down Expand Up @@ -306,16 +305,9 @@ of the iOS app that is using this module. (aar and ios-framework only)''',
return shorebirdEnv.flutterRevision;
}

final parsedVersion = tryParseVersion(flutterVersionArg!);
if (parsedVersion == null) {
// If we fail to parse flutterVersionArg as a semver version, attempt to
// use the provided value as a revision.
return flutterVersionArg!;
}

final String? revision;
try {
revision = await shorebirdFlutter.getRevisionForVersion(
revision = await shorebirdFlutter.resolveFlutterRevision(
flutterVersionArg!,
);
} catch (error) {
Expand Down
45 changes: 45 additions & 0 deletions packages/shorebird_cli/lib/src/shorebird_flutter.dart
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ 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/executables/executables.dart';
import 'package:shorebird_cli/src/extensions/version.dart';
import 'package:shorebird_cli/src/logger.dart';
import 'package:shorebird_cli/src/platform.dart';
import 'package:shorebird_cli/src/shorebird_env.dart';
Expand Down Expand Up @@ -205,6 +206,50 @@ class ShorebirdFlutter {
.firstOrNull;
}

/// Translates [versionOrHash] into a Flutter revision. If this is a semver
/// version, it will simply parse that into a [Version]. If not, it will
/// attempt to look up the Flutter version for the provided revision hash and
/// return the hash if a version is found, or null if not.
Future<String?> resolveFlutterRevision(String versionOrHash) async {
final parsedVersion = tryParseVersion(versionOrHash);
if (parsedVersion != null) {
return getRevisionForVersion(versionOrHash);
}

// If we were unable to parse the version, assume it's a revision hash.
try {
final version =
await getVersionForRevision(flutterRevision: versionOrHash);
if (version != null) {
return versionOrHash;
}
} catch (_) {
return null;
}

return null;
}

/// Translates [versionOrHash] into a Flutter [Version]. If [versionOrHash]
/// is semver version string, it will simply parse that into a [Version]. If
/// not, it will assume that the input is a git commit hash and attempt to
/// map it to a Flutter version.
Future<Version?> resolveFlutterVersion(String versionOrHash) async {
final parsedVersion = tryParseVersion(versionOrHash);
if (parsedVersion != null) {
return parsedVersion;
}

try {
// If we were unable to parse the version, assume it's a revision hash.
final versionString =
await getVersionForRevision(flutterRevision: versionOrHash);
return versionString != null ? tryParseVersion(versionString) : null;
} catch (_) {
return null;
}
}

/// Returns the git revision for the provided [version].
/// e.g. 3.16.3 -> b9b23902966504a9778f4c07e3a3487fa84dcb2a
Future<String?> getRevisionForVersion(String version) async {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import 'package:mason_logger/mason_logger.dart';
import 'package:mocktail/mocktail.dart';
import 'package:path/path.dart' as p;
import 'package:platform/platform.dart';
import 'package:pub_semver/pub_semver.dart';
import 'package:scoped_deps/scoped_deps.dart';
import 'package:shorebird_cli/src/artifact_builder.dart';
import 'package:shorebird_cli/src/artifact_manager.dart';
Expand Down Expand Up @@ -123,9 +124,13 @@ void main() {
});

group('assertPreconditions', () {
final flutterVersion = Version(3, 0, 0);

setUp(() {
when(() => doctor.iosCommandValidators)
.thenReturn([flutterValidator]);
when(() => shorebirdFlutter.resolveFlutterVersion(any()))
.thenAnswer((_) async => flutterVersion);
when(flutterValidator.validate).thenAnswer((_) async => []);
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -441,7 +441,7 @@ Note: ${lightCyan.wrap('shorebird patch --platforms=android --flavor=$flavor --t
final exception = Exception('oops');
setUp(() {
when(
() => shorebirdFlutter.getRevisionForVersion(any()),
() => shorebirdFlutter.resolveFlutterRevision(any()),
).thenThrow(exception);
});

Expand All @@ -463,7 +463,7 @@ $exception''',
group('when flutter version is not supported', () {
setUp(() {
when(
() => shorebirdFlutter.getRevisionForVersion(any()),
() => shorebirdFlutter.resolveFlutterRevision(any()),
).thenAnswer((_) async => null);
});

Expand All @@ -484,7 +484,7 @@ $exception''',
const revision = '771d07b2cf';
setUp(() {
when(
() => shorebirdFlutter.getRevisionForVersion(any()),
() => shorebirdFlutter.resolveFlutterRevision(any()),
).thenAnswer((_) async => revision);
});

Expand Down Expand Up @@ -522,28 +522,6 @@ $exception''',
).called(1);
});
});

group('when flutter-version is a git hash', () {
const revision = 'deadbeef';
setUp(() {
when(() => argResults['flutter-version']).thenReturn(revision);
});

test(
'''installs the flutter version with the provided revision and completes''',
() async {
await runWithOverrides(command.run);

verify(() => shorebirdFlutter.installRevision(revision: revision))
.called(1);
// We should never attempt to treat this as a semver version.
verifyNever(
() => shorebirdFlutter.getVersionForRevision(
flutterRevision: any(named: 'flutterRevision'),
),
);
});
});
});
});

Expand Down
105 changes: 105 additions & 0 deletions packages/shorebird_cli/test/src/shorebird_flutter_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,111 @@ Tools • Dart 3.0.6 • DevTools 2.23.1''');
});
});

group('resolveFlutterRevision', () {
group('when input is a semver version', () {
test('returns the revision associated with the version if it exists',
() async {
final revision = await runWithOverrides(
() => shorebirdFlutter.resolveFlutterRevision('3.10.6'),
);
expect(revision, equals(flutterRevision));
});
});

group('when input is a commit hash that maps to a Flutter version', () {
setUp(() {
when(
() => git.forEachRef(
directory: any(named: 'directory'),
contains: any(named: 'contains'),
format: any(named: 'format'),
pattern: any(named: 'pattern'),
),
).thenAnswer((_) async => 'origin/flutter_release/1.2.3');
});

test('returns the input string', () async {
final revision = await runWithOverrides(
() => shorebirdFlutter.resolveFlutterRevision('deadbeef'),
);
expect(revision, equals('deadbeef'));
});
});

group('when input is not a commit hash that maps to a Flutter version',
() {
setUp(() {
when(
() => git.forEachRef(
directory: any(named: 'directory'),
contains: any(named: 'contains'),
format: any(named: 'format'),
pattern: any(named: 'pattern'),
),
).thenAnswer((_) async => '');
});

test('returns the input string', () async {
final revision = await runWithOverrides(
() => shorebirdFlutter.resolveFlutterRevision('not-a-version'),
);
expect(revision, isNull);
});
});
});

group('resolveFlutterVersion', () {
group('when input is a semver version', () {
test('returns the revision associated with the version if it exists',
() async {
final revision = await runWithOverrides(
() => shorebirdFlutter.resolveFlutterVersion('3.10.6'),
);
expect(revision, equals(Version(3, 10, 6)));
});
});

group('when input is not a recognized commit hash', () {
setUp(() {
when(
() => git.forEachRef(
directory: any(named: 'directory'),
contains: any(named: 'contains'),
format: any(named: 'format'),
pattern: any(named: 'pattern'),
),
).thenAnswer((_) async => '');
});

test('returns null', () async {
final revision = await runWithOverrides(
() => shorebirdFlutter.resolveFlutterVersion('not-a-version'),
);
expect(revision, isNull);
});
});

group('when input is a recognized commit hash', () {
setUp(() {
when(
() => git.forEachRef(
directory: any(named: 'directory'),
contains: any(named: 'contains'),
format: any(named: 'format'),
pattern: any(named: 'pattern'),
),
).thenAnswer((_) async => 'origin/flutter_release/1.2.3');
});

test('returns a parsed version', () async {
final revision = await runWithOverrides(
() => shorebirdFlutter.resolveFlutterVersion('deadbeef'),
);
expect(revision, equals(Version(1, 2, 3)));
});
});
});

group('getRevisionForVersion', () {
const version = '3.16.3';
const exception = ProcessException('git', ['rev-parse']);
Expand Down

0 comments on commit 4b95ae7

Please sign in to comment.