Validate git is clean when publishing (#4373)
diff --git a/lib/src/git.dart b/lib/src/git.dart index 8bbbb53..2f0603f 100644 --- a/lib/src/git.dart +++ b/lib/src/git.dart
@@ -50,6 +50,37 @@ /// Tests whether or not the git command-line app is available for use. bool get isInstalled => command != null; +/// Splits the [output] of a git -z command at \0. +/// +/// The first [skipPrefix] bytes of each substring will be ignored (useful for +/// `git status -z`). If there are not enough bytes to skip, throws a +/// [FormatException]. +List<Uint8List> splitZeroTerminated(Uint8List output, {int skipPrefix = 0}) { + final result = <Uint8List>[]; + var start = 0; + + for (var i = 0; i < output.length; i++) { + if (output[i] != 0) { + continue; + } + if (start + skipPrefix > i) { + throw FormatException('Substring too short for prefix at $start'); + } + result.add( + Uint8List.sublistView( + output, + // The first 3 bytes are the modification status. + // Skip those. + start + skipPrefix, + i, + ), + ); + + start = i + 1; + } + return result; +} + /// Run a git process with [args] from [workingDir]. /// /// Returns the stdout if it succeeded. Completes to ans exception if it failed.
diff --git a/lib/src/validator.dart b/lib/src/validator.dart index fa204ec..80c41a0 100644 --- a/lib/src/validator.dart +++ b/lib/src/validator.dart
@@ -25,6 +25,7 @@ import 'validator/file_case.dart'; import 'validator/flutter_constraint.dart'; import 'validator/flutter_plugin_format.dart'; +import 'validator/git_status.dart'; import 'validator/gitignore.dart'; import 'validator/leak_detection.dart'; import 'validator/license.dart'; @@ -143,6 +144,7 @@ FileCaseValidator(), AnalyzeValidator(), GitignoreValidator(), + GitStatusValidator(), PubspecValidator(), LicenseValidator(), NameValidator(),
diff --git a/lib/src/validator/git_status.dart b/lib/src/validator/git_status.dart new file mode 100644 index 0000000..c060925 --- /dev/null +++ b/lib/src/validator/git_status.dart
@@ -0,0 +1,96 @@ +// Copyright (c) 2024, the Dart project authors. Please see the AUTHORS file +// for details. 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:convert'; +import 'dart:typed_data'; + +import 'package:path/path.dart' as p; + +import '../git.dart' as git; +import '../log.dart' as log; +import '../utils.dart'; +import '../validator.dart'; + +/// A validator that validates that no checked in files are modified in git. +/// +/// Doesn't report on newly added files, as generated files might not be checked +/// in to git. +class GitStatusValidator extends Validator { + @override + Future<void> validate() async { + if (!package.inGitRepo) { + return; + } + final Uint8List output; + final String reporoot; + try { + final maybeReporoot = git.repoRoot(package.dir); + if (maybeReporoot == null) { + log.fine( + 'Could not determine the repository root from ${package.dir}.', + ); + // This validation is only a warning. + return; + } + reporoot = maybeReporoot; + output = git.runSyncBytes( + [ + 'status', + '-z', // Machine parsable + '--no-renames', // We don't care about renames. + + '--untracked-files=no', // Don't show untracked files. + ], + workingDir: package.dir, + ); + } on git.GitException catch (e) { + log.fine('Could not run `git status` files in repo (${e.message}).'); + // This validation is only a warning. + // If git is not supported on the platform, we just continue silently. + return; + } + final List<String> modifiedFiles; + try { + modifiedFiles = git + .splitZeroTerminated(output, skipPrefix: 3) + .map((bytes) { + try { + final filename = utf8.decode(bytes); + final fullPath = p.join(reporoot, filename); + if (!files.any((f) => p.equals(fullPath, f))) { + // File is not in the published set - ignore. + return null; + } + return p.relative(fullPath); + } on FormatException catch (e) { + // Filename is not utf8 - ignore. + log.fine('Cannot decode file name: $e'); + return null; + } + }) + .nonNulls + .toList(); + } on FormatException catch (e) { + // Malformed output from `git status`. Skip this validation. + log.fine('Malformed output from `git status -z`: $e'); + return; + } + if (modifiedFiles.isNotEmpty) { + warnings.add(''' +${modifiedFiles.length} checked-in ${pluralize('file', modifiedFiles.length)} ${modifiedFiles.length == 1 ? 'is' : 'are'} modified in git. + +Usually you want to publish from a clean git state. + +Consider committing these files or reverting the changes. + +Modified files: + +${modifiedFiles.take(10).map(p.relative).join('\n')} +${modifiedFiles.length > 10 ? '...\n' : ''} +Run `git status` for more information. +'''); + } + } +}
diff --git a/lib/src/validator/gitignore.dart b/lib/src/validator/gitignore.dart index 630a1dd..5d0ffb7 100644 --- a/lib/src/validator/gitignore.dart +++ b/lib/src/validator/gitignore.dart
@@ -44,16 +44,15 @@ // --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 List<String> checkedIntoGit; + try { + checkedIntoGit = git.splitZeroTerminated(output).map((b) { + return utf8.decode(b); + }).toList(); + } on FormatException catch (e) { + log.fine('Failed decoding git output. Skipping validation. $e.'); + return; } final root = git.repoRoot(package.dir) ?? package.dir; var beneath = p.posix.joinAll(
diff --git a/test/git_test.dart b/test/git_test.dart new file mode 100644 index 0000000..3dae020 --- /dev/null +++ b/test/git_test.dart
@@ -0,0 +1,48 @@ +// Copyright (c) 2024, the Dart project authors. Please see the AUTHORS file +// for details. 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:typed_data'; + +import 'package:pub/src/git.dart'; +import 'package:test/test.dart'; + +void main() { + test('splitZeroTerminated works', () { + expect(splitZeroTerminated(Uint8List.fromList([])), <Uint8List>[]); + expect( + splitZeroTerminated(Uint8List.fromList([0])), + <Uint8List>[Uint8List.fromList([])], + ); + + expect(splitZeroTerminated(Uint8List.fromList([1, 0, 1])), <Uint8List>[ + Uint8List.fromList([1]), + ]); + expect( + splitZeroTerminated(Uint8List.fromList([2, 1, 0, 1, 0, 0])), + <Uint8List>[ + Uint8List.fromList([2, 1]), + Uint8List.fromList([1]), + Uint8List.fromList([]), + ], + ); + expect( + splitZeroTerminated( + Uint8List.fromList([2, 1, 0, 1, 0, 2, 3, 0]), + skipPrefix: 1, + ), + <Uint8List>[ + Uint8List.fromList([1]), + Uint8List.fromList([]), + Uint8List.fromList([3]), + ], + ); + expect( + () => splitZeroTerminated( + Uint8List.fromList([2, 1, 0, 1, 0, 0]), + skipPrefix: 1, + ), + throwsA(isA<FormatException>()), + ); + }); +}
diff --git a/test/validator/git_status_test.dart b/test/validator/git_status_test.dart new file mode 100644 index 0000000..f92653a --- /dev/null +++ b/test/validator/git_status_test.dart
@@ -0,0 +1,233 @@ +// Copyright (c) 2020, the Dart project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +import 'package:path/path.dart' as p; +import 'package:pub/src/exit_codes.dart' as exit_codes; +import 'package:test/test.dart'; + +import '../descriptor.dart' as d; +import '../test_pub.dart'; + +Future<void> expectValidation( + Matcher error, + int exitCode, { + List<String> extraArgs = const [], + Map<String, String> environment = const {}, + String? workingDirectory, +}) async { + await runPub( + error: error, + args: ['publish', '--dry-run', ...extraArgs], + environment: environment, + workingDirectory: workingDirectory ?? d.path(appPath), + exitCode: exitCode, + ); +} + +void main() { + test( + 'should consider a package valid ' + 'if it contains no modified files (but contains a newly created one)', + () async { + await d.git('myapp', [ + ...d.validPackage().contents, + d.file('foo.txt', 'foo'), + d.file('.pubignore', 'bob.txt\n'), + d.file('bob.txt', 'bob'), + ]).create(); + + await d.dir('myapp', [ + d.file('bar.txt', 'bar'), // Create untracked file. + d.file('bob.txt', 'bob2'), // Modify pub-ignored file. + ]).create(); + + await expectValidation(contains('Package has 0 warnings.'), 0); + }); + + test('Warns if files are modified', () async { + await d.git('myapp', [ + ...d.validPackage().contents, + d.file('foo.txt', 'foo'), + ]).create(); + + await d.dir('myapp', [ + d.file('foo.txt', 'foo2'), + ]).create(); + + await expectValidation( + allOf([ + contains('Package has 1 warning.'), + contains( + ''' +* 1 checked-in file is modified in git. + + Usually you want to publish from a clean git state. + + Consider committing these files or reverting the changes. + + Modified files: + + foo.txt + + Run `git status` for more information.''', + ), + ]), + exit_codes.DATA, + ); + + // Stage but do not commit foo.txt. The warning should still be active. + await d.git('myapp').runGit(['add', 'foo.txt']); + await expectValidation( + allOf([ + contains('Package has 1 warning.'), + contains('foo.txt'), + ]), + exit_codes.DATA, + ); + await d.git('myapp').runGit(['commit', '-m', 'message']); + + await d.dir('myapp', [ + d.file('bar.txt', 'bar'), // Create untracked file. + d.file('bob.txt', 'bob2'), // Modify pub-ignored file. + ]).create(); + + // Stage untracked file, now the warning should be about that. + await d.git('myapp').runGit(['add', 'bar.txt']); + + await expectValidation( + allOf([ + contains('Package has 1 warning.'), + contains( + ''' +* 1 checked-in file is modified in git. + + Usually you want to publish from a clean git state. + + Consider committing these files or reverting the changes. + + Modified files: + + bar.txt + + Run `git status` for more information.''', + ), + ]), + exit_codes.DATA, + ); + }); + + test('Works with non-ascii unicode characters in file name', () async { + await d.git('myapp', [ + ...d.validPackage().contents, + d.file('non_ascii_и.txt', 'foo'), + d.file('non_ascii_и_ignored.txt', 'foo'), + d.file('.pubignore', 'non_ascii_и_ignored.txt'), + ]).create(); + await d.dir('myapp', [ + ...d.validPackage().contents, + d.file('non_ascii_и.txt', 'foo2'), + d.file('non_ascii_и_ignored.txt', 'foo2'), + ]).create(); + + await expectValidation( + allOf([ + contains('Package has 1 warning.'), + contains( + ''' +* 1 checked-in file is modified in git. + + Usually you want to publish from a clean git state. + + Consider committing these files or reverting the changes. + + Modified files: + + non_ascii_и.txt + + Run `git status` for more information.''', + ), + ]), + exit_codes.DATA, + ); + }); + + test('Works in workspace', () async { + await d.git(appPath, [ + d.libPubspec( + 'myapp', + '1.2.3', + extras: { + 'workspace': ['a'], + }, + sdk: '^3.5.0', + ), + d.dir('a', [ + ...d.validPackage().contents, + d.file('non_ascii_и.txt', 'foo'), + d.file('non_ascii_и_ignored.txt', 'foo'), + d.file('.pubignore', 'non_ascii_и_ignored.txt'), + ]), + ]).create(); + + await d.dir(appPath, [ + d.dir('a', [ + d.file('non_ascii_и.txt', 'foo2'), + d.file('non_ascii_и_ignored.txt', 'foo2'), + d.file('.pubignore', 'non_ascii_и_ignored.txt'), + ]), + ]).create(); + + await expectValidation( + workingDirectory: p.join( + d.sandbox, + appPath, + ), + extraArgs: ['-C', 'a'], + allOf([ + contains('Package has 1 warning.'), + contains( + ''' +* 1 checked-in file is modified in git. + + Usually you want to publish from a clean git state. + + Consider committing these files or reverting the changes. + + Modified files: + + a${p.separator}non_ascii_и.txt + + Run `git status` for more information.''', + ), + ]), + exit_codes.DATA, + ); + + await expectValidation( + workingDirectory: p.join( + d.sandbox, + appPath, + 'a', + ), + allOf([ + contains('Package has 1 warning.'), + contains( + ''' +* 1 checked-in file is modified in git. + + Usually you want to publish from a clean git state. + + Consider committing these files or reverting the changes. + + Modified files: + + non_ascii_и.txt + + Run `git status` for more information.''', + ), + ]), + exit_codes.DATA, + ); + }); +}