[CLI] Account for SDK version changes that happen between when resident frontend compilers are started and when they are used
TEST=test case added to pkg/dartdev/test/commands/run_test.dart
Issue: https://github.com/dart-lang/sdk/issues/54245
Fixes: https://github.com/dart-lang/sdk/issues/55349
Change-Id: Ic3db064d0975e6b999beefed8b6fc32d2c603cff
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/362522
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Reviewed-by: Ben Konyi <bkonyi@google.com>
Commit-Queue: Derek Xu <derekx@google.com>
diff --git a/pkg/dartdev/lib/src/generate_kernel.dart b/pkg/dartdev/lib/src/generate_kernel.dart
index c681e9e..00b4fc0 100644
--- a/pkg/dartdev/lib/src/generate_kernel.dart
+++ b/pkg/dartdev/lib/src/generate_kernel.dart
@@ -5,6 +5,7 @@
import 'dart:io';
import 'package:args/args.dart';
+import 'package:kernel/binary/tag.dart' show isValidSdkHash;
import 'package:path/path.dart' as p;
import 'package:pub/pub.dart';
@@ -131,7 +132,21 @@
File serverInfoFile,
) async {
if (serverInfoFile.existsSync()) {
- return;
+ final residentCompilerInfo = ResidentCompilerInfo.fromFile(serverInfoFile);
+ if (residentCompilerInfo.sdkHash != null &&
+ isValidSdkHash(residentCompilerInfo.sdkHash!)) {
+ // There is already a Resident Frontend Compiler associated with
+ // [serverInfoFile] that is running and compatible with the Dart SDK that
+ // the user is currently using.
+ return;
+ } else {
+ log.stderr(
+ 'The Dart SDK has been upgraded or downgraded since the Resident '
+ 'Frontend Compiler was started, so the Resident Frontend Compiler will '
+ 'now be restarted for compatibility reasons.',
+ );
+ await shutDownOrForgetResidentFrontendCompiler(serverInfoFile);
+ }
}
try {
Directory(p.dirname(serverInfoFile.path)).createSync(recursive: true);
diff --git a/pkg/dartdev/lib/src/resident_frontend_utils.dart b/pkg/dartdev/lib/src/resident_frontend_utils.dart
index 839b085..0fb6830 100644
--- a/pkg/dartdev/lib/src/resident_frontend_utils.dart
+++ b/pkg/dartdev/lib/src/resident_frontend_utils.dart
@@ -52,9 +52,14 @@
final String packageConfigName = p.join('.dart_tool', 'package_config.json');
final class ResidentCompilerInfo {
+ final String? _sdkHash;
final InternetAddress _address;
final int _port;
+ /// The SDK hash that kernel files compiled using the Resident Frontend
+ /// Compiler associated with this object will be stamped with.
+ String? get sdkHash => _sdkHash;
+
/// The address that the Resident Frontend Compiler associated with this
/// object is listening from.
InternetAddress get address => _address;
@@ -72,6 +77,9 @@
final fileContents = file.readAsStringSync();
return ResidentCompilerInfo._(
+ sdkHash: fileContents.contains('sdkHash:')
+ ? _extractValueAssociatedWithKey(fileContents, 'sdkHash')
+ : null,
address: InternetAddress(
_extractValueAssociatedWithKey(fileContents, 'address'),
),
@@ -80,9 +88,11 @@
}
ResidentCompilerInfo._({
+ required String? sdkHash,
required int port,
required InternetAddress address,
- }) : _port = port,
+ }) : _sdkHash = sdkHash,
+ _port = port,
_address = address;
}
diff --git a/pkg/dartdev/pubspec.yaml b/pkg/dartdev/pubspec.yaml
index 723ee14..908f989 100644
--- a/pkg/dartdev/pubspec.yaml
+++ b/pkg/dartdev/pubspec.yaml
@@ -22,6 +22,7 @@
dtd_impl: any
front_end: any
http: any
+ kernel: any
logging: any
meta: any
native_assets_builder: any
diff --git a/pkg/dartdev/test/commands/run_test.dart b/pkg/dartdev/test/commands/run_test.dart
index b51f472..cf9b6db 100644
--- a/pkg/dartdev/test/commands/run_test.dart
+++ b/pkg/dartdev/test/commands/run_test.dart
@@ -8,6 +8,7 @@
import 'package:dartdev/src/resident_frontend_constants.dart';
import 'package:dartdev/src/resident_frontend_utils.dart';
+import 'package:kernel/binary/tag.dart' show sdkHashNull;
import 'package:path/path.dart' as path;
import 'package:pub_semver/pub_semver.dart';
import 'package:test/test.dart';
@@ -871,15 +872,20 @@
dartdevKernelCache,
)).listSync(),
isNotEmpty);
- });
- tearDown(() async {
- try {
- await sendAndReceiveResponse(
- residentServerShutdownCommand,
- File(path.join(serverInfoDirectory.dirPath, 'info')),
- );
- } catch (_) {}
+ // [TestProject] uses [addTearDown] to register cleanup code that will
+ // delete the project, and due to ordering guarantees of callbacks
+ // registered using [addTearDown] (refer to [addTearDown]'s doc comment), we
+ // need to use [addTearDown] here to ensure that the resident frontend
+ // compiler we started above will be shut down before the project is
+ // deleted.
+ addTearDown(() async {
+ await serverInfoDirectory.run([
+ 'compilation-server',
+ 'shutdown',
+ '--$residentCompilerInfoFileOption=$serverInfoFile',
+ ]);
+ });
});
test(
@@ -970,6 +976,60 @@
expect(kernelCache, isNotNull);
});
+ test(
+ 'a running resident compiler is restarted if the Dart SDK was '
+ 'upgraded or downgraded since it was started', () async {
+ p = project(mainSrc: 'void main() {}');
+
+ final residentCompilerInfo = ResidentCompilerInfo.fromFile(
+ File(serverInfoFile),
+ );
+ if (residentCompilerInfo.sdkHash != null &&
+ residentCompilerInfo.sdkHash == sdkHashNull) {
+ // dartdev does not consider the SDK hash changing from [sdkHashNull] to a
+ // different SDK hash to be an SDK upgrade or downgrade. So, if an SDK
+ // of [sdkHashNull] was recorded into [serverInfoFile], the test logic
+ // below will not work as intended. For local developement, we make the
+ // test fail in this situation, alerting the developer that the test is
+ // only meaningful when run from an SDK built with --verify-sdk-hash. The
+ // SDK can only be built with --no-verify-sdk-hash on CI, so we just skip
+ // this test on CI.
+ if (Platform.environment.containsKey('BUILDBOT_BUILDERNAME') ||
+ Platform.environment.containsKey('SWARMING_TASK_ID')) {
+ return;
+ } else {
+ fail(
+ 'This test is guaranteed to pass, and thus is not meaningful, when '
+ 'run from an SDK built with --no-verify-sdk-hash',
+ );
+ }
+ }
+
+ // Replace the SDK hash in [serverInfoFile] to make dartdev think the Dart
+ // SDK version has changed since the resident compiler was started.
+ File(serverInfoFile).writeAsStringSync(
+ 'address:${residentCompilerInfo.address.address} '
+ 'sdkHash:${'1' * residentCompilerInfo.sdkHash!.length} '
+ 'port:${residentCompilerInfo.port} ',
+ );
+ final result = await p.run([
+ 'run',
+ '--resident',
+ '--$residentCompilerInfoFileOption=$serverInfoFile',
+ p.relativeFilePath,
+ ]);
+
+ expect(result.exitCode, 0);
+ expect(result.stdout, contains(residentFrontendCompilerPrefix));
+ expect(
+ result.stderr,
+ 'The Dart SDK has been upgraded or downgraded since the Resident '
+ 'Frontend Compiler was started, so the Resident Frontend Compiler will '
+ 'now be restarted for compatibility reasons.\n',
+ );
+ expect(File(serverInfoFile).existsSync(), true);
+ });
+
test('when a connection to a running resident compiler cannot be established',
() async {
// When this occurs, the user should be informed that the resident frontend
@@ -979,7 +1039,9 @@
// that a connection will not be established.
final testServerInfoFile = File(path.join(p.dirPath, 'info'));
testServerInfoFile.createSync();
- testServerInfoFile.writeAsStringSync('address:127.0.0.1 port:-12 ');
+ testServerInfoFile.writeAsStringSync(
+ 'address:127.0.0.1 sdkHash:$sdkHashNull port:-12 ',
+ );
final result = await p.run([
'run',
'--resident',
diff --git a/pkg/frontend_server/lib/src/resident_frontend_server.dart b/pkg/frontend_server/lib/src/resident_frontend_server.dart
index 1a893df..99bb559 100644
--- a/pkg/frontend_server/lib/src/resident_frontend_server.dart
+++ b/pkg/frontend_server/lib/src/resident_frontend_server.dart
@@ -17,6 +17,7 @@
import 'package:args/args.dart';
import 'package:front_end/src/api_unstable/vm.dart';
+import 'package:kernel/binary/tag.dart' show expectedSdkHash;
import '../frontend_server.dart';
@@ -461,8 +462,21 @@
throw new StateError('A server is already running.');
}
server = await ServerSocket.bind(address, port);
+ // There are particular aspects of the info file format that must be
+ // preserved to ensure backwards compatibility with the original versions
+ // of the utilities for parsing this file.
+ //
+ // The aspects of the info file format that must be preserved are:
+ // 1. The file must begin with 'address:$address '. Note that $address IS
+ // NOT preceded by a space and IS followed by a space.
+ // 2. The file must end with 'port:$port'. Note that $port IS NOT preceded
+ // by a space. $port may be followed by zero or more whitespace
+ // characters.
serverInfoFile.writeAsStringSync(
- 'address:${server.address.address} port:${server.port}');
+ 'address:${server.address.address} '
+ 'sdkHash:${expectedSdkHash} '
+ 'port:${server.port} ',
+ );
} on StateError catch (e) {
print('Error: $e\n');
return null;
