Skip to contentSkip to conversation

Show an error if too-long paths are seen by git clean -dfx

PATCH Superseded Johannes Schindelin via GitGitGadget 5 participants 12 messages Jul 16, 2019 – Jul 19, 2019

lore
Share & tools
lore
#0 lore raw

cover Show an error if too-long paths are seen by git clean -dfx

This is particularly important on Windows, where PATH_MAX is very small compared to Unix/Linux.

Johannes Schindelin (1):
clean: show an error message when the path is too long

 builtin/clean.c  |  3 ++-
 t/t7300-clean.sh | 11 +++++++++++
 2 files changed, 13 insertions(+), 1 deletion(-)

base-commit: aa25c82427ae70aebf3b8f970f2afd54e9a2a8c6
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-219%2Fdscho%2Fclean-long-paths-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-219/dscho/clean-long-paths-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/219

Signature · 1 line
gitgitgadget
#1 lore raw

PATCH 1/1 clean: show an error message when the path is too long

From: Johannes Schindelin <[email protected]>

Without an error message when stat() failed, e.g. git clean would abort without an error message, leaving the user quite puzzled.

In particular on Windows, where the default maximum path length is quite small (yet there are ways to circumvent that limit in many cases), it is very important that users be given an indication why their command failed because of too long paths when it did.

This test case makes sure that a warning is issued that would have helped the user who reported this issue:

	https://github.com/git-for-windows/git/issues/521
---
 builtin/clean.c  |  3 ++-
 t/t7300-clean.sh | 11 +++++++++++
 2 files changed, 13 insertions(+), 1 deletion(-)
builtin/clean.c +2 -1
index aaba4af3c2..7be689f480 100644
@@ -194,7 +194,8 @@ static int remove_dirs(struct strbuf *path, const char *prefix, int force_flag,
 		strbuf_setlen(path, len);
 		strbuf_addstr(path, e->d_name);
 		if (lstat(path->buf, &st))
-			; /* fall thru */
+			warning("Could not stat path '%s': %s",
+				path->buf, strerror(errno));
 		else if (S_ISDIR(st.st_mode)) {
 			if (remove_dirs(path, prefix, force_flag, dry_run, quiet, &gone))
 				ret = 1;
t/t7300-clean.sh +11 -0
index 7b36954d63..aa08443f6a 100755
@@ -669,4 +669,15 @@ test_expect_success 'git clean -d skips untracked dirs containing ignored files'
 	test_path_is_missing foo/b/bb
 '
 
+test_expect_success MINGW 'handle clean & core.longpaths = false nicely' '
+	git config core.longpaths false &&
+	test_when_finished git config --unset core.longpaths &&
+	a50=aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa &&
+	mkdir -p $a50$a50/$a50$a50/$a50$a50 &&
+	touch $a50$a50/test.txt &&
+	touch $a50$a50/$a50$a50/$a50$a50/test.txt &&
+	test_must_fail git clean -xdf 2>.git/err &&
+	grep "too long" .git/err
+'
+
 test_done
Signature · 1 line
gitgitgadget
#2 lore raw

Re: [PATCH 1/1] clean: show an error message when the path is too long

Am 16.07.19 um 16:04 schrieb Johannes Schindelin via GitGitGadget:

Quoted 26 lines

From: Johannes Schindelin <[email protected]>

Without an error message when stat() failed, e.g. git clean would abort without an error message, leaving the user quite puzzled.

In particular on Windows, where the default maximum path length is quite small (yet there are ways to circumvent that limit in many cases), it is very important that users be given an indication why their command failed because of too long paths when it did.

This test case makes sure that a warning is issued that would have helped the user who reported this issue:

	https://github.com/git-for-windows/git/issues/521
---
 builtin/clean.c  |  3 ++-
 t/t7300-clean.sh | 11 +++++++++++
 2 files changed, 13 insertions(+), 1 deletion(-)
builtin/clean.c +0 -0
index aaba4af3c2..7be689f480 100644

builtin/clean.c static int remove_dirs(struct strbuf *path, const char *prefix, int force_flag, · line 197

Quoted 4 lines
builtin/clean.c +0 -1
@@ -194,7 +194,8 @@ static int remove_dirs(struct strbuf *path, const char *prefix, int force_flag,
 		strbuf_setlen(path, len);
 		strbuf_addstr(path, e->d_name);
 		if (lstat(path->buf, &st))
-			; /* fall thru */

I don't understand the "fall thru" comment here. It only makes sense in switch statements, doesn't it? And the code after this if/else-if/else block is only executed if we pass through here, so why was it placed way down in the first place? Perhaps for historical reasons.

dir.c::remove_dir_recurse() has such a comment as well, by the way.

Anyway, I'd keep that strange comment, as I don't see a connection to your changes. (Or explain in the commit message why we no longer "fall thru", whatever that may mean. Or perhaps I'm just thick.)

builtin/clean.c static int remove_dirs(struct strbuf *path, const char *prefix, int force_flag, · line 198

Quoted 2 lines
builtin/clean.c +2 -0
+			warning("Could not stat path '%s': %s",
+				path->buf, strerror(errno));

The other warnings in that function are issued using warning_errno() (shorter code, consistency is enforced) and messages are marked for translation. That would be nice to have here as well, no?

Quoted 10 lines
builtin/clean.c +0 -0
 		else if (S_ISDIR(st.st_mode)) {
 			if (remove_dirs(path, prefix, force_flag, dry_run, quiet, &gone))
 				ret = 1;
t/t7300-clean.sh +0 -0
index 7b36954d63..aa08443f6a 100755
@@ -669,4 +669,15 @@ test_expect_success 'git clean -d skips untracked dirs containing ignored files'
 	test_path_is_missing foo/b/bb
 '

t/t7300-clean.sh test_expect_success 'git clean -d skips untracked dirs containing ignored files' · line 680

Quoted 10 lines
t/t7300-clean.sh +9 -0
 
+test_expect_success MINGW 'handle clean & core.longpaths = false nicely' '
+	git config core.longpaths false &&
+	test_when_finished git config --unset core.longpaths &&
+	a50=aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa &&
+	mkdir -p $a50$a50/$a50$a50/$a50$a50 &&
+	touch $a50$a50/test.txt &&
+	touch $a50$a50/$a50$a50/$a50$a50/test.txt &&
+	test_must_fail git clean -xdf 2>.git/err &&
+	grep "too long" .git/err

The pattern "too long" is expected to be supplied by strerror(3), right? Depending on the locale it might return an message in a different language, so test_i18ngrep should be used here even if the warning above is not translated, right?

Quoted 4 lines
t/t7300-clean.sh +2 -0
+'
+
 test_done
#6 lore raw

Re: [PATCH 1/1] clean: show an error message when the path is too long

In reply to Johannes Schindelin via GitGitGadget

t/t7300-clean.sh test_expect_success 'git clean -d skips untracked dirs containing ignored files' · line 674

Quoted 3 lines · Johannes Schindelin via GitGitGadget
t/t7300-clean.sh +3 -0
+test_expect_success MINGW 'handle clean & core.longpaths = false nicely' '
+	git config core.longpaths false &&
+	test_when_finished git config --unset core.longpaths &&

'test_config core.longpaths false' could replace the above two lines with a single one.

t/t7300-clean.sh test_expect_success 'git clean -d skips untracked dirs containing ignored files' · line 678

Quoted 4 lines
t/t7300-clean.sh +4 -0
+	a50=aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa &&
+	mkdir -p $a50$a50/$a50$a50/$a50$a50 &&
+	touch $a50$a50/test.txt &&
+	touch $a50$a50/$a50$a50/$a50$a50/test.txt &&

Is there a reason for using 'touch' to create these files here, instead of the usual '>"$file"' shell redirections? Something Windows/MinGW/long path specific, perhaps?

t/t7300-clean.sh test_expect_success 'git clean -d skips untracked dirs containing ignored files' · line 679

Quoted 1 line
t/t7300-clean.sh +1 -0
+	test_must_fail git clean -xdf 2>.git/err &&

I was puzzled when I saw that '2>.git/err' first, because why put that file in the .git directory?! but of course 'git clean' would delete that file if it were in the worktree. OK.

Quoted 6 lines
t/t7300-clean.sh +3 -0
+	grep "too long" .git/err
+'
+
 test_done
Signature · 1 line
gitgitgadget
#3 lore raw

Re: [PATCH 1/1] clean: show an error message when the path is too long

In reply to René Scharfe
Quoted 6 lines · René Scharfe
Quoted 6 lines
builtin/clean.c +0 -0
index aaba4af3c2..7be689f480 100644
@@ -194,7 +194,8 @@ static int remove_dirs(struct strbuf *path, const char *prefix, int force_flag,
 		strbuf_setlen(path, len);
Quoted 8 lines

builtin/clean.c static int remove_dirs(struct strbuf *path, const char *prefix, int force_flag, · line 197

Quoted 3 lines
builtin/clean.c +0 -1
 		strbuf_addstr(path, e->d_name);
 		if (lstat(path->buf, &st))
-			; /* fall thru */

I don't understand the "fall thru" comment here. It only makes sense in switch statements, doesn't it? And the code after this if/else-if/else block is only executed if we pass through here, so why was it placed way down in the first place? Perhaps for historical reasons.

f538a91e ("git-clean: Display more accurate delete messages", 2013-01-11) introduced that line when it first introduced the function and it is not inherited from anything else. As the if/else cascade has a catch-all else that always continues at the end, failing lstat is the only way for the entire loop to break out early, so as you hinted above, having the "fail, break and return" right there would probably be a better organization of this loop.

Quoted 10 lines

Anyway, I'd keep that strange comment, as I don't see a connection to your changes. (Or explain in the commit message why we no longer "fall thru", whatever that may mean. Or perhaps I'm just thick.)

builtin/clean.c static int remove_dirs(struct strbuf *path, const char *prefix, int force_flag, · line 198

Quoted 2 lines
builtin/clean.c +2 -0
+			warning("Could not stat path '%s': %s",
+				path->buf, strerror(errno));

The other warnings in that function are issued using warning_errno() (shorter code, consistency is enforced) and messages are marked for translation. That would be nice to have here as well, no?

Absolutely. Also, downcase "Could" and perhaps use _() around.

As to the "fall thru" comment, I tend to agree that it does not fall through to the next "case" in the usual sense and is confusing. Mentioning that we removed a confusing and pointless comment in the log message would be nice, but I'd vote for removing it if I was asked.

Thanks.

#4 lore raw

Re: [PATCH 1/1] clean: show an error message when the path is too long

Quoted 5 lines · Junio C Hamano
Quoted 3 lines

The other warnings in that function are issued using warning_errno() (shorter code, consistency is enforced) and messages are marked for translation. That would be nice to have here as well, no?

Absolutely. Also, downcase "Could" and perhaps use _() around.

This one is easy enough (not just in the technical sense, but in the sense that it has little room wasting our time bikeshedding), so let's tie the loose ends and move on.

I was tempted to fix the proposed log message to excise exaggeration (I prefer not to see "very", "important", etc.---other things that is said in the message should be enough to convince readers about the importance), but didn't.

What I did do was to not just rephrasing the warning message, but to give it its own constant and to feed it to warning_errno(), to match the other warning message.

I also saved one (or perhaps two) fork(s) from the test script ;-) and added a portability note there.

1: d93f701a2e ! 1: b1e100aa6a clean: show an error message when the path is too long

    @@ Metadata
      ## Commit message ##
         clean: show an error message when the path is too long
     
    -    Without an error message when stat() failed, e.g. `git clean` would
    +    Without an error message when lstat() failed, `git clean` would
         abort without an error message, leaving the user quite puzzled.
     
         In particular on Windows, where the default maximum path length is quite
    @@ Commit message
                 https://github.com/git-for-windows/git/issues/521
     
         Signed-off-by: Johannes Schindelin <[email protected]>
    +    [jc: matched the warning message style to existing ones, fixed test]
         Signed-off-by: Junio C Hamano <[email protected]>
     
      ## builtin/clean.c ##
    +@@ builtin/clean.c: static const char *msg_would_remove = N_("Would remove %s\n");
    + static const char *msg_skip_git_dir = N_("Skipping repository %s\n");
    + static const char *msg_would_skip_git_dir = N_("Would skip repository %s\n");
    + static const char *msg_warn_remove_failed = N_("failed to remove %s");
    ++static const char *msg_warn_lstat_failed = N_("could not lstat %s\n");
    + 
    + enum color_clean {
    + 	CLEAN_COLOR_RESET = 0,
     @@ builtin/clean.c: static int remove_dirs(struct strbuf *path, const char *prefix, int force_flag,
      		strbuf_setlen(path, len);
      		strbuf_addstr(path, e->d_name);
      		if (lstat(path->buf, &st))
     -			; /* fall thru */
    -+			warning("Could not stat path '%s': %s",
    -+				path->buf, strerror(errno));
    ++			warning_errno(_(msg_warn_lstat_failed), path->buf);
      		else if (S_ISDIR(st.st_mode)) {
      			if (remove_dirs(path, prefix, force_flag, dry_run, quiet, &gone))
      				ret = 1;
    @@ t/t7300-clean.sh: test_expect_success 'git clean -d skips untracked dirs contain
     +	test_when_finished git config --unset core.longpaths &&
     +	a50=aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa &&
     +	mkdir -p $a50$a50/$a50$a50/$a50$a50 &&
    -+	touch $a50$a50/test.txt &&
    -+	touch $a50$a50/$a50$a50/$a50$a50/test.txt &&
    ++	: >"$a50$a50/test.txt" 2>"$a50$a50/$a50$a50/$a50$a50/test.txt" &&
    ++	# create a temporary outside the working tree to hide from "git clean"
     +	test_must_fail git clean -xdf 2>.git/err &&
    -+	grep "too long" .git/err
    ++	# grepping for a strerror string is unportable but it is OK here with
    ++	# MINGW prereq
    ++	test_i18ngrep "too long" .git/err
     +'
     +
      test_done

-- >8 --
From: Johannes Schindelin <[email protected]>
Subject: [PATCH] clean: show an error message when the path is too long

Without an error message when lstat() failed, git clean would abort without an error message, leaving the user quite puzzled.

In particular on Windows, where the default maximum path length is quite small (yet there are ways to circumvent that limit in many cases), it is very important that users be given an indication why their command failed because of too long paths when it did.

This test case makes sure that a warning is issued that would have helped the user who reported this issue:

	https://github.com/git-for-windows/git/issues/521
[jc: matched the warning message style to existing ones, fixed test]
---
 builtin/clean.c  |  3 ++-
 t/t7300-clean.sh | 13 +++++++++++++
 2 files changed, 15 insertions(+), 1 deletion(-)
builtin/clean.c +2 -1
index aaba4af3c2..d5579da716 100644
@@ -34,6 +34,7 @@ static const char *msg_would_remove = N_("Would remove %s\n");
 static const char *msg_skip_git_dir = N_("Skipping repository %s\n");
 static const char *msg_would_skip_git_dir = N_("Would skip repository %s\n");
 static const char *msg_warn_remove_failed = N_("failed to remove %s");
+static const char *msg_warn_lstat_failed = N_("could not lstat %s\n");
 
 enum color_clean {
 	CLEAN_COLOR_RESET = 0,
@@ -194,7 +195,7 @@ static int remove_dirs(struct strbuf *path, const char *prefix, int force_flag,
 		strbuf_setlen(path, len);
 		strbuf_addstr(path, e->d_name);
 		if (lstat(path->buf, &st))
-			; /* fall thru */
+			warning_errno(_(msg_warn_lstat_failed), path->buf);
 		else if (S_ISDIR(st.st_mode)) {
 			if (remove_dirs(path, prefix, force_flag, dry_run, quiet, &gone))
 				ret = 1;
t/t7300-clean.sh +13 -0
index 7b36954d63..bde55b358c 100755
@@ -669,4 +669,17 @@ test_expect_success 'git clean -d skips untracked dirs containing ignored files'
 	test_path_is_missing foo/b/bb
 '
 
+test_expect_success MINGW 'handle clean & core.longpaths = false nicely' '
+	git config core.longpaths false &&
+	test_when_finished git config --unset core.longpaths &&
+	a50=aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa &&
+	mkdir -p $a50$a50/$a50$a50/$a50$a50 &&
+	: >"$a50$a50/test.txt" 2>"$a50$a50/$a50$a50/$a50$a50/test.txt" &&
+	# create a temporary outside the working tree to hide from "git clean"
+	test_must_fail git clean -xdf 2>.git/err &&
+	# grepping for a strerror string is unportable but it is OK here with
+	# MINGW prereq
+	test_i18ngrep "too long" .git/err
+'
+
 test_done
Signature · 1 line
2.22.0-653-g37fc7794bc
#5 lore raw

Re: [PATCH 1/1] clean: show an error message when the path is too long

Hi,

Quoted 19 lines · Junio C Hamano
Quoted 6 lines · Junio C Hamano
Quoted 4 lines

The other warnings in that function are issued using warning_errno() (shorter code, consistency is enforced) and messages are marked for translation. That would be nice to have here as well, no?

Absolutely. Also, downcase "Could" and perhaps use _() around.

This one is easy enough (not just in the technical sense, but in the sense that it has little room wasting our time bikeshedding), so let's tie the loose ends and move on.

I was tempted to fix the proposed log message to excise exaggeration (I prefer not to see "very", "important", etc.---other things that is said in the message should be enough to convince readers about the importance), but didn't.

Quoted 6 lines

What I did do was to not just rephrasing the warning message, but to give it its own constant and to feed it to warning_errno(), to match the other warning message.

I also saved one (or perhaps two) fork(s) from the test script ;-) and added a portability note there.

Thanks!

On top, I integrated Gabór's suggestion to use test_config and threw in a paragraph in the commit message to explain why the core.longpaths variable is touched at all.

v2 incoming,
Dscho

#7 lore raw

v2 cover Show an error if too-long paths are seen by git clean -dfx

In reply to Johannes Schindelin via GitGitGadget

This is particularly important on Windows, where PATH_MAX is very small compared to Unix/Linux.

Changes since v1:

* Matched the warning message style to existing ones,
* Fixed test in multiple ways: * Avoiding touch in favor of : >.
* Using test_config.
* Using test_i18ngrep instead of grep to avoid localization problems.
* Add helpful comments.

* The commit message now talks about lstat() instead of stat().
* The commit message also explains where that core.longpaths comes from.

Johannes Schindelin (1):
clean: show an error message when the path is too long

 builtin/clean.c  |  3 ++-
 t/t7300-clean.sh | 12 ++++++++++++
 2 files changed, 14 insertions(+), 1 deletion(-)

base-commit: aa25c82427ae70aebf3b8f970f2afd54e9a2a8c6
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-219%2Fdscho%2Fclean-long-paths-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-219/dscho/clean-long-paths-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/219

Range-diff vs v1:

1: 36677556a2 ! 1: c7b11fe410 clean: show an error message when the path is too long

     @@ -2,7 +2,7 @@
      
          clean: show an error message when the path is too long
      
     -    Without an error message when stat() failed, e.g. `git clean` would
     +    Without an error message when `lstat()` failed, `git clean` would
          abort without an error message, leaving the user quite puzzled.
      
          In particular on Windows, where the default maximum path length is quite
     @@ -15,18 +15,32 @@
      
                  https://github.com/git-for-windows/git/issues/521
      
     +    Note that we temporarily set `core.longpaths = false` in the regression
     +    test; This ensures forward-compatibility with the `core.longpaths`
     +    feature that has not yet been upstreamed from Git for Windows.
     +
     +    Helped-by: René Scharfe <[email protected]>
     +    Helped-by: SZEDER Gábor <[email protected]>
     +    Helped-by: Junio C Hamano <[email protected]>
          Signed-off-by: Johannes Schindelin <[email protected]>
      
       diff --git a/builtin/clean.c b/builtin/clean.c
       --- a/builtin/clean.c
       +++ b/builtin/clean.c
     +@@
     + static const char *msg_skip_git_dir = N_("Skipping repository %s\n");
     + static const char *msg_would_skip_git_dir = N_("Would skip repository %s\n");
     + static const char *msg_warn_remove_failed = N_("failed to remove %s");
     ++static const char *msg_warn_lstat_failed = N_("could not lstat %s\n");
     + 
     + enum color_clean {
     + 	CLEAN_COLOR_RESET = 0,
      @@
       		strbuf_setlen(path, len);
       		strbuf_addstr(path, e->d_name);
       		if (lstat(path->buf, &st))
      -			; /* fall thru */
     -+			warning("Could not stat path '%s': %s",
     -+				path->buf, strerror(errno));
     ++			warning_errno(_(msg_warn_lstat_failed), path->buf);
       		else if (S_ISDIR(st.st_mode)) {
       			if (remove_dirs(path, prefix, force_flag, dry_run, quiet, &gone))
       				ret = 1;
     @@ -39,14 +53,15 @@
       '
       
      +test_expect_success MINGW 'handle clean & core.longpaths = false nicely' '
     -+	git config core.longpaths false &&
     -+	test_when_finished git config --unset core.longpaths &&
     ++	test_config core.longpaths false &&
      +	a50=aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa &&
      +	mkdir -p $a50$a50/$a50$a50/$a50$a50 &&
     -+	touch $a50$a50/test.txt &&
     -+	touch $a50$a50/$a50$a50/$a50$a50/test.txt &&
     ++	: >"$a50$a50/test.txt" 2>"$a50$a50/$a50$a50/$a50$a50/test.txt" &&
     ++	# create a temporary outside the working tree to hide from "git clean"
      +	test_must_fail git clean -xdf 2>.git/err &&
     -+	grep "too long" .git/err
     ++	# grepping for a strerror string is unportable but it is OK here with
     ++	# MINGW prereq
     ++	test_i18ngrep "too long" .git/err
      +'
      +
       test_done
Signature · 1 line
gitgitgadget
#8 lore raw

PATCH v2 1/1 clean: show an error message when the path is too long

From: Johannes Schindelin <[email protected]>

Without an error message when lstat() failed, git clean would abort without an error message, leaving the user quite puzzled.

In particular on Windows, where the default maximum path length is quite small (yet there are ways to circumvent that limit in many cases), it is very important that users be given an indication why their command failed because of too long paths when it did.

This test case makes sure that a warning is issued that would have helped the user who reported this issue:

	https://github.com/git-for-windows/git/issues/521

Note that we temporarily set core.longpaths = false in the regression test; This ensures forward-compatibility with the core.longpaths feature that has not yet been upstreamed from Git for Windows.

---
 builtin/clean.c  |  3 ++-
 t/t7300-clean.sh | 12 ++++++++++++
 2 files changed, 14 insertions(+), 1 deletion(-)
builtin/clean.c +2 -1
index aaba4af3c2..d5579da716 100644
@@ -34,6 +34,7 @@ static const char *msg_would_remove = N_("Would remove %s\n");
 static const char *msg_skip_git_dir = N_("Skipping repository %s\n");
 static const char *msg_would_skip_git_dir = N_("Would skip repository %s\n");
 static const char *msg_warn_remove_failed = N_("failed to remove %s");
+static const char *msg_warn_lstat_failed = N_("could not lstat %s\n");
 
 enum color_clean {
 	CLEAN_COLOR_RESET = 0,
@@ -194,7 +195,7 @@ static int remove_dirs(struct strbuf *path, const char *prefix, int force_flag,
 		strbuf_setlen(path, len);
 		strbuf_addstr(path, e->d_name);
 		if (lstat(path->buf, &st))
-			; /* fall thru */
+			warning_errno(_(msg_warn_lstat_failed), path->buf);
 		else if (S_ISDIR(st.st_mode)) {
 			if (remove_dirs(path, prefix, force_flag, dry_run, quiet, &gone))
 				ret = 1;
t/t7300-clean.sh +12 -0
index 7b36954d63..a2c45d1902 100755
@@ -669,4 +669,16 @@ test_expect_success 'git clean -d skips untracked dirs containing ignored files'
 	test_path_is_missing foo/b/bb
 '
 
+test_expect_success MINGW 'handle clean & core.longpaths = false nicely' '
+	test_config core.longpaths false &&
+	a50=aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa &&
+	mkdir -p $a50$a50/$a50$a50/$a50$a50 &&
+	: >"$a50$a50/test.txt" 2>"$a50$a50/$a50$a50/$a50$a50/test.txt" &&
+	# create a temporary outside the working tree to hide from "git clean"
+	test_must_fail git clean -xdf 2>.git/err &&
+	# grepping for a strerror string is unportable but it is OK here with
+	# MINGW prereq
+	test_i18ngrep "too long" .git/err
+'
+
 test_done
Signature · 1 line
gitgitgadget
#9 lore raw

Re: [PATCH v2 1/1] clean: show an error message when the path is too long

"Johannes Schindelin via GitGitGadget" <[email protected]>

Quoted 4 lines · writes:

From: Johannes Schindelin <[email protected]>

Without an error message when lstat() failed, git clean would abort without an error message, leaving the user quite puzzled.

Let's drop the first three words ;-) Sorry for not catching it earlier and parrotting the same mistake in my variant yesterday.

Quoted 4 lines

In particular on Windows, where the default maximum path length is quite small (yet there are ways to circumvent that limit in many cases), it is very important that users be given an indication why their command failed because of too long paths when it did.

s/it is very important that users be given/it helps to give users/

#10 lore raw

Re: [PATCH v2 1/1] clean: show an error message when the path is too long

Hi Junio,

Quoted 10 lines · Junio C Hamano

"Johannes Schindelin via GitGitGadget" <[email protected]>

Quoted 4 lines · writes:

From: Johannes Schindelin <[email protected]>

Without an error message when lstat() failed, git clean would abort without an error message, leaving the user quite puzzled.

Let's drop the first three words ;-) Sorry for not catching it earlier and parrotting the same mistake in my variant yesterday.

You mean the first four words.

Quoted 6 lines
Quoted 4 lines

In particular on Windows, where the default maximum path length is quite small (yet there are ways to circumvent that limit in many cases), it is very important that users be given an indication why their command failed because of too long paths when it did.

s/it is very important that users be given/it helps to give users/

If you really feel it important to invalidate my personal style of expression, sure.

Ciao,
Dscho

#11 lore raw

Re: [PATCH v2 1/1] clean: show an error message when the path is too long

Quoted 7 lines · Johannes Schindelin
Quoted 5 lines · Junio C Hamano

"Johannes Schindelin via GitGitGadget" <[email protected]>

Quoted 2 lines · writes:

From: Johannes Schindelin <[email protected]>

Quoted 7 lines
Quoted 5 lines
Quoted 2 lines

Without an error message when lstat() failed, git clean would abort without an error message, leaving the user quite puzzled.

Let's drop the first three words ;-) Sorry for not catching it earlier and parrotting the same mistake in my variant yesterday.

You mean the first four words.

Eek, yes, I cannot count ;-)

Quoted 9 lines
Quoted 6 lines
Quoted 4 lines

In particular on Windows, where the default maximum path length is quite small (yet there are ways to circumvent that limit in many cases), it is very important that users be given an indication why their command failed because of too long paths when it did.

s/it is very important that users be given/it helps to give users/

If you really feel it important to invalidate my personal style of expression, sure.

I meant it as a pure improvement---I think the importance of this change to the affected population is so obvious that it does not need self promoting exaggeration to convince readers by stating plainly how it helps.

But if it is very important for you, I'll undo the change before merging it to 'next'.

Thanks.
t