Git development
 help / color / mirror / Atom feed
From: Emily Xie <emilyxxie@gmail.com>
To: git@vger.kernel.org
Cc: novalis@novalis.org, gitster@pobox.com, Emily Xie <emilyxxie@gmail.com>
Subject: [PATCH] pathspec: warn on empty strings as pathspec
Date: Mon, 20 Jun 2016 22:15:15 -0400	[thread overview]
Message-ID: <20160621021515.64964-1-emilyxxie@gmail.com> (raw)

For any command that takes a pathspec, passing an empty string will
execute the command on all files in the current directory. This
results in unexpected behavior. For example, git add "" adds all
files to staging, while git rm -rf "" recursively removes all files
from the working tree and index. A two-step implemetation will
prevent such cases.

This patch, as step one, invokes a warning whenever an empty
string is detected as a pathspec, introducing users to the upcoming
change. For step two, a follow-up patch several release cycles later
will remove the warnings and actually implement the change by
throwing an error instead.

Signed-off-by: Emily Xie <emilyxxie@gmail.com>
Reported-by: David Turner <novalis@novalis.org>
Mentored-by: Michail Denchev <mdenchev@gmail.com>
Thanks-to: Sarah Sharp <sarah@thesharps.us> and James Sharp <jamey@minilop.net>
---
 pathspec.c     | 6 +++++-
 t/t3600-rm.sh  | 6 +++++-
 t/t3700-add.sh | 4 ++++
 3 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/pathspec.c b/pathspec.c
index c9e9b6c..79e370e 100644
--- a/pathspec.c
+++ b/pathspec.c
@@ -402,8 +402,12 @@ void parse_pathspec(struct pathspec *pathspec,
 	}
 
 	n = 0;
-	while (argv[n])
+	while (argv[n]) {
+		if (*argv[n] == '\0')
+			warning(_("empty strings are not valid pathspecs and will no longer "
+			          "be supported in upcoming releases"));
 		n++;
+	}
 
 	pathspec->nr = n;
 	ALLOC_ARRAY(pathspec->items, n);
diff --git a/t/t3600-rm.sh b/t/t3600-rm.sh
index d046d98..4503a14 100755
--- a/t/t3600-rm.sh
+++ b/t/t3600-rm.sh
@@ -881,4 +881,8 @@ test_expect_success 'rm files with two different errors' '
 	test_i18ncmp expect actual
 '
 
-test_done
+test_expect_success 'rm empty string should invoke warning' '
+	git rm -rf "" 2>&1 | grep "warning: empty string"
+'
+
+test_done
\ No newline at end of file
diff --git a/t/t3700-add.sh b/t/t3700-add.sh
index f14a665..5dbe8c2 100755
--- a/t/t3700-add.sh
+++ b/t/t3700-add.sh
@@ -207,6 +207,10 @@ test_expect_success POSIXPERM,SANITY 'git add should fail atomically upon an unr
 	! ( git ls-files foo1 | grep foo1 )
 '
 
+test_expect_success 'git add empty string should invoke warning' '
+	git add "" 2>&1 | grep "warning: empty string"
+'
+
 rm -f foo2
 
 test_expect_success POSIXPERM,SANITY 'git add --ignore-errors' '
-- 
2.8.4


             reply	other threads:[~2016-06-21  2:15 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-06-21  2:15 Emily Xie [this message]
2016-06-21  5:11 ` [PATCH] pathspec: warn on empty strings as pathspec Junio C Hamano
  -- strict thread matches above, loose matches on Subject: below --
2016-06-22 23:00 Emily Xie
2016-06-22 23:12 ` Junio C Hamano
2016-06-22 23:48   ` Emily Xie
2016-06-23  6:17     ` Junio C Hamano
2016-06-24 18:21       ` Emily Xie

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160621021515.64964-1-emilyxxie@gmail.com \
    --to=emilyxxie@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=novalis@novalis.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox