fix(shorebird_cli): infer --build-name and --build-number from --release-version if necessary (#2280)

This commit is contained in:
Bryan Oltman
2024-06-24 17:19:49 -04:00
committed by GitHub
parent 29e8af96bd
commit da12995451
10 changed files with 171 additions and 12 deletions
@@ -77,7 +77,7 @@ class AarPatcher extends Patcher {
);
@override
Future<File> buildPatchArtifact() async {
Future<File> buildPatchArtifact({String? releaseVersion}) async {
final flutterVersionString = await shorebirdFlutter.getVersionAndRevision();
final buildProgress =
logger.progress('Building patch with Flutter $flutterVersionString');
@@ -69,7 +69,7 @@ class AndroidPatcher extends Patcher {
}
@override
Future<File> buildPatchArtifact() async {
Future<File> buildPatchArtifact({String? releaseVersion}) async {
final File aabFile;
final flutterVersionString = await shorebirdFlutter.getVersionAndRevision();
final buildProgress =
@@ -79,7 +79,8 @@ class AndroidPatcher extends Patcher {
aabFile = await artifactBuilder.buildAppBundle(
flavor: flavor,
target: target,
args: argResults.forwardedArgs,
args: argResults.forwardedArgs +
buildNameAndNumberArgsFromReleaseVersion(releaseVersion),
base64PublicKey: argResults.encodedPublicKey,
);
buildProgress.complete();
@@ -93,7 +93,7 @@ class IosFrameworkPatcher extends Patcher {
);
@override
Future<File> buildPatchArtifact() async {
Future<File> buildPatchArtifact({String? releaseVersion}) async {
final flutterVersionString = await shorebirdFlutter.getVersionAndRevision();
final buildProgress = logger.progress(
'Building patch with Flutter $flutterVersionString',
@@ -134,7 +134,7 @@ This may indicate that the patch contains native changes, which cannot be applie
}
@override
Future<File> buildPatchArtifact() async {
Future<File> buildPatchArtifact({String? releaseVersion}) async {
final File exportOptionsPlist;
try {
exportOptionsPlist = ios.exportOptionsPlistFromArgs(argResults);
@@ -172,7 +172,8 @@ For more information see: $supportedVersionsLink''',
exportOptionsPlist: exportOptionsPlist,
flavor: flavor,
target: target,
args: argResults.forwardedArgs,
args: argResults.forwardedArgs +
buildNameAndNumberArgsFromReleaseVersion(releaseVersion),
base64PublicKey: argResults.encodedPublicKey,
);
} on ProcessException catch (error) {
@@ -264,7 +264,9 @@ NOTE: this is ${styleBold.wrap('not')} recommended. Asset changes cannot be incl
// Don't built the patch artifact twice with the same Flutter revision.
if (lastBuiltFlutterRevision != release.flutterRevision) {
patchArtifactFile = await patcher.buildPatchArtifact();
patchArtifactFile = await patcher.buildPatchArtifact(
releaseVersion: release.version,
);
}
final diffStatus = await assertUnpatchableDiffs(
@@ -77,7 +77,7 @@ https://docs.shorebird.dev/status#link-percentage-ios
/// Builds the release artifacts for the given platform. Returns the "primary"
/// artifact for the platform (e.g. the AAB for Android, the IPA for iOS).
Future<File> buildPatchArtifact();
Future<File> buildPatchArtifact({String? releaseVersion});
/// Determines the release version from the provided app artifact.
Future<String> extractReleaseVersionFromArtifact(File artifact);
@@ -115,4 +115,42 @@ https://docs.shorebird.dev/status#link-percentage-ios
File get debugInfoFile {
return File(p.join(buildDirectory.path, 'patch-debug.zip'));
}
/// Extracts the --build-name and --build-number from the --release-version
/// argument if it's provided. Given `--release-version=1.2.3+4`, this will
/// return `['--build-name=1.2.3', '--build-number=4']`, with the intent that
/// these values will be forwarded to the `flutter build` command.
///
/// Because not all platform types support both --build-name and
/// --build-number, this needs to be handled in the platform-specific
/// patchers instead of at the patch command level.
///
/// We do this because some platforms encode the build version in their
/// binaries (Android does this with .dex files). If a release and a patch
/// have different version numbers, our [PatchDiffChecker] to warn the user of
/// native changes, even though the user may not have actually changed any
/// code or dependencies.
///
/// Context: https://github.com/shorebirdtech/shorebird/issues/2270
List<String> buildNameAndNumberArgsFromReleaseVersion(
String? releaseVersion,
) {
if (releaseVersion == null || !releaseVersion.contains('+')) {
return [];
}
// If the user already provided --build-name or --build-number, we don't
// want to override them.
if (argResults.rest.any(
(a) => a.startsWith('--build-name') || a.startsWith('--build-number'),
)) {
return [];
}
final parts = releaseVersion.split('+');
return [
'--build-name=${parts[0]}',
'--build-number=${parts[1]}',
];
}
}
@@ -334,6 +334,26 @@ Looked in:
});
});
group('when releaseVersion is provided', () {
setUp(setUpProjectRootArtifacts);
test('forwards --build-name and --build-number to builder', () async {
await runWithOverrides(
() => patcher.buildPatchArtifact(releaseVersion: '1.2.3+4'),
);
verify(
() => artifactBuilder.buildAppBundle(
flavor: any(named: 'flavor'),
target: any(named: 'target'),
args: any(
named: 'args',
that: containsAll(['--build-name=1.2.3', '--build-number=4']),
),
),
).called(1);
});
});
group('when build succeeds', () {
setUp(setUpProjectRootArtifacts);
@@ -674,6 +674,29 @@ For more information see: $supportedVersionsLink''',
);
});
group('when releaseVersion is provided', () {
test('forwards --build-name and --build-number to builder',
() async {
await runWithOverrides(
() => patcher.buildPatchArtifact(releaseVersion: '1.2.3+4'),
);
verify(
() => artifactBuilder.buildIpa(
flavor: any(named: 'flavor'),
exportOptionsPlist: any(named: 'exportOptionsPlist'),
codesign: any(named: 'codesign'),
target: any(named: 'target'),
args: any(
named: 'args',
that: containsAll(
['--build-name=1.2.3', '--build-number=4'],
),
),
),
).called(1);
});
});
group('when platform was specified via arg results rest', () {
setUp(() {
when(() => argResults.rest).thenReturn(['ios', '--verbose']);
@@ -225,7 +225,9 @@ void main() {
() => patcher.extractReleaseVersionFromArtifact(any()),
).thenAnswer((_) async => releaseVersion);
when(
() => patcher.buildPatchArtifact(),
() => patcher.buildPatchArtifact(
releaseVersion: any(named: 'releaseVersion'),
),
).thenAnswer((_) async => File(''));
when(() => patcher.releaseType).thenReturn(ReleaseType.android);
when(() => patcher.primaryReleaseArtifactArch).thenReturn('aab');
@@ -580,7 +582,7 @@ void main() {
arch: patcher.primaryReleaseArtifactArch,
platform: releasePlatform,
),
() => patcher.buildPatchArtifact(),
() => patcher.buildPatchArtifact(releaseVersion: releaseVersion),
() => patcher.assertUnpatchableDiffs(
releaseArtifact: any(named: 'releaseArtifact'),
releaseArchive: any(named: 'releaseArchive'),
@@ -730,7 +732,7 @@ void main() {
() => shorebirdEnv.copyWith(
flutterRevisionOverride: releaseFlutterRevision,
),
() => patcher.buildPatchArtifact(),
() => patcher.buildPatchArtifact(releaseVersion: releaseVersion),
]);
});
@@ -1,5 +1,7 @@
import 'dart:io';
import 'package:args/args.dart';
import 'package:mocktail/mocktail.dart';
import 'package:shorebird_cli/src/code_push_client_wrapper.dart';
import 'package:shorebird_cli/src/commands/commands.dart';
import 'package:shorebird_cli/src/patch_diff_checker.dart';
@@ -38,6 +40,76 @@ void main() {
);
});
});
group('buildNameAndNumberArgsFromReleaseVersion', () {
late ArgResults argResults;
setUp(() {
argResults = MockArgResults();
});
group('when releaseVersion is not specified', () {
test('returns an empty list', () {
expect(
_TestPatcher(
argResults: MockArgResults(),
flavor: null,
target: null,
).buildNameAndNumberArgsFromReleaseVersion(null),
isEmpty,
);
});
});
group('when an invalid --release-version is specified', () {
test('returns an empty list', () {
expect(
_TestPatcher(
argResults: argResults,
flavor: null,
target: null,
).buildNameAndNumberArgsFromReleaseVersion('invalid'),
isEmpty,
);
});
});
group('when a valid --release-version is specified', () {
group('when --build-name and --build-number are specified', () {
setUp(() {
when(() => argResults.rest).thenReturn([
'--build-name=foo',
'--build-number=42',
]);
});
test('returns an empty list', () {
expect(
_TestPatcher(
argResults: argResults,
flavor: null,
target: null,
).buildNameAndNumberArgsFromReleaseVersion('1.2.3+4'),
isEmpty,
);
});
});
group('when neither --build-name nor --build-number are specified', () {
test('returns --build-name and --build-number', () {
when(() => argResults.rest).thenReturn([]);
expect(
_TestPatcher(
argResults: argResults,
flavor: null,
target: null,
).buildNameAndNumberArgsFromReleaseVersion('1.2.3+4'),
equals(['--build-name=1.2.3', '--build-number=4']),
);
});
});
});
});
});
}
@@ -63,7 +135,7 @@ class _TestPatcher extends Patcher {
}
@override
Future<File> buildPatchArtifact() {
Future<File> buildPatchArtifact({String? releaseVersion}) {
throw UnimplementedError();
}