From a4a42c548b2bd3d5988cddd1b153b5f2eca67048 Mon Sep 17 00:00:00 2001 From: godofredoc <54371434+godofredoc@users.noreply.github.com> Date: Tue, 14 Apr 2020 12:57:32 -0700 Subject: [PATCH] Update package server to use the port file. (#137) * Update package server to use the port file. This is to simplify the package server removing the logic to get the port from stdout. https://github.com/flutter/flutter/issues/43285 * Add missing dash to flag. * Import path with path alias. --- .../fuchsia_ctl/lib/src/package_server.dart | 42 +++++++++++++------ .../fuchsia_ctl/test/package_server_test.dart | 34 +++++++++++---- 2 files changed, 57 insertions(+), 19 deletions(-) diff --git a/packages/fuchsia_ctl/lib/src/package_server.dart b/packages/fuchsia_ctl/lib/src/package_server.dart index fd36fff2ad..7a41fca5a3 100644 --- a/packages/fuchsia_ctl/lib/src/package_server.dart +++ b/packages/fuchsia_ctl/lib/src/package_server.dart @@ -6,8 +6,12 @@ import 'dart:async'; import 'dart:convert'; import 'dart:io'; +import 'package:file/file.dart'; +import 'package:file/local.dart'; import 'package:fuchsia_ctl/fuchsia_ctl.dart'; import 'package:process/process.dart'; +import 'package:path/path.dart' as path; +import 'package:uuid/uuid.dart'; /// A wrapper around the Fuchsia SDK `pm` tool. class PackageServer { @@ -15,8 +19,10 @@ class PackageServer { PackageServer( this.pmPath, { this.processManager = const LocalProcessManager(), + this.fileSystem = const LocalFileSystem(), }) : assert(pmPath != null), - assert(processManager != null); + assert(processManager != null), + assert(fileSystem != null); /// The path on the file system to the `pm` executable. final String pmPath; @@ -24,8 +30,14 @@ class PackageServer { /// The process manager to use for launching `pm`. final ProcessManager processManager; + /// The file sytem for the package server. + final FileSystem fileSystem; + Process _pmServerProcess; + /// Path to the port file generated by `pm`. + String portPath; + /// The port the server is listening on, if the server is running. /// /// Throws a [StateError] if accessed when the server is not running. @@ -84,33 +96,38 @@ class PackageServer { String repo, { String address = '', int port = 0, + String portFilePath, }) async { assert(repo != null); assert(port != null); + + final String uuid = Uuid().v4(); + portPath = portFilePath ?? + path.join(fileSystem.systemTempDirectory.path, '${uuid}_port.txt'); final List pmCommand = [ pmPath, 'serve', - '-repo', repo, // - '-l', '$address:$port', + '-repo', + repo, + '-l', + '$address:$port', + '-f', + portPath, ]; stdout.writeln('Running ${pmCommand.join(' ')}'); _pmServerProcess = await processManager.start(pmCommand); - final Completer serverPortCompleter = Completer(); + await Future.delayed(const Duration(seconds: 5), () async { + final String portString = await fileSystem.file(portPath).readAsString(); + _serverPort = int.parse(portString); + }); _pmServerProcess.stdout .transform(utf8.decoder) .transform(const LineSplitter()) - .listen((String line) { - if (line.contains('serving $repo at http://')) { - _serverPort = int.parse(line.substring(line.lastIndexOf(':') + 1)); - serverPortCompleter.complete(); - } - stdout.writeln(line); - }); + .listen(stdout.writeln); _pmServerProcess.stderr .transform(utf8.decoder) .transform(const LineSplitter()) .listen(stderr.writeln); - await serverPortCompleter.future; } /// Closes a running server. @@ -120,6 +137,7 @@ class PackageServer { if (_pmServerProcess == null) { throw StateError('Must call serveRepo before calling close.'); } + await fileSystem.file(portPath).delete(); _pmServerProcess.kill(); final int exitCode = await _pmServerProcess.exitCode; _pmServerProcess = null; diff --git a/packages/fuchsia_ctl/test/package_server_test.dart b/packages/fuchsia_ctl/test/package_server_test.dart index a8e99b0d7b..d9036e4186 100644 --- a/packages/fuchsia_ctl/test/package_server_test.dart +++ b/packages/fuchsia_ctl/test/package_server_test.dart @@ -5,6 +5,8 @@ import 'dart:io' show ProcessResult; import 'dart:math' show Random; +import 'package:file/file.dart'; +import 'package:file/memory.dart'; import 'package:fuchsia_ctl/fuchsia_ctl.dart'; import 'package:fuchsia_ctl/src/package_server.dart'; import 'package:fuchsia_ctl/src/operation_result.dart'; @@ -67,8 +69,10 @@ void main() { pmBin, 'publish', '-a', - '-repo', repoPath, // - '-f', farFile, + '-repo', + repoPath, + '-f', + farFile, ]); expect(result.success, true); }); @@ -80,8 +84,6 @@ void main() { 0, [ '', - 'YYYY-MM-DD HH:mm:ss [pm serve] serving $repoPath at http://:$randomPort', - 'YYYY-MM-DD HH:mm:ss [pm serve] 200 /', ], [''], ); @@ -90,14 +92,28 @@ void main() { return serverProcess; }); + final MemoryFileSystem fs = MemoryFileSystem(); + final PackageServer server = PackageServer( pmBin, processManager: processManager, + fileSystem: fs, ); expect(server.serving, false); + final File portFile = fs.file( + 'port.txt', + ) + ..create() + ..writeAsString( + randomPort.toString(), + ); - await server.serveRepo(repoPath, port: 0); + await server.serveRepo( + repoPath, + port: 0, + portFilePath: portFile.path, + ); expect(server.serving, true); final List capturedStartArgs = @@ -109,8 +125,12 @@ void main() { expect(capturedStartArgs, [ pmBin, 'serve', - '-repo', repoPath, // - '-l', ':0', + '-repo', + repoPath, + '-l', + ':0', + '-f', + 'port.txt', ]); expect(server.serverPort, randomPort);