worktree: send "chatty" messages to stderr
The order in which the stdout and stderr streams are flushed is not guaranteed to be the same across platforms or `libc` implementations. This lack of determinism can lead to anomalous and potentially confusing output if normal (stdout) output is flushed after error (stderr) output. For instance, the following output which clearly indicates a failure due to a fatal error: % git worktree add ../foo bar Preparing worktree (checking out 'bar') fatal: 'bar' is already checked out at '.../wherever' has been reported[1] on Microsoft Windows to appear as: % git worktree add ../foo bar fatal: 'bar' is already checked out at '.../wherever' Preparing worktree (checking out 'bar') which may confuse the reader into thinking that the command somehow recovered and ran to completion despite the error. This problem crops up because the "chatty" status message "Preparing worktree" is sent to stdout, whereas the "fatal" error message is sent to stderr. One way to fix this would be to flush stdout manually before git-worktree reports any errors to stderr. However, common practice in Git is for "chatty" messages to be sent to stderr. Therefore, a more appropriate fix is to adjust git-worktree to conform to that practice by sending its "chatty" messages to stderr rather than stdout as is currently the case. There may be concern that relocating messages from stdout to stderr could break existing tooling, however, these messages are already internationalized, thus are unstable. And, indeed, the "Preparing worktree" message has already been the subject of somewhat significant changes in 2c27002a0a (worktree: improve message when creating a new worktree, 2018-04-24). Moreover, there is existing precedent, such as 68b939b2f0 (clone: send diagnostic messages to stderr, 2013-09-18) which likewise relocated "chatty" messages from stdout to stderr for git-clone. [1]: https://lore.kernel.org/git/CA+34VNLj6VB1kCkA=MfM7TZR+6HgqNi5-UaziAoCXacSVkch4A@mail.gmail.com/T/ Reported-by: Baruch Burstein <bmburstein@gmail.com> Signed-off-by: Eric Sunshine <sunshine@sunshineco.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Eric Sunshine committed
Dec 2, 2021 at 22:44 UTC
da8fb6be5594b4a06ef2cc2ff7a5363a6fac2b5a
4 files changed
+27
-33
builtin/worktree.c
+7
-7
@@ -72,7 +72,7 @@ static void delete_worktrees_dir_if_empty(void)
72
static void prune_worktree(const char *id, const char *reason)
73
{
74
if (show_only || verbose)
75
- printf_ln(_("Removing %s/%s: %s"), "worktrees", id, reason);
75
+ fprintf_ln(stderr, _("Removing %s/%s: %s"), "worktrees", id, reason);
76
if (!show_only)
77
delete_git_dir(id);
78
}
@@ -418,24 +418,24 @@ static void print_preparing_worktree_line(int detach,
418
if (force_new_branch) {
419
struct commit *commit = lookup_commit_reference_by_name(new_branch);
420
if (!commit)
421
- printf_ln(_("Preparing worktree (new branch '%s')"), new_branch);
421
+ fprintf_ln(stderr, _("Preparing worktree (new branch '%s')"), new_branch);
422
else
423
- printf_ln(_("Preparing worktree (resetting branch '%s'; was at %s)"),
423
+ fprintf_ln(stderr, _("Preparing worktree (resetting branch '%s'; was at %s)"),
424
new_branch,
425
find_unique_abbrev(&commit->object.oid, DEFAULT_ABBREV));
426
} else if (new_branch) {
427
- printf_ln(_("Preparing worktree (new branch '%s')"), new_branch);
427
+ fprintf_ln(stderr, _("Preparing worktree (new branch '%s')"), new_branch);
428
} else {
429
struct strbuf s = STRBUF_INIT;
430
if (!detach && !strbuf_check_branch_ref(&s, branch) &&
431
ref_exists(s.buf))
432
- printf_ln(_("Preparing worktree (checking out '%s')"),
432
+ fprintf_ln(stderr, _("Preparing worktree (checking out '%s')"),
433
branch);
434
else {
435
struct commit *commit = lookup_commit_reference_by_name(branch);
436
if (!commit)
437
die(_("invalid reference: %s"), branch);
438
- printf_ln(_("Preparing worktree (detached HEAD %s)"),
438
+ fprintf_ln(stderr, _("Preparing worktree (detached HEAD %s)"),
439
find_unique_abbrev(&commit->object.oid, DEFAULT_ABBREV));
440
}
441
strbuf_release(&s);
@@ -1006,7 +1006,7 @@ static int remove_worktree(int ac, const char **av, const char *prefix)
1006
static void report_repair(int iserr, const char *path, const char *msg, void *cb_data)
1007
{
1008
if (!iserr) {
1009
- printf_ln(_("repair: %s: %s"), msg, path);
1009
+ fprintf_ln(stderr, _("repair: %s: %s"), msg, path);
1010
} else {
1011
int *exit_status = (int *)cb_data;
1012
fprintf_ln(stderr, _("error: %s: %s"), msg, path);
t/t2401-worktree-prune.sh
+7
-7
@@ -19,7 +19,7 @@ test_expect_success 'worktree prune on normal repo' '
19
test_expect_success 'prune files inside $GIT_DIR/worktrees' '
20
mkdir .git/worktrees &&
21
: >.git/worktrees/abc &&
22
- git worktree prune --verbose >actual &&
22
+ git worktree prune --verbose 2>actual &&
23
cat >expect <<EOF &&
24
Removing worktrees/abc: not a valid directory
25
EOF
@@ -34,7 +34,7 @@ test_expect_success 'prune directories without gitdir' '
34
cat >expect <<EOF &&
35
Removing worktrees/def: gitdir file does not exist
36
EOF
37
- git worktree prune --verbose >actual &&
37
+ git worktree prune --verbose 2>actual &&
38
test_cmp expect actual &&
39
! test -d .git/worktrees/def &&
40
! test -d .git/worktrees
@@ -45,7 +45,7 @@ test_expect_success SANITY 'prune directories with unreadable gitdir' '
45
: >.git/worktrees/def/def &&
46
: >.git/worktrees/def/gitdir &&
47
chmod u-r .git/worktrees/def/gitdir &&
48
- git worktree prune --verbose >actual &&
48
+ git worktree prune --verbose 2>actual &&
49
test_i18ngrep "Removing worktrees/def: unable to read gitdir file" actual &&
50
! test -d .git/worktrees/def &&
51
! test -d .git/worktrees
@@ -55,7 +55,7 @@ test_expect_success 'prune directories with invalid gitdir' '
55
mkdir -p .git/worktrees/def/abc &&
56
: >.git/worktrees/def/def &&
57
: >.git/worktrees/def/gitdir &&
58
- git worktree prune --verbose >actual &&
58
+ git worktree prune --verbose 2>actual &&
59
test_i18ngrep "Removing worktrees/def: invalid gitdir file" actual &&
60
! test -d .git/worktrees/def &&
61
! test -d .git/worktrees
@@ -65,7 +65,7 @@ test_expect_success 'prune directories with gitdir pointing to nowhere' '
65
mkdir -p .git/worktrees/def/abc &&
66
: >.git/worktrees/def/def &&
67
echo "$(pwd)"/nowhere >.git/worktrees/def/gitdir &&
68
- git worktree prune --verbose >actual &&
68
+ git worktree prune --verbose 2>actual &&
69
test_i18ngrep "Removing worktrees/def: gitdir file points to non-existent location" actual &&
70
! test -d .git/worktrees/def &&
71
! test -d .git/worktrees
@@ -101,7 +101,7 @@ test_expect_success 'prune duplicate (linked/linked)' '
101
git worktree add --detach w2 &&
102
sed "s/w2/w1/" .git/worktrees/w2/gitdir >.git/worktrees/w2/gitdir.new &&
103
mv .git/worktrees/w2/gitdir.new .git/worktrees/w2/gitdir &&
104
- git worktree prune --verbose >actual &&
104
+ git worktree prune --verbose 2>actual &&
105
test_i18ngrep "duplicate entry" actual &&
106
test -d .git/worktrees/w1 &&
107
! test -d .git/worktrees/w2
@@ -114,7 +114,7 @@ test_expect_success 'prune duplicate (main/linked)' '
114
git -C repo worktree add --detach ../wt &&
115
rm -fr wt &&
116
mv repo wt &&
117
- git -C wt worktree prune --verbose >actual &&
117
+ git -C wt worktree prune --verbose 2>actual &&
118
test_i18ngrep "duplicate entry" actual &&
119
! test -d .git/worktrees/wt
120
'
t/t2402-worktree-list.sh
+1
-1
@@ -134,7 +134,7 @@ test_expect_success '"list" all worktrees with prunable consistent with "prune"'
134
git worktree list >out &&
135
grep "/prunable *[0-9a-f].* prunable$" out &&
136
! grep "/unprunable *[0-9a-f].* unprunable$" out &&
137
- git worktree prune --verbose >out &&
137
+ git worktree prune --verbose 2>out &&
138
test_i18ngrep "^Removing worktrees/prunable" out &&
139
test_i18ngrep ! "^Removing worktrees/unprunable" out
140
'
t/t2406-worktree-repair.sh
+12
-18
@@ -45,9 +45,8 @@ test_corrupt_gitfile () {
45
git worktree add --detach corrupt &&
46
git -C corrupt rev-parse --absolute-git-dir >expect &&
47
eval "$butcher" &&
48
- git -C "$repairdir" worktree repair >out 2>err &&
49
- test_i18ngrep "$problem" out &&
50
- test_must_be_empty err &&
48
+ git -C "$repairdir" worktree repair 2>err &&
49
+ test_i18ngrep "$problem" err &&
50
git -C corrupt rev-parse --absolute-git-dir >actual &&
51
test_cmp expect actual
52
}
@@ -130,10 +129,9 @@ test_expect_success 'repair broken gitdir' '
129
sed s,orig/\.git$,moved/.git, .git/worktrees/orig/gitdir >expect &&
130
rm .git/worktrees/orig/gitdir &&
131
mv orig moved &&
133
- git worktree repair moved >out 2>err &&
132
+ git worktree repair moved 2>err &&
133
test_cmp expect .git/worktrees/orig/gitdir &&
135
- test_i18ngrep "gitdir unreadable" out &&
136
- test_must_be_empty err
134
+ test_i18ngrep "gitdir unreadable" err
135
'
136
137
test_expect_success 'repair incorrect gitdir' '
@@ -141,10 +139,9 @@ test_expect_success 'repair incorrect gitdir' '
139
git worktree add --detach orig &&
140
sed s,orig/\.git$,moved/.git, .git/worktrees/orig/gitdir >expect &&
141
mv orig moved &&
144
- git worktree repair moved >out 2>err &&
142
+ git worktree repair moved 2>err &&
143
test_cmp expect .git/worktrees/orig/gitdir &&
146
- test_i18ngrep "gitdir incorrect" out &&
147
- test_must_be_empty err
144
+ test_i18ngrep "gitdir incorrect" err
145
'
146
147
test_expect_success 'repair gitdir (implicit) from linked worktree' '
@@ -152,10 +149,9 @@ test_expect_success 'repair gitdir (implicit) from linked worktree' '
149
git worktree add --detach orig &&
150
sed s,orig/\.git$,moved/.git, .git/worktrees/orig/gitdir >expect &&
151
mv orig moved &&
155
- git -C moved worktree repair >out 2>err &&
152
+ git -C moved worktree repair 2>err &&
153
test_cmp expect .git/worktrees/orig/gitdir &&
157
- test_i18ngrep "gitdir incorrect" out &&
158
- test_must_be_empty err
154
+ test_i18ngrep "gitdir incorrect" err
155
'
156
157
test_expect_success 'unable to repair gitdir (implicit) from main worktree' '
@@ -163,9 +159,8 @@ test_expect_success 'unable to repair gitdir (implicit) from main worktree' '
159
git worktree add --detach orig &&
160
cat .git/worktrees/orig/gitdir >expect &&
161
mv orig moved &&
166
- git worktree repair >out 2>err &&
162
+ git worktree repair 2>err &&
163
test_cmp expect .git/worktrees/orig/gitdir &&
168
- test_must_be_empty out &&
164
test_must_be_empty err
165
'
166
@@ -178,12 +173,11 @@ test_expect_success 'repair multiple gitdir files' '
173
sed s,orig2/\.git$,moved2/.git, .git/worktrees/orig2/gitdir >expect2 &&
174
mv orig1 moved1 &&
175
mv orig2 moved2 &&
181
- git worktree repair moved1 moved2 >out 2>err &&
176
+ git worktree repair moved1 moved2 2>err &&
177
test_cmp expect1 .git/worktrees/orig1/gitdir &&
178
test_cmp expect2 .git/worktrees/orig2/gitdir &&
184
- test_i18ngrep "gitdir incorrect:.*orig1/gitdir$" out &&
185
- test_i18ngrep "gitdir incorrect:.*orig2/gitdir$" out &&
186
- test_must_be_empty err
179
+ test_i18ngrep "gitdir incorrect:.*orig1/gitdir$" err &&
180
+ test_i18ngrep "gitdir incorrect:.*orig2/gitdir$" err
181
'
182
183
test_expect_success 'repair moved main and linked worktrees' '