Use -z for git ls-files (#4368)

lib/src/command_runner.dart

@@ -203,7 +203,7 @@ class PubCommandRunner extends CommandRunner<int> implements PubTopLevel {
     final pubRoot = p.dirname(p.dirname(p.fromUri(Platform.script)));
     try {
       actualRev =
-          git.runSync(['rev-parse', 'HEAD'], workingDir: pubRoot).single;
+          git.runSync(['rev-parse', 'HEAD'], workingDir: pubRoot).trim();
     } on git.GitException catch (_) {
       // When building for Debian, pub isn't checked out via git.
       return;

lib/src/git.dart

@@ -8,6 +8,7 @@ library;
 import 'dart:async';
 import 'dart:convert';
 import 'dart:io';
+import 'dart:typed_data';
 
 import 'package:collection/collection.dart';
 import 'package:path/path.dart' as p;
@@ -25,18 +26,18 @@ class GitException implements ApplicationException {
   final List<String> args;
 
   /// The standard error emitted by git.
-  final String stderr;
+  final dynamic stderr;
 
   /// The standard out emitted by git.
-  final String stdout;
+  final dynamic stdout;
 
   /// The error code
   final int exitCode;
 
   @override
   String get message => 'Git error. Command: `git ${args.join(' ')}`\n'
-      'stdout: $stdout\n'
-      'stderr: $stderr\n'
+      'stdout: ${stdout is String ? stdout : '<binary>'}\n'
+      'stderr: ${stderr is String ? stderr : '<binary>'}\n'
       'exit code: $exitCode';
 
   GitException(Iterable<String> args, this.stdout, this.stderr, this.exitCode)
@@ -51,12 +52,13 @@ bool get isInstalled => command != null;
 
 /// Run a git process with [args] from [workingDir].
 ///
-/// Returns the stdout as a list of strings if it succeeded. Completes to an
-/// exception if it failed.
-Future<List<String>> run(
+/// Returns the stdout if it succeeded. Completes to ans exception if it failed.
+Future<String> run(
   List<String> args, {
   String? workingDir,
   Map<String, String>? environment,
+  Encoding stdoutEncoding = systemEncoding,
+  Encoding stderrEncoding = systemEncoding,
 }) async {
   if (!isInstalled) {
     fail('Cannot find a Git executable.\n'
@@ -70,12 +72,14 @@ Future<List<String>> run(
       args,
       workingDir: workingDir,
       environment: {...?environment, 'LANG': 'en_GB'},
+      stdoutEncoding: stdoutEncoding,
+      stderrEncoding: stderrEncoding,
     );
     if (!result.success) {
       throw GitException(
         args,
-        result.stdout.join('\n'),
-        result.stderr.join('\n'),
+        result.stdout,
+        result.stderr,
         result.exitCode,
       );
     }
@@ -86,12 +90,12 @@ Future<List<String>> run(
 }
 
 /// Like [run], but synchronous.
-List<String> runSync(
+String runSync(
   List<String> args, {
   String? workingDir,
   Map<String, String>? environment,
-  Encoding? stdoutEncoding = systemEncoding,
-  Encoding? stderrEncoding = systemEncoding,
+  Encoding stdoutEncoding = systemEncoding,
+  Encoding stderrEncoding = systemEncoding,
 }) {
   if (!isInstalled) {
     fail('Cannot find a Git executable.\n'
@@ -109,8 +113,39 @@ List<String> runSync(
   if (!result.success) {
     throw GitException(
       args,
-      result.stdout.join('\n'),
-      result.stderr.join('\n'),
+      result.stdout,
+      result.stderr,
+      result.exitCode,
+    );
+  }
+
+  return result.stdout;
+}
+
+/// Like [run], but synchronous. Returns raw stdout as `Uint8List`.
+Uint8List runSyncBytes(
+  List<String> args, {
+  String? workingDir,
+  Map<String, String>? environment,
+  Encoding stderrEncoding = systemEncoding,
+}) {
+  if (!isInstalled) {
+    fail('Cannot find a Git executable.\n'
+        'Please ensure Git is correctly installed.');
+  }
+
+  final result = runProcessSyncBytes(
+    command!,
+    args,
+    workingDir: workingDir,
+    environment: environment,
+    stderrEncoding: stderrEncoding,
+  );
+  if (!result.success) {
+    throw GitException(
+      args,
+      result.stdout,
+      result.stderr,
       result.exitCode,
     );
   }
@@ -128,7 +163,7 @@ String? repoRoot(String dir) {
   if (isInstalled) {
     try {
       return p.normalize(
-        runSync(['rev-parse', '--show-toplevel'], workingDir: dir).first,
+        runSync(['rev-parse', '--show-toplevel'], workingDir: dir).trim(),
       );
     } on GitException {
       // Not in a git folder.
@@ -152,17 +187,10 @@ bool _tryGitCommand(String command) {
     // Some users may have configured commands such as autorun, which may
     // produce additional output, so we need to look for "git version"
     // in every line of the output.
-    Match? match;
-    String? versionString;
-    for (var line in output) {
-      match = RegExp(r'^git version (\d+)\.(\d+)\.').matchAsPrefix(line);
-      if (match != null) {
-        versionString = line.substring('git version '.length);
-        break;
-      }
-    }
+    final match = RegExp(r'^git version (\d+)\.(\d+)\..*$', multiLine: true)
+        .matchAsPrefix(output);
     if (match == null) return false;
-
+    final versionString = match[0]!.substring('git version '.length);
     // Git seems to use many parts in the version number. We just check the
     // first two.
     final major = int.parse(match[1]!);

lib/src/io.dart

@@ -775,14 +775,14 @@ Future flushThenExit(int status) {
 /// The spawned process will inherit its parent's environment variables. If
 /// [environment] is provided, that will be used to augment (not replace) the
 /// the inherited variables.
-Future<PubProcessResult> runProcess(
+Future<StringProcessResult> runProcess(
   String executable,
   List<String> args, {
   String? workingDir,
   Map<String, String>? environment,
   bool runInShell = false,
-  Encoding? stdoutEncoding = systemEncoding,
-  Encoding? stderrEncoding = systemEncoding,
+  Encoding stdoutEncoding = systemEncoding,
+  Encoding stderrEncoding = systemEncoding,
 }) {
   ArgumentError.checkNotNull(executable, 'executable');
 
@@ -806,13 +806,12 @@ Future<PubProcessResult> runProcess(
       );
     }
 
-    final pubResult = PubProcessResult(
+    log.processResult(executable, result);
+    return StringProcessResult(
       result.stdout as String,
       result.stderr as String,
       result.exitCode,
     );
-    log.processResult(executable, pubResult);
-    return pubResult;
   });
 }
 
@@ -857,14 +856,14 @@ Future<PubProcess> startProcess(
 }
 
 /// Like [runProcess], but synchronous.
-PubProcessResult runProcessSync(
+StringProcessResult runProcessSync(
   String executable,
   List<String> args, {
   String? workingDir,
   Map<String, String>? environment,
   bool runInShell = false,
-  Encoding? stdoutEncoding = systemEncoding,
-  Encoding? stderrEncoding = systemEncoding,
+  Encoding stdoutEncoding = systemEncoding,
+  Encoding stderrEncoding = systemEncoding,
 }) {
   ArgumentError.checkNotNull(executable, 'executable');
   ProcessResult result;
@@ -883,13 +882,67 @@ PubProcessResult runProcessSync(
   } on IOException catch (e) {
     throw RunProcessException('Pub failed to run subprocess `$executable`: $e');
   }
-  final pubResult = PubProcessResult(
+  log.processResult(executable, result);
+  return StringProcessResult(
     result.stdout as String,
     result.stderr as String,
     result.exitCode,
   );
-  log.processResult(executable, pubResult);
-  return pubResult;
+}
+
+/// Like [runProcess], but synchronous.
+/// Always outputs stdout as `List<int>`.
+BytesProcessResult runProcessSyncBytes(
+  String executable,
+  List<String> args, {
+  String? workingDir,
+  Map<String, String>? environment,
+  bool runInShell = false,
+  Encoding stderrEncoding = systemEncoding,
+}) {
+  ProcessResult result;
+  try {
+    (executable, args) =
+        _sanitizeExecutablePath(executable, args, workingDir: workingDir);
+    result = Process.runSync(
+      executable,
+      args,
+      workingDirectory: workingDir,
+      environment: environment,
+      runInShell: runInShell,
+      stdoutEncoding: null,
+      stderrEncoding: stderrEncoding,
+    );
+  } on IOException catch (e) {
+    throw RunProcessException('Pub failed to run subprocess `$executable`: $e');
+  }
+  log.processResult(executable, result);
+  return BytesProcessResult(
+    result.stdout as List<int>,
+    result.stderr as String,
+    result.exitCode,
+  );
+}
+
+/// Adaptation of ProcessResult when stdout is a `List<String>`.
+class StringProcessResult {
+  final String stdout;
+  final String stderr;
+  final int exitCode;
+  StringProcessResult(this.stdout, this.stderr, this.exitCode);
+  bool get success => exitCode == exit_codes.SUCCESS;
+}
+
+/// Adaptation of ProcessResult when stdout is a `List<bytes>`.
+class BytesProcessResult {
+  final Uint8List stdout;
+  final String stderr;
+  final int exitCode;
+  BytesProcessResult(List<int> stdout, this.stderr, this.exitCode)
+      :
+        // Not clear that we need to do this, but seems harmless.
+        stdout = stdout is Uint8List ? stdout : Uint8List.fromList(stdout);
+  bool get success => exitCode == exit_codes.SUCCESS;
 }
 
 /// A wrapper around [Process] that exposes `dart:async`-style APIs.
@@ -1229,30 +1282,6 @@ ByteStream createTarGz(
   );
 }
 
-/// Contains the results of invoking a [Process] and waiting for it to complete.
-class PubProcessResult {
-  final List<String> stdout;
-  final List<String> stderr;
-  final int exitCode;
-
-  PubProcessResult(String stdout, String stderr, this.exitCode)
-      : stdout = _toLines(stdout),
-        stderr = _toLines(stderr);
-
-  // TODO(rnystrom): Remove this and change to returning one string.
-  static List<String> _toLines(String output) {
-    final lines = const LineSplitter().convert(output);
-
-    if (lines.isNotEmpty && lines.last == '') {
-      lines.removeLast();
-    }
-
-    return lines;
-  }
-
-  bool get success => exitCode == exit_codes.SUCCESS;
-}
-
 /// The location for dart-specific configuration.
 ///
 /// `null` if no config dir could be found.

lib/src/log.dart

@@ -251,18 +251,22 @@ void process(
 }
 
 /// Logs the results of running [executable].
-void processResult(String executable, PubProcessResult result) {
+void processResult(String executable, ProcessResult result) {
   // Log it all as one message so that it shows up as a single unit in the logs.
   final buffer = StringBuffer();
   buffer.writeln('Finished $executable. Exit code ${result.exitCode}.');
 
-  void dumpOutput(String name, List<String> output) {
+  void dumpOutput(String name, dynamic output) {
+    if (output is! String) {
+      buffer.writeln('Binary output on $name.');
+      return;
+    }
     if (output.isEmpty) {
       buffer.writeln('Nothing output on $name.');
     } else {
       buffer.writeln('$name:');
       var numLines = 0;
-      for (var line in output) {
+      for (var line in output.split('\n')) {
         if (++numLines > 1000) {
           buffer.writeln('[${output.length - 1000}] more lines of output '
               'truncated...]');

lib/src/source/git.dart

@@ -273,19 +273,15 @@ class GitSource extends CachedSource {
     final repoPath = _repoCachePath(description, cache);
     final revision = resolvedDescription.resolvedRef;
 
-    late List<String> lines;
     try {
-      // TODO(sigurdm): We should have a `git.run` alternative that gives back
-      // a stream of stdout instead of the lines.
-      lines = await git.run(
+      return await git.run(
         [_gitDirArg(repoPath), 'show', '$revision:$pathInCache'],
         workingDir: repoPath,
       );
     } on git.GitException catch (_) {
       fail('Could not find a file named "$pathInCache" in '
           '${GitDescription.prettyUri(description.url)} $revision.');
     }
-    return lines.join('\n');
   }
 
   @override
@@ -605,7 +601,7 @@ class GitSource extends CachedSource {
         [_gitDirArg(dirPath), 'rev-parse', '--is-inside-git-dir'],
         workingDir: dirPath,
       );
-      if (result.join('\n') != 'true') {
+      if (result.trim() != 'true') {
         isValid = false;
       }
     } on git.GitException {
@@ -655,21 +651,22 @@ class GitSource extends CachedSource {
   ///
   /// This assumes that the canonical clone already exists.
   Future<String> _firstRevision(String path, String reference) async {
-    final List<String> lines;
+    final String output;
     try {
-      lines = await git.run(
+      output = (await git.run(
         [_gitDirArg(path), 'rev-list', '--max-count=1', reference],
         workingDir: path,
-      );
+      ))
+          .trim();
     } on git.GitException catch (e) {
       throw PackageNotFoundException(
         "Could not find git ref '$reference' (${e.stderr})",
       );
     }
-    if (lines.isEmpty) {
+    if (output.isEmpty) {
       throw PackageNotFoundException("Could not find git ref '$reference'.");
     }
-    return lines.first;
+    return output;
   }
 
   /// Clones the repo at the URI [from] to the path [to] on the local