diff --git a/lib/src/git.dart b/lib/src/git.dart index 8bbbb534e..2f0603f66 100644 --- a/lib/src/git.dart +++ b/lib/src/git.dart @@ -50,6 +50,37 @@ class GitException implements ApplicationException { /// 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 splitZeroTerminated(Uint8List output, {int skipPrefix = 0}) { + final result = []; + 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 fa204ec53..80c41a079 100644 --- a/lib/src/validator.dart +++ b/lib/src/validator.dart @@ -25,6 +25,7 @@ import 'validator/executable.dart'; 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 @@ abstract class Validator { 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 000000000..c060925a1 --- /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 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 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 630a1dd26..5d0ffb785 100644 --- a/lib/src/validator/gitignore.dart +++ b/lib/src/validator/gitignore.dart @@ -44,16 +44,15 @@ class GitignoreValidator extends Validator { // --recurse-submodules we just continue silently. return; } - final checkedIntoGit = []; - // 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 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 000000000..3dae02054 --- /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([])), []); + expect( + splitZeroTerminated(Uint8List.fromList([0])), + [Uint8List.fromList([])], + ); + + expect(splitZeroTerminated(Uint8List.fromList([1, 0, 1])), [ + Uint8List.fromList([1]), + ]); + expect( + splitZeroTerminated(Uint8List.fromList([2, 1, 0, 1, 0, 0])), + [ + Uint8List.fromList([2, 1]), + Uint8List.fromList([1]), + Uint8List.fromList([]), + ], + ); + expect( + splitZeroTerminated( + Uint8List.fromList([2, 1, 0, 1, 0, 2, 3, 0]), + skipPrefix: 1, + ), + [ + Uint8List.fromList([1]), + Uint8List.fromList([]), + Uint8List.fromList([3]), + ], + ); + expect( + () => splitZeroTerminated( + Uint8List.fromList([2, 1, 0, 1, 0, 0]), + skipPrefix: 1, + ), + throwsA(isA()), + ); + }); +} diff --git a/test/validator/git_status_test.dart b/test/validator/git_status_test.dart new file mode 100644 index 000000000..f92653a21 --- /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 expectValidation( + Matcher error, + int exitCode, { + List extraArgs = const [], + Map 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, + ); + }); +}