Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(shorebird_cli): fix an issue where ios releases could not properly parse a flutter hash #2467

Merged
merged 2 commits into from
Sep 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 =
bryanoltman marked this conversation as resolved.
Show resolved Hide resolved
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
Loading