Use `-z` for `git ls-files` (#4368)
diff --git a/lib/src/command_runner.dart b/lib/src/command_runner.dart index 03c724b..e089aad 100644 --- a/lib/src/command_runner.dart +++ b/lib/src/command_runner.dart
@@ -203,7 +203,7 @@ 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;
diff --git a/lib/src/git.dart b/lib/src/git.dart index 1087034..8bbbb53 100644 --- a/lib/src/git.dart +++ b/lib/src/git.dart
@@ -8,6 +8,7 @@ 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 @@ 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 @@ /// 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 @@ 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 @@ } /// 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 @@ 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 @@ 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 @@ // 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]!);
diff --git a/lib/src/io.dart b/lib/src/io.dart index ab980f5..136b943 100644 --- a/lib/src/io.dart +++ b/lib/src/io.dart
@@ -775,14 +775,14 @@ /// 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 @@ ); } - 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 @@ } /// 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 @@ } 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 @@ ); } -/// 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.
diff --git a/lib/src/log.dart b/lib/src/log.dart index f8864fb..87ccb7c 100644 --- a/lib/src/log.dart +++ b/lib/src/log.dart
@@ -251,18 +251,22 @@ } /// 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...]');
diff --git a/lib/src/source/git.dart b/lib/src/source/git.dart index bbe4d21..54f4dd2 100644 --- a/lib/src/source/git.dart +++ b/lib/src/source/git.dart
@@ -273,11 +273,8 @@ 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, ); @@ -285,7 +282,6 @@ fail('Could not find a file named "$pathInCache" in ' '${GitDescription.prettyUri(description.url)} $revision.'); } - return lines.join('\n'); } @override @@ -605,7 +601,7 @@ [_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 @@ /// /// 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
diff --git a/lib/src/validator/analyze.dart b/lib/src/validator/analyze.dart index 839db28..3e1cb00 100644 --- a/lib/src/validator/analyze.dart +++ b/lib/src/validator/analyze.dart
@@ -32,7 +32,7 @@ ['analyze', ...entries, p.join(package.dir, 'pubspec.yaml')], ); if (result.exitCode != 0) { - final limitedOutput = limitLength(result.stdout.join('\n'), 1000); + final limitedOutput = limitLength(result.stdout, 1000); warnings .add('`dart analyze` found the following issue(s):\n$limitedOutput'); }
diff --git a/lib/src/validator/gitignore.dart b/lib/src/validator/gitignore.dart index 253834e..630a1dd 100644 --- a/lib/src/validator/gitignore.dart +++ b/lib/src/validator/gitignore.dart
@@ -5,6 +5,7 @@ import 'dart:async'; import 'dart:convert'; import 'dart:io'; +import 'dart:typed_data'; import 'package:path/path.dart' as p; @@ -22,19 +23,19 @@ @override Future<void> validate() async { if (package.inGitRepo) { - late final List<String> checkedIntoGit; + final Uint8List output; try { - checkedIntoGit = git.runSync( + output = git.runSyncBytes( [ '-c', 'core.quotePath=false', 'ls-files', + '-z', '--cached', '--exclude-standard', '--recurse-submodules', ], workingDir: package.dir, - stdoutEncoding: const Utf8Codec(), ); } on git.GitException catch (e) { log.fine('Could not run `git ls-files` files in repo (${e.message}).'); @@ -43,6 +44,17 @@ // --recurse-submodules we just continue silently. return; } + final checkedIntoGit = <String>[]; + // Split at \0. + var start = 0; + for (var i = 0; i < output.length; i++) { + if (output[i] == 0) { + checkedIntoGit.add( + utf8.decode(Uint8List.sublistView(output, start, i)), + ); + start = i + 1; + } + } final root = git.repoRoot(package.dir) ?? package.dir; var beneath = p.posix.joinAll( p.split(p.normalize(p.relative(package.dir, from: root))),
diff --git a/test/descriptor/git.dart b/test/descriptor/git.dart index addde73..95ae0b2 100644 --- a/test/descriptor/git.dart +++ b/test/descriptor/git.dart
@@ -46,15 +46,16 @@ /// [parent] defaults to [sandbox]. Future<String> revParse(String ref, [String? parent]) async { final output = await _runGit(['rev-parse', ref], parent); - return output[0]; + return (output as String).trim(); } /// Runs a Git command in this repository. /// /// [parent] defaults to [sandbox]. - Future runGit(List<String> args, [String? parent]) => _runGit(args, parent); + Future<void> runGit(List<String> args, [String? parent]) => + _runGit(args, parent); - Future<List<String>> _runGit(List<String> args, String? parent) { + Future<dynamic> _runGit(List<String> args, String? parent) { // Explicitly specify the committer information. Git needs this to commit // and we don't want to rely on the buildbots having this already set up. final environment = {