Merge branch 'ex/deprecate-empty-pathspec-as-match-all'
authorJunio C Hamano <gitster@pobox.com>
Wed, 26 Oct 2016 20:14:56 +0000 (13:14 -0700)
committerJunio C Hamano <gitster@pobox.com>
Wed, 26 Oct 2016 20:14:56 +0000 (13:14 -0700)
An empty string used as a pathspec element has always meant
'everything matches', but it is too easy to write a script that
finds a path to remove in $path and run 'git rm "$paht"', which
ends up removing everything. Start warning about this use of an
empty string used for 'everything matches' and ask users to use a
more explicit '.' for that instead.

The hope is that existing users will not mind this change, and
eventually the warning can be turned into a hard error, upgrading
the deprecation into removal of this (mis)feature.

* ex/deprecate-empty-pathspec-as-match-all:
pathspec: warn on empty strings as pathspec

1  2 
pathspec.c
t/t3700-add.sh
diff --combined pathspec.c
index 86f2b449b1b43d7abb0917c07676304a666056a9,729df9f385fd2542d7b889b010fe33dfc8ddd019..22ca74a126e79995607e68a557ae8bd4c2ab02f7
@@@ -364,7 -364,7 +364,7 @@@ void parse_pathspec(struct pathspec *pa
  {
        struct pathspec_item *item;
        const char *entry = argv ? *argv : NULL;
-       int i, n, prefixlen, nr_exclude = 0;
+       int i, n, prefixlen, warn_empty_string, nr_exclude = 0;
  
        memset(pathspec, 0, sizeof(*pathspec));
  
        }
  
        n = 0;
-       while (argv[n])
+       warn_empty_string = 1;
+       while (argv[n]) {
+               if (*argv[n] == '\0' && warn_empty_string) {
+                       warning(_("empty strings as pathspecs will be made invalid in upcoming releases. "
+                                 "please use . instead if you meant to match all paths"));
+                       warn_empty_string = 0;
+               }
                n++;
+       }
  
        pathspec->nr = n;
        ALLOC_ARRAY(pathspec->items, n);
        if (pathspec->magic & PATHSPEC_MAXDEPTH) {
                if (flags & PATHSPEC_KEEP_ORDER)
                        die("BUG: PATHSPEC_MAXDEPTH_VALID and PATHSPEC_KEEP_ORDER are incompatible");
 -              qsort(pathspec->items, pathspec->nr,
 -                    sizeof(struct pathspec_item), pathspec_item_cmp);
 +              QSORT(pathspec->items, pathspec->nr, pathspec_item_cmp);
        }
  }
  
@@@ -484,10 -492,11 +491,10 @@@ void copy_pathspec(struct pathspec *dst
  {
        *dst = *src;
        ALLOC_ARRAY(dst->items, dst->nr);
 -      memcpy(dst->items, src->items,
 -             sizeof(struct pathspec_item) * dst->nr);
 +      COPY_ARRAY(dst->items, src->items, dst->nr);
  }
  
 -void free_pathspec(struct pathspec *pathspec)
 +void clear_pathspec(struct pathspec *pathspec)
  {
        free(pathspec->items);
        pathspec->items = NULL;
diff --combined t/t3700-add.sh
index 53c0cb6dea686ac872c219b746a0c24b3d505416,05379d0a4a5c5312896f8cc13e286ebb37c517e7..f3a4b4a913f344ce140344ec7b70482a6d36bcbe
@@@ -7,20 -7,6 +7,20 @@@ test_description='Test of git add, incl
  
  . ./test-lib.sh
  
 +# Test the file mode "$1" of the file "$2" in the index.
 +test_mode_in_index () {
 +      case "$(git ls-files -s "$2")" in
 +      "$1 "*" $2")
 +              echo pass
 +              ;;
 +      *)
 +              echo fail
 +              git ls-files -s "$2"
 +              return 1
 +              ;;
 +      esac
 +}
 +
  test_expect_success \
      'Test of git add' \
      'touch foo && git add foo'
@@@ -39,12 -25,18 +39,12 @@@ test_expect_success 
         echo foo >xfoo1 &&
         chmod 755 xfoo1 &&
         git add xfoo1 &&
 -       case "$(git ls-files --stage xfoo1)" in
 -       100644" "*xfoo1) echo pass;;
 -       *) echo fail; git ls-files --stage xfoo1; (exit 1);;
 -       esac'
 +       test_mode_in_index 100644 xfoo1'
  
  test_expect_success 'git add: filemode=0 should not get confused by symlink' '
        rm -f xfoo1 &&
        test_ln_s_add foo xfoo1 &&
 -      case "$(git ls-files --stage xfoo1)" in
 -      120000" "*xfoo1) echo pass;;
 -      *) echo fail; git ls-files --stage xfoo1; (exit 1);;
 -      esac
 +      test_mode_in_index 120000 xfoo1
  '
  
  test_expect_success \
         echo foo >xfoo2 &&
         chmod 755 xfoo2 &&
         git update-index --add xfoo2 &&
 -       case "$(git ls-files --stage xfoo2)" in
 -       100644" "*xfoo2) echo pass;;
 -       *) echo fail; git ls-files --stage xfoo2; (exit 1);;
 -       esac'
 +       test_mode_in_index 100644 xfoo2'
  
  test_expect_success 'git add: filemode=0 should not get confused by symlink' '
        rm -f xfoo2 &&
        test_ln_s_add foo xfoo2 &&
 -      case "$(git ls-files --stage xfoo2)" in
 -      120000" "*xfoo2) echo pass;;
 -      *) echo fail; git ls-files --stage xfoo2; (exit 1);;
 -      esac
 +      test_mode_in_index 120000 xfoo2
  '
  
  test_expect_success \
        'git update-index --add: Test that executable bit is not used...' \
        'git config core.filemode 0 &&
         test_ln_s_add xfoo2 xfoo3 &&   # runs git update-index --add
 -       case "$(git ls-files --stage xfoo3)" in
 -       120000" "*xfoo3) echo pass;;
 -       *) echo fail; git ls-files --stage xfoo3; (exit 1);;
 -       esac'
 +       test_mode_in_index 120000 xfoo3'
  
  test_expect_success '.gitignore test setup' '
        echo "*.ig" >.gitignore &&
@@@ -331,71 -332,9 +331,76 @@@ test_expect_success 'git add --dry-run 
        test_i18ncmp expect.err actual.err
  '
  
+ test_expect_success 'git add empty string should invoke warning' '
+       git add "" 2>output &&
+       test_i18ngrep "warning: empty strings" output
+ '
 +test_expect_success 'git add --chmod=[+-]x stages correctly' '
 +      rm -f foo1 &&
 +      echo foo >foo1 &&
 +      git add --chmod=+x foo1 &&
 +      test_mode_in_index 100755 foo1 &&
 +      git add --chmod=-x foo1 &&
 +      test_mode_in_index 100644 foo1
 +'
 +
 +test_expect_success POSIXPERM,SYMLINKS 'git add --chmod=+x with symlinks' '
 +      git config core.filemode 1 &&
 +      git config core.symlinks 1 &&
 +      rm -f foo2 &&
 +      echo foo >foo2 &&
 +      git add --chmod=+x foo2 &&
 +      test_mode_in_index 100755 foo2
 +'
 +
 +test_expect_success 'git add --chmod=[+-]x changes index with already added file' '
 +      rm -f foo3 xfoo3 &&
 +      echo foo >foo3 &&
 +      git add foo3 &&
 +      git add --chmod=+x foo3 &&
 +      test_mode_in_index 100755 foo3 &&
 +      echo foo >xfoo3 &&
 +      chmod 755 xfoo3 &&
 +      git add xfoo3 &&
 +      git add --chmod=-x xfoo3 &&
 +      test_mode_in_index 100644 xfoo3
 +'
 +
 +test_expect_success POSIXPERM 'git add --chmod=[+-]x does not change the working tree' '
 +      echo foo >foo4 &&
 +      git add foo4 &&
 +      git add --chmod=+x foo4 &&
 +      ! test -x foo4
 +'
 +
 +test_expect_success 'no file status change if no pathspec is given' '
 +      >foo5 &&
 +      >foo6 &&
 +      git add foo5 foo6 &&
 +      git add --chmod=+x &&
 +      test_mode_in_index 100644 foo5 &&
 +      test_mode_in_index 100644 foo6
 +'
 +
 +test_expect_success 'no file status change if no pathspec is given in subdir' '
 +      mkdir -p sub &&
 +      (
 +              cd sub &&
 +              >sub-foo1 &&
 +              >sub-foo2 &&
 +              git add . &&
 +              git add --chmod=+x &&
 +              test_mode_in_index 100644 sub-foo1 &&
 +              test_mode_in_index 100644 sub-foo2
 +      )
 +'
 +
 +test_expect_success 'all statuses changed in folder if . is given' '
 +      git add --chmod=+x . &&
 +      test $(git ls-files --stage | grep ^100644 | wc -l) -eq 0 &&
 +      git add --chmod=-x . &&
 +      test $(git ls-files --stage | grep ^100755 | wc -l) -eq 0
 +'
 +
  test_done