@matheusbsbs / matheus-bagliano / commits / 5f393dc3aa
Fork 0

rm: support the --pathspec-from-file option

Decisions taken for simplicity: 1) It is not allowed to pass pathspec in both args and file. Adjustments were needed for `if (!argc)` block: This code actually means "pathspec is not present". Previously, pathspec could only come from commandline arguments, so testing for `argc` was a valid way of testing for the presence of pathspec. But this is no longer true with `--pathspec-from-file`. During the entire `--pathspec-from-file` story, I tried to keep its behavior very close to giving pathspec on commandline, so that switching from one to another doesn't involve any surprises. However, throwing usage at user in the case of empty `--pathspec-from-file` would puzzle because there's nothing wrong with "usage" (that is, argc/argv array). On the other hand, throwing usage in the old case also feels bad to me. While it's less of a puzzle, I (as user) never liked the experience of comparing my commandline to "usage", trying to spot a difference. Since it's already known what the error is, it feels a lot better to give that specific error to user. Judging from [1] it doesn't seem that showing usage in this case was important (the patch was to avoid segfault), and it doesn't fit into how other commands react to empty pathspec (see for example `git add` with a custom message). Therefore, I decided to show new error text in both cases. In order to continue testing for error early, I moved `parse_pathspec()` higher. Now it happens before `read_cache()` / `hold_locked_index()` / `setup_work_tree()`, which shouldn't cause any issues. [1] Commit 7612a1ef ("git-rm: honor -n flag" 2006-06-09) Signed-off-by: Alexandr Miloslavskiy <alexandr.miloslavskiy@syntevo.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Alexandr Miloslavskiy committed Feb 17, 2020 at 17:25 UTC 5f393dc3aa62ba3e13a9890af4e769ad84812fbc
3 files changed +117 -7
Documentation/git-rm.txt
+16 -1
@@ -8,7 +8,9 @@ git-rm - Remove files from the working tree and from the index
8 SYNOPSIS
9 --------
10 [verse]
11 -'git rm' [-f | --force] [-n] [-r] [--cached] [--ignore-unmatch] [--quiet] [--] <pathspec>...
11 +'git rm' [-f | --force] [-n] [-r] [--cached] [--ignore-unmatch]
12 + [--quiet] [--pathspec-from-file=<file> [--pathspec-file-nul]]
13 + [--] [<pathspec>...]
14
15 DESCRIPTION
16 -----------
@@ -73,6 +75,19 @@ For more details, see the 'pathspec' entry in linkgit:gitglossary[7].
75 `git rm` normally outputs one line (in the form of an `rm` command)
76 for each file removed. This option suppresses that output.
77
78 +--pathspec-from-file=<file>::
79 + Pathspec is passed in `<file>` instead of commandline args. If
80 + `<file>` is exactly `-` then standard input is used. Pathspec
81 + elements are separated by LF or CR/LF. Pathspec elements can be
82 + quoted as explained for the configuration variable `core.quotePath`
83 + (see linkgit:git-config[1]). See also `--pathspec-file-nul` and
84 + global `--literal-pathspecs`.
85 +
86 +--pathspec-file-nul::
87 + Only meaningful with `--pathspec-from-file`. Pathspec elements are
88 + separated with NUL character and all other characters are taken
89 + literally (including newlines and quotes).
90 +
91
92 REMOVING FILES THAT HAVE DISAPPEARED FROM THE FILESYSTEM
93 --------------------------------------------------------
builtin/rm.c
+22 -6
@@ -235,7 +235,8 @@ static int check_local_mod(struct object_id *head, int index_only)
235 }
236
237 static int show_only = 0, force = 0, index_only = 0, recursive = 0, quiet = 0;
238 -static int ignore_unmatch = 0;
238 +static int ignore_unmatch = 0, pathspec_file_nul;
239 +static char *pathspec_from_file;
240
241 static struct option builtin_rm_options[] = {
242 OPT__DRY_RUN(&show_only, N_("dry run")),
@@ -245,6 +246,8 @@ static struct option builtin_rm_options[] = {
246 OPT_BOOL('r', NULL, &recursive, N_("allow recursive removal")),
247 OPT_BOOL( 0 , "ignore-unmatch", &ignore_unmatch,
248 N_("exit with a zero status even if nothing matched")),
249 + OPT_PATHSPEC_FROM_FILE(&pathspec_from_file),
250 + OPT_PATHSPEC_FILE_NUL(&pathspec_file_nul),
251 OPT_END(),
252 };
253
@@ -259,8 +262,24 @@ int cmd_rm(int argc, const char **argv, const char *prefix)
262
263 argc = parse_options(argc, argv, prefix, builtin_rm_options,
264 builtin_rm_usage, 0);
262 - if (!argc)
263 - usage_with_options(builtin_rm_usage, builtin_rm_options);
265 +
266 + parse_pathspec(&pathspec, 0,
267 + PATHSPEC_PREFER_CWD,
268 + prefix, argv);
269 +
270 + if (pathspec_from_file) {
271 + if (pathspec.nr)
272 + die(_("--pathspec-from-file is incompatible with pathspec arguments"));
273 +
274 + parse_pathspec_file(&pathspec, 0,
275 + PATHSPEC_PREFER_CWD,
276 + prefix, pathspec_from_file, pathspec_file_nul);
277 + } else if (pathspec_file_nul) {
278 + die(_("--pathspec-file-nul requires --pathspec-from-file"));
279 + }
280 +
281 + if (!pathspec.nr)
282 + die(_("No pathspec was given. Which files should I remove?"));
283
284 if (!index_only)
285 setup_work_tree();
@@ -270,9 +289,6 @@ int cmd_rm(int argc, const char **argv, const char *prefix)
289 if (read_cache() < 0)
290 die(_("index file corrupt"));
291
273 - parse_pathspec(&pathspec, 0,
274 - PATHSPEC_PREFER_CWD,
275 - prefix, argv);
292 refresh_index(&the_index, REFRESH_QUIET|REFRESH_UNMERGED, &pathspec, NULL, NULL);
293
294 seen = xcalloc(pathspec.nr, 1);
t/t3601-rm-pathspec-file.sh new
+79
@@ -0,0 +1,79 @@
1 +#!/bin/sh
2 +
3 +test_description='rm --pathspec-from-file'
4 +
5 +. ./test-lib.sh
6 +
7 +test_tick
8 +
9 +test_expect_success setup '
10 + echo A >fileA.t &&
11 + echo B >fileB.t &&
12 + echo C >fileC.t &&
13 + echo D >fileD.t &&
14 + git add fileA.t fileB.t fileC.t fileD.t &&
15 + git commit -m "files" &&
16 +
17 + git tag checkpoint
18 +'
19 +
20 +restore_checkpoint () {
21 + git reset --hard checkpoint
22 +}
23 +
24 +verify_expect () {
25 + git status --porcelain --untracked-files=no -- fileA.t fileB.t fileC.t fileD.t >actual &&
26 + test_cmp expect actual
27 +}
28 +
29 +test_expect_success 'simplest' '
30 + restore_checkpoint &&
31 +
32 + cat >expect <<-\EOF &&
33 + D fileA.t
34 + EOF
35 +
36 + echo fileA.t | git rm --pathspec-from-file=- &&
37 + verify_expect
38 +'
39 +
40 +test_expect_success '--pathspec-file-nul' '
41 + restore_checkpoint &&
42 +
43 + cat >expect <<-\EOF &&
44 + D fileA.t
45 + D fileB.t
46 + EOF
47 +
48 + printf "fileA.t\0fileB.t\0" | git rm --pathspec-from-file=- --pathspec-file-nul &&
49 + verify_expect
50 +'
51 +
52 +test_expect_success 'only touches what was listed' '
53 + restore_checkpoint &&
54 +
55 + cat >expect <<-\EOF &&
56 + D fileB.t
57 + D fileC.t
58 + EOF
59 +
60 + printf "fileB.t\nfileC.t\n" | git rm --pathspec-from-file=- &&
61 + verify_expect
62 +'
63 +
64 +test_expect_success 'error conditions' '
65 + restore_checkpoint &&
66 + echo fileA.t >list &&
67 +
68 + test_must_fail git rm --pathspec-from-file=list -- fileA.t 2>err &&
69 + test_i18ngrep -e "--pathspec-from-file is incompatible with pathspec arguments" err &&
70 +
71 + test_must_fail git rm --pathspec-file-nul 2>err &&
72 + test_i18ngrep -e "--pathspec-file-nul requires --pathspec-from-file" err &&
73 +
74 + >empty_list &&
75 + test_must_fail git rm --pathspec-from-file=empty_list 2>err &&
76 + test_i18ngrep -e "No pathspec was given. Which files should I remove?" err
77 +'
78 +
79 +test_done