Merge branch 'jc/add-2.0-delete-default' (early part)
authorJunio C Hamano <gitster@pobox.com>
Mon, 22 Apr 2013 18:11:45 +0000 (11:11 -0700)
committerJunio C Hamano <gitster@pobox.com>
Mon, 22 Apr 2013 18:11:45 +0000 (11:11 -0700)
Preparatory steps to make "git add <pathspec>" take notice of
removed paths that match <pathspec> by default in Git 2.0.

* 'jc/add-2.0-delete-default' (early part):
git add: rephrase the "removal will cease to be ignored" warning
git add: rework the logic to warn "git add <pathspec>..." default change
git add: start preparing for "git add <pathspec>..." to default to "-A"
builtin/add.c: simplify boolean variables

1  2 
builtin/add.c
t/t2200-add-update.sh
diff --combined builtin/add.c
index daf02c6affb6115d058736a6859ceeb10e0791f0,20f459a0a389f8c265212295445d093555b7b13a..54cd2d417d30408c8a4b13099efc6eee6a3d1e31
@@@ -26,52 -26,11 +26,55 @@@ static int take_worktree_changes
  struct update_callback_data {
        int flags;
        int add_errors;
 +      const char *implicit_dot;
 +      size_t implicit_dot_len;
+       /* only needed for 2.0 transition preparation */
+       int warn_add_would_remove;
  };
  
 +static const char *option_with_implicit_dot;
 +static const char *short_option_with_implicit_dot;
 +
 +static void warn_pathless_add(void)
 +{
 +      static int shown;
 +      assert(option_with_implicit_dot && short_option_with_implicit_dot);
 +
 +      if (shown)
 +              return;
 +      shown = 1;
 +
 +      /*
 +       * To be consistent with "git add -p" and most Git
 +       * commands, we should default to being tree-wide, but
 +       * this is not the original behavior and can't be
 +       * changed until users trained themselves not to type
 +       * "git add -u" or "git add -A". For now, we warn and
 +       * keep the old behavior. Later, the behavior can be changed
 +       * to tree-wide, keeping the warning for a while, and
 +       * eventually we can drop the warning.
 +       */
 +      warning(_("The behavior of 'git add %s (or %s)' with no path argument from a\n"
 +                "subdirectory of the tree will change in Git 2.0 and should not be used anymore.\n"
 +                "To add content for the whole tree, run:\n"
 +                "\n"
 +                "  git add %s :/\n"
 +                "  (or git add %s :/)\n"
 +                "\n"
 +                "To restrict the command to the current directory, run:\n"
 +                "\n"
 +                "  git add %s .\n"
 +                "  (or git add %s .)\n"
 +                "\n"
 +                "With the current Git version, the command is restricted to "
 +                "the current directory.\n"
 +                ""),
 +              option_with_implicit_dot, short_option_with_implicit_dot,
 +              option_with_implicit_dot, short_option_with_implicit_dot,
 +              option_with_implicit_dot, short_option_with_implicit_dot);
 +}
 +
  static int fix_unmerged_status(struct diff_filepair *p,
                               struct update_callback_data *data)
  {
                return DIFF_STATUS_MODIFIED;
  }
  
+ static const char *add_would_remove_warning = N_(
+       "You ran 'git add' with neither '-A (--all)' or '--no-all', whose\n"
+ "behaviour will change in Git 2.0 with respect to paths you removed from\n"
+ "your working tree. Paths like '%s' that are\n"
+ "removed are ignored with this version of Git.\n"
+ "\n"
+ "* 'git add --no-all <pathspec>', which is the current default, ignores\n"
+ "  paths you removed from your working tree.\n"
+ "\n"
+ "* 'git add --all <pathspec>' will let you also record the removals.\n"
+ "\n"
+ "Run 'git status' to check the paths you removed from your working tree.\n");
+ static void warn_add_would_remove(const char *path)
+ {
+       warning(_(add_would_remove_warning), path);
+ }
  static void update_callback(struct diff_queue_struct *q,
                            struct diff_options *opt, void *cbdata)
  {
        int i;
        struct update_callback_data *data = cbdata;
 +      const char *implicit_dot = data->implicit_dot;
 +      size_t implicit_dot_len = data->implicit_dot_len;
  
        for (i = 0; i < q->nr; i++) {
                struct diff_filepair *p = q->queue[i];
                const char *path = p->one->path;
 +              /*
 +               * Check if "git add -A" or "git add -u" was run from a
 +               * subdirectory with a modified file outside that directory,
 +               * and warn if so.
 +               *
 +               * "git add -u" will behave like "git add -u :/" instead of
 +               * "git add -u ." in the future.  This warning prepares for
 +               * that change.
 +               */
 +              if (implicit_dot &&
 +                  strncmp_icase(path, implicit_dot, implicit_dot_len)) {
 +                      warn_pathless_add();
 +                      continue;
 +              }
                switch (fix_unmerged_status(p, data)) {
                default:
                        die(_("unexpected diff status %c"), p->status);
                        }
                        break;
                case DIFF_STATUS_DELETED:
+                       if (data->warn_add_would_remove) {
+                               warn_add_would_remove(path);
+                               data->warn_add_would_remove = 0;
+                       }
                        if (data->flags & ADD_CACHE_IGNORE_REMOVAL)
                                break;
                        if (!(data->flags & ADD_CACHE_PRETEND))
        }
  }
  
- int add_files_to_cache(const char *prefix, const char **pathspec, int flags)
+ static void update_files_in_cache(const char *prefix, const char **pathspec,
+                                 struct update_callback_data *data)
  {
-       struct update_callback_data data;
        struct rev_info rev;
-       memset(&data, 0, sizeof(data));
-       data.flags = flags & ~ADD_CACHE_IMPLICIT_DOT;
-       if ((flags & ADD_CACHE_IMPLICIT_DOT) && prefix) {
-               /*
-                * Check for modified files throughout the worktree so
-                * update_callback has a chance to warn about changes
-                * outside the cwd.
-                */
-               data.implicit_dot = prefix;
-               data.implicit_dot_len = strlen(prefix);
-               pathspec = NULL;
-       }
 +
        init_revisions(&rev, prefix);
        setup_revisions(0, NULL, &rev, NULL);
        init_pathspec(&rev.prune_data, pathspec);
        rev.diffopt.output_format = DIFF_FORMAT_CALLBACK;
        rev.diffopt.format_callback = update_callback;
-       rev.diffopt.format_callback_data = &data;
+       rev.diffopt.format_callback_data = data;
        rev.max_count = 0; /* do not compare unmerged paths with stage #2 */
        run_diff_files(&rev, DIFF_RACY_IS_MODIFIED);
+ }
+ int add_files_to_cache(const char *prefix, const char **pathspec, int flags)
+ {
+       struct update_callback_data data;
+       memset(&data, 0, sizeof(data));
+       data.flags = flags;
+       update_files_in_cache(prefix, pathspec, &data);
        return !!data.add_errors;
  }
  
 -static char *prune_directory(struct dir_struct *dir, const char **pathspec, int prefix)
 +#define WARN_IMPLICIT_DOT (1u << 0)
 +static char *prune_directory(struct dir_struct *dir, const char **pathspec,
 +                           int prefix, unsigned flag)
  {
        char *seen;
        int i, specs;
                if (match_pathspec(pathspec, entry->name, entry->len,
                                   prefix, seen))
                        *dst++ = entry;
 +              else if (flag & WARN_IMPLICIT_DOT)
 +                      /*
 +                       * "git add -A" was run from a subdirectory with a
 +                       * new file outside that directory.
 +                       *
 +                       * "git add -A" will behave like "git add -A :/"
 +                       * instead of "git add -A ." in the future.
 +                       * Warn about the coming behavior change.
 +                       */
 +                      warn_pathless_add();
        }
        dir->nr = dst - dir->entries;
        add_pathspec_matches_against_index(pathspec, seen, specs);
@@@ -354,23 -302,27 +375,27 @@@ static struct lock_file lock_file
  static const char ignore_error[] =
  N_("The following paths are ignored by one of your .gitignore files:\n");
  
- static int verbose = 0, show_only = 0, ignored_too = 0, refresh_only = 0;
- static int ignore_add_errors, addremove, intent_to_add, ignore_missing = 0;
+ static int verbose, show_only, ignored_too, refresh_only;
+ static int ignore_add_errors, intent_to_add, ignore_missing;
+ #define ADDREMOVE_DEFAULT 0 /* Change to 1 in Git 2.0 */
+ static int addremove = ADDREMOVE_DEFAULT;
+ static int addremove_explicit = -1; /* unspecified */
  
  static struct option builtin_add_options[] = {
        OPT__DRY_RUN(&show_only, N_("dry run")),
        OPT__VERBOSE(&verbose, N_("be verbose")),
        OPT_GROUP(""),
-       OPT_BOOLEAN('i', "interactive", &add_interactive, N_("interactive picking")),
-       OPT_BOOLEAN('p', "patch", &patch_interactive, N_("select hunks interactively")),
-       OPT_BOOLEAN('e', "edit", &edit_interactive, N_("edit current diff and apply")),
+       OPT_BOOL('i', "interactive", &add_interactive, N_("interactive picking")),
+       OPT_BOOL('p', "patch", &patch_interactive, N_("select hunks interactively")),
+       OPT_BOOL('e', "edit", &edit_interactive, N_("edit current diff and apply")),
        OPT__FORCE(&ignored_too, N_("allow adding otherwise ignored files")),
-       OPT_BOOLEAN('u', "update", &take_worktree_changes, N_("update tracked files")),
-       OPT_BOOLEAN('N', "intent-to-add", &intent_to_add, N_("record only the fact that the path will be added later")),
-       OPT_BOOLEAN('A', "all", &addremove, N_("add changes from all tracked and untracked files")),
-       OPT_BOOLEAN( 0 , "refresh", &refresh_only, N_("don't add, only refresh the index")),
-       OPT_BOOLEAN( 0 , "ignore-errors", &ignore_add_errors, N_("just skip files which cannot be added because of errors")),
-       OPT_BOOLEAN( 0 , "ignore-missing", &ignore_missing, N_("check if - even missing - files are ignored in dry run")),
+       OPT_BOOL('u', "update", &take_worktree_changes, N_("update tracked files")),
+       OPT_BOOL('N', "intent-to-add", &intent_to_add, N_("record only the fact that the path will be added later")),
+       OPT_BOOL('A', "all", &addremove_explicit, N_("add changes from all tracked and untracked files")),
+       OPT_BOOL( 0 , "refresh", &refresh_only, N_("don't add, only refresh the index")),
+       OPT_BOOL( 0 , "ignore-errors", &ignore_add_errors, N_("just skip files which cannot be added because of errors")),
+       OPT_BOOL( 0 , "ignore-missing", &ignore_missing, N_("check if - even missing - files are ignored in dry run")),
        OPT_END(),
  };
  
@@@ -405,6 -357,35 +430,6 @@@ static int add_files(struct dir_struct 
        return exit_status;
  }
  
 -static void warn_pathless_add(const char *option_name, const char *short_name) {
 -      /*
 -       * To be consistent with "git add -p" and most Git
 -       * commands, we should default to being tree-wide, but
 -       * this is not the original behavior and can't be
 -       * changed until users trained themselves not to type
 -       * "git add -u" or "git add -A". For now, we warn and
 -       * keep the old behavior. Later, this warning can be
 -       * turned into a die(...), and eventually we may
 -       * reallow the command with a new behavior.
 -       */
 -      warning(_("The behavior of 'git add %s (or %s)' with no path argument from a\n"
 -                "subdirectory of the tree will change in Git 2.0 and should not be used anymore.\n"
 -                "To add content for the whole tree, run:\n"
 -                "\n"
 -                "  git add %s :/\n"
 -                "  (or git add %s :/)\n"
 -                "\n"
 -                "To restrict the command to the current directory, run:\n"
 -                "\n"
 -                "  git add %s .\n"
 -                "  (or git add %s .)\n"
 -                "\n"
 -                "With the current Git version, the command is restricted to the current directory."),
 -              option_name, short_name,
 -              option_name, short_name,
 -              option_name, short_name);
 -}
 -
  int cmd_add(int argc, const char **argv, const char *prefix)
  {
        int exit_status = 0;
        int add_new_files;
        int require_pathspec;
        char *seen = NULL;
 -      const char *option_with_implicit_dot = NULL;
 -      const char *short_option_with_implicit_dot = NULL;
 +      int implicit_dot = 0;
+       struct update_callback_data update_data;
  
        git_config(add_config, NULL);
  
        argc--;
        argv++;
  
+       if (0 <= addremove_explicit)
+               addremove = addremove_explicit;
+       else if (take_worktree_changes && ADDREMOVE_DEFAULT)
+               addremove = 0; /* "-u" was given but not "-A" */
        if (addremove && take_worktree_changes)
                die(_("-A and -u are mutually incompatible"));
+       /*
+        * Warn when "git add pathspec..." was given without "-u" or "-A"
+        * and pathspec... covers a removed path.
+        */
+       memset(&update_data, 0, sizeof(update_data));
+       if (!take_worktree_changes && addremove_explicit < 0)
+               update_data.warn_add_would_remove = 1;
+       if (!take_worktree_changes && addremove_explicit < 0 && argc)
+               /*
+                * Turn "git add pathspec..." to "git add -A pathspec..."
+                * in Git 2.0 but not yet
+                */
+               ; /* addremove = 1; */
        if (!show_only && ignore_missing)
                die(_("Option --ignore-missing can only be used together with --dry-run"));
        if (addremove) {
        }
        if (option_with_implicit_dot && !argc) {
                static const char *here[2] = { ".", NULL };
 -              if (prefix)
 -                      warn_pathless_add(option_with_implicit_dot,
 -                                        short_option_with_implicit_dot);
                argc = 1;
                argv = here;
 +              implicit_dot = 1;
        }
  
        add_new_files = !take_worktree_changes && !refresh_only;
                 (intent_to_add ? ADD_CACHE_INTENT : 0) |
                 (ignore_add_errors ? ADD_CACHE_IGNORE_ERRORS : 0) |
                 (!(addremove || take_worktree_changes)
 -                ? ADD_CACHE_IGNORE_REMOVAL : 0));
 +                ? ADD_CACHE_IGNORE_REMOVAL : 0)) |
 +               (implicit_dot ? ADD_CACHE_IMPLICIT_DOT : 0);
  
        if (require_pathspec && argc == 0) {
                fprintf(stderr, _("Nothing specified, nothing added.\n"));
                }
  
                /* This picks up the paths that are not tracked */
 -              baselen = fill_directory(&dir, pathspec);
 +              baselen = fill_directory(&dir, implicit_dot ? NULL : pathspec);
                if (pathspec)
 -                      seen = prune_directory(&dir, pathspec, baselen);
 +                      seen = prune_directory(&dir, pathspec, baselen,
 +                                      implicit_dot ? WARN_IMPLICIT_DOT : 0);
        }
  
        if (refresh_only) {
  
        plug_bulk_checkin();
  
-       exit_status |= add_files_to_cache(prefix, pathspec, flags);
 -      update_data.flags = flags;
++      if ((flags & ADD_CACHE_IMPLICIT_DOT) && prefix) {
++              /*
++               * Check for modified files throughout the worktree so
++               * update_callback has a chance to warn about changes
++               * outside the cwd.
++               */
++              update_data.implicit_dot = prefix;
++              update_data.implicit_dot_len = strlen(prefix);
++              pathspec = NULL;
++      }
++      update_data.flags = flags & ~ADD_CACHE_IMPLICIT_DOT;
+       update_files_in_cache(prefix, pathspec, &update_data);
  
+       exit_status |= !!update_data.add_errors;
        if (add_new_files)
                exit_status |= add_files(&dir, flags);
  
diff --combined t/t2200-add-update.sh
index c317254b9a82befd3400d300f85ecfd661d1a948,32f932a4cef9e1c80f52def592a4e26d8877680a..b2bd41918ee7f8b19e3ef906f27796f331ed9ea5
@@@ -80,22 -80,6 +80,22 @@@ test_expect_success 'change gets notice
  
  '
  
 +# Note that this is scheduled to change in Git 2.0, when
 +# "git add -u" will become full-tree by default.
 +test_expect_success 'non-limited update in subdir leaves root alone' '
 +      (
 +              cd dir1 &&
 +              echo even more >>sub2 &&
 +              git add -u
 +      ) &&
 +      cat >expect <<-\EOF &&
 +      check
 +      top
 +      EOF
 +      git diff-files --name-only >actual &&
 +      test_cmp expect actual
 +'
 +
  test_expect_success SYMLINKS 'replace a file with a symlink' '
  
        rm foo &&
@@@ -166,9 -150,9 +166,9 @@@ test_expect_success 'add -u resolves un
        echo 2 >path3 &&
        echo 2 >path5 &&
  
-       # Explicit resolving by adding removed paths should fail
-       test_must_fail git add path4 &&
-       test_must_fail git add path6 &&
+       # Fail to explicitly resolve removed paths with "git add"
+       test_must_fail git add --no-all path4 &&
+       test_must_fail git add --no-all path6 &&
  
        # "add -u" should notice removals no matter what stages
        # the index entries are in.