Skip to content

Commit

Permalink
fix: old aot_tools binaries do not work (#1645)
Browse files Browse the repository at this point in the history
We were missing a bunch of tests which are now added.

Also fixed us to correctly fallback to an executable when
the .dill file is not present for a given flutter revision.
  • Loading branch information
eseidel authored Jan 17, 2024
1 parent 2c4711b commit 726dbe1
Show file tree
Hide file tree
Showing 6 changed files with 266 additions and 85 deletions.
78 changes: 55 additions & 23 deletions packages/shorebird_cli/lib/src/cache.dart
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ class CacheUpdateFailure implements Exception {
String toString() => 'CacheUpdateFailure: $message';
}

typedef ArchiveExtracter = Future<void> Function(
typedef ArchiveExtractor = Future<void> Function(
String archivePath,
String outputPath,
);
Expand Down Expand Up @@ -59,10 +59,11 @@ class Cache {
Cache({this.extractArchive = _defaultArchiveExtractor}) {
registerArtifact(PatchArtifact(cache: this, platform: platform));
registerArtifact(BundleToolArtifact(cache: this, platform: platform));
registerArtifact(AotToolsArtifact(cache: this, platform: platform));
registerArtifact(AotToolsDillArtifact(cache: this, platform: platform));
registerArtifact(AotToolsExeArtifact(cache: this, platform: platform));
}

final ArchiveExtracter extractArchive;
final ArchiveExtractor extractArchive;

void registerArtifact(CachedArtifact artifact) => _artifacts.add(artifact);

Expand Down Expand Up @@ -133,19 +134,21 @@ abstract class CachedArtifact {
final Cache cache;
final Platform platform;

/// The on-disk name of the artifact.
String get name;

String get storageUrl;

String get fileName;

/// Should the artifact be marked executable.
bool get isExecutable;

/// The URL from which the artifact can be downloaded.
String get storageUrl;

/// Whether the artifact is required for Shorebird to function.
/// If we fail to fetch it we will exit with an error.
bool get required => true;

Future<void> extractArtifact(http.ByteStream stream, String outputPath) {
final file = File(p.join(outputPath, fileName))
..createSync(recursive: true);
final file = File(p.join(outputPath, name))..createSync(recursive: true);
return stream.pipe(file.openWrite());
}

Expand Down Expand Up @@ -185,21 +188,18 @@ allowed to access $storageUrl.''',
if (!platform.isWindows && isExecutable) {
final result = await process.start(
'chmod',
['+x', p.join(location.path, fileName)],
['+x', p.join(location.path, name)],
);
await result.exitCode;
}
}
}

class AotToolsArtifact extends CachedArtifact {
AotToolsArtifact({required super.cache, required super.platform});
class AotToolsDillArtifact extends CachedArtifact {
AotToolsDillArtifact({required super.cache, required super.platform});

@override
String get name => 'aot-tools';

@override
String get fileName => 'aot-tools.dill';
String get name => 'aot-tools.dill';

@override
bool get isExecutable => false;
Expand All @@ -218,7 +218,45 @@ class AotToolsArtifact extends CachedArtifact {

@override
String get storageUrl =>
'${cache.storageBaseUrl}/${cache.storageBucket}/shorebird/${shorebirdEnv.shorebirdEngineRevision}/aot-tools.dill';
'${cache.storageBaseUrl}/${cache.storageBucket}/shorebird/${shorebirdEnv.shorebirdEngineRevision}/$name';
}

/// For a few revisions in Dec 2023, we distributed aot-tools as an executable.
/// Should be removed sometime after June 2024.
class AotToolsExeArtifact extends CachedArtifact {
AotToolsExeArtifact({required super.cache, required super.platform});

@override
String get name => 'aot-tools';

@override
bool get isExecutable => true;

/// The aot-tools are only available for revisions that support mixed-mode.
@override
bool get required => false;

@override
Directory get location => Directory(
p.join(
cache.getArtifactDirectory(name).path,
shorebirdEnv.shorebirdEngineRevision,
),
);

@override
String get storageUrl {
var artifactName = 'aot-tools-';
if (platform.isMacOS) {
artifactName += 'darwin-x64';
} else if (platform.isLinux) {
artifactName += 'linux-x64';
} else if (platform.isWindows) {
artifactName += 'windows-x64';
}

return '${cache.storageBaseUrl}/${cache.storageBucket}/shorebird/${shorebirdEnv.shorebirdEngineRevision}/$artifactName';
}
}

class PatchArtifact extends CachedArtifact {
Expand All @@ -227,9 +265,6 @@ class PatchArtifact extends CachedArtifact {
@override
String get name => 'patch';

@override
String get fileName => 'patch';

@override
bool get isExecutable => true;

Expand Down Expand Up @@ -265,9 +300,6 @@ class BundleToolArtifact extends CachedArtifact {
@override
String get name => 'bundletool.jar';

@override
String get fileName => 'bundletool.jar';

@override
bool get isExecutable => false;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,7 @@ Please re-run the release command for this version or create a new release.''');
return ExitCode.software.code;
}

// TODO use linker if available
// TODO(bryanoltman): use linker if available

final File patchFile;
if (await aotTools.isGeneratePatchDiffBaseSupported()) {
Expand Down
19 changes: 17 additions & 2 deletions packages/shorebird_cli/lib/src/executables/aot_tools.dart
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,21 @@ class AotTools {
artifact: ShorebirdArtifact.aotTools,
);

// Fallback behavior for older versions of shorebird where aot-tools was
// distributed as an executable.
final extension = p.extension(artifactPath);
if (extension != '.dill' && extension != '.dart') {
return process.run(
artifactPath,
command,
workingDirectory: workingDirectory,
);
}

// local engine versions use .dart and we distribute aot-tools as a .dill
return process.run(
shorebirdEnv.dartBinaryFile.path,
[artifactPath, ...command],
['run', artifactPath, ...command],
workingDirectory: workingDirectory,
);
}
Expand Down Expand Up @@ -69,8 +81,11 @@ class AotTools {
// "Unrecognized flags: dump_blobs"
final result = await _exec(
[
// TODO(eseidel): add a --help, or --version or some other way to
// get a non-zero exit code without needing to pass in a path to a
// snapshot. This shows up during verbose mode and is confusing.
'dump_blobs',
'--analyze-snapshot=nonexistent_analzye_snapshot',
'--analyze-snapshot=nonexistent_analyze_snapshot',
'--output=out',
'--snapshot=nonexistent_snapshot',
],
Expand Down
4 changes: 2 additions & 2 deletions packages/shorebird_cli/pubspec.lock
Original file line number Diff line number Diff line change
Expand Up @@ -293,10 +293,10 @@ packages:
dependency: "direct main"
description:
name: http
sha256: d4872660c46d929f6b8a9ef4e7a7eff7e49bbf0c4ec3f385ee32df5119175139
sha256: a2bbf9d017fcced29139daa8ed2bba4ece450ab222871df93ca9eec6f80c34ba
url: "https://pub.dev"
source: hosted
version: "1.1.2"
version: "1.2.0"
http_multi_server:
dependency: transitive
description:
Expand Down
Loading

0 comments on commit 726dbe1

Please sign in to comment.