From 30525491dca450119ba4f95fda8164b6edefeaca Mon Sep 17 00:00:00 2001 From: Godofredo Contreras Date: Wed, 13 May 2020 16:44:40 -0700 Subject: [PATCH 1/2] Add timeout to tar command and exit if it doesn't succeed. Also remove the verbose option of the tar file to improve speed. Bug: https://github.com/flutter/flutter/issues/57176 --- packages/fuchsia_ctl/bin/main.dart | 7 ++++++- packages/fuchsia_ctl/lib/src/tar.dart | 4 ++-- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/packages/fuchsia_ctl/bin/main.dart b/packages/fuchsia_ctl/bin/main.dart index 3f5c03a38bc8..156284eb4b69 100644 --- a/packages/fuchsia_ctl/bin/main.dart +++ b/packages/fuchsia_ctl/bin/main.dart @@ -262,7 +262,12 @@ Future pushPackages( stdout.writeln('Untaring $repoArchive to ${repo.path}'); repo.createSync(recursive: true); - await tar.untar(repoArchive, repo.path); + final OperationResult result = await tar.untar(repoArchive, repo.path); + if (!result.success) { + stdout.writeln( + 'Error untarring $repoArchive \nstdout: ${result.info} \nstderr: ${result.error}'); + exit(-1); + } final String repositoryBase = path.join(repo.path, 'amber-files'); stdout.writeln('Serving $repositoryBase to $targetIp'); diff --git a/packages/fuchsia_ctl/lib/src/tar.dart b/packages/fuchsia_ctl/lib/src/tar.dart index 9ab2167f976b..f721f2b3bfed 100644 --- a/packages/fuchsia_ctl/lib/src/tar.dart +++ b/packages/fuchsia_ctl/lib/src/tar.dart @@ -42,9 +42,9 @@ class SystemTar implements Tar { ) async { final ProcessResult result = await processManager.run([ 'tar', - '-xvf', src, // + '-xf', src, // '-C', destination, - ]); + ]).timeout(const Duration(minutes: 10)); return OperationResult.fromProcessResult(result); } From 76cedb57062f6c902c73a1728b4e963ce501a962 Mon Sep 17 00:00:00 2001 From: Godofredo Contreras Date: Wed, 13 May 2020 18:59:28 -0700 Subject: [PATCH 2/2] Add tests and reduce timeout to 5 mins. --- packages/fuchsia_ctl/lib/src/tar.dart | 17 ++++++--- .../fuchsia_ctl/test/image_paver_test.dart | 3 +- packages/fuchsia_ctl/test/tar_test.dart | 35 +++++++++++++++++++ 3 files changed, 50 insertions(+), 5 deletions(-) create mode 100644 packages/fuchsia_ctl/test/tar_test.dart diff --git a/packages/fuchsia_ctl/lib/src/tar.dart b/packages/fuchsia_ctl/lib/src/tar.dart index f721f2b3bfed..0a55442a6d77 100644 --- a/packages/fuchsia_ctl/lib/src/tar.dart +++ b/packages/fuchsia_ctl/lib/src/tar.dart @@ -17,7 +17,11 @@ abstract class Tar { const Tar(); /// Untars a tar file. - Future untar(String src, String destination); + Future untar( + String src, + String destination, { + Duration timeoutMs, + }); } /// The archive package is very slow and memory intensive. Use @@ -35,16 +39,21 @@ class SystemTar implements Tar { /// program. final ProcessManager processManager; + /// The default timeout for untar operations as [Duration] in milliseconds. + static const Duration defaultTarTimeoutMs = + Duration(milliseconds: 5 * 60 * 1000); + @override Future untar( String src, - String destination, - ) async { + String destination, { + Duration timeoutMs = defaultTarTimeoutMs, + }) async { final ProcessResult result = await processManager.run([ 'tar', '-xf', src, // '-C', destination, - ]).timeout(const Duration(minutes: 10)); + ]).timeout(timeoutMs); return OperationResult.fromProcessResult(result); } diff --git a/packages/fuchsia_ctl/test/image_paver_test.dart b/packages/fuchsia_ctl/test/image_paver_test.dart index 38ca0e52c912..f5cc0818be06 100644 --- a/packages/fuchsia_ctl/test/image_paver_test.dart +++ b/packages/fuchsia_ctl/test/image_paver_test.dart @@ -158,7 +158,8 @@ class FakeTar implements Tar { final MemoryFileSystem fs; @override - Future untar(String src, String destination) async { + Future untar(String src, String destination, + {Duration timeoutMs}) async { if (passes) { final Directory dir = fs.directory(destination) ..createSync(recursive: true); diff --git a/packages/fuchsia_ctl/test/tar_test.dart b/packages/fuchsia_ctl/test/tar_test.dart new file mode 100644 index 000000000000..98581b44e440 --- /dev/null +++ b/packages/fuchsia_ctl/test/tar_test.dart @@ -0,0 +1,35 @@ +// Copyright 2020 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +import 'dart:async'; +import 'dart:io'; + +import 'package:process/process.dart'; +import 'package:test/test.dart'; +import 'package:fuchsia_ctl/src/tar.dart'; +import 'package:mockito/mockito.dart'; + +void main() { + group('Tar', () { + test('Untar file times out', () async { + final MockProcessManager processManager = MockProcessManager(); + + when(processManager.run(any)).thenAnswer((_) async { + await Future.delayed(const Duration(milliseconds: 3)); + return ProcessResult(0, 0, 'Good job', ''); + }); + + final Tar tar = SystemTar(processManager: processManager); + expect( + tar.untar( + 'source.tar', + '/destination', + timeoutMs: const Duration(milliseconds: 1), + ), + throwsA(const TypeMatcher())); + }); + }); +} + +class MockProcessManager extends Mock implements ProcessManager {}