Re: What's cooking in git.git (Apr 2013, #05; Mon, 15)
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Apr 18, 2013, 01:39 UTC
- Message-ID
- <7va9owd3d1.fsf@alter.siamese.dyndns.org>
- In-Reply-To
- <20130417201056.GA2914@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> Yeah, I had the same thought, as this warning has been bugging me for > the last day or two. The worst part about it is that I finally trained > myself to type "git add ." to silence the _other_ warning, and now it > triggers this one. :)
So here is the "reworked" one on top of what is in 'next'.
It introduces a bit of conflict with the "add -u/-A" topic, so I am not ready to push out the integration result yet.
-- >8 -- Subject: [PATCH] git add: rework the logic to warn "git add <pathspec>..." default change
The earlier logic to warn against "git add subdir" that is run without "-A" or "--no-all" was only to check any <pathspec> given exactly spells a directory name that (still) exists on the filesystem. This had number of problems:
* "git add '*dir'" (note that the wildcard is hidden from the shell) would not trigger the warning.
* "git add '*.py'" would behave differently between the current version of Git and Git 2.0 for the same reason as "subdir", but would not trigger the warning.
* "git add dir" for a submodule "dir" would just update the index entry for the submodule "dir" without ever recursing into it, and use of "-A" or "--no-all" would matter. But the logic only checks the directory-ness of "dir" and gives an unnecessary warning.
Rework the logic to detect the case where the behaviour will be different in Git 2.0, and issue a warning only when it matters. Even with the code before this warning, "git add subdir" will have to traverse the directory in order to find _new_ files the index does not know about _anyway_, so we can do this check without adding an extra pass to find if <pathspec> matches any removed file.
This essentially updates the "add_files_to_cache()" public API to "update_files_in_cache()" API that is internal to "git add", because with the "--all" option, the function is no longer about "adding" paths to the cache, but is also used to remove them.
There are other callers of the former from "checkout" (used when "checkout -m" prepares the temporary tree that represents the local modifications to be merged) and "commit" ("commit --include" that picks up local changes in addition to what is in the index). Since ADD_CACHE_IGNORE_ERRORS (aka "--no-all") is not used by either of them, once dust settles after Git 2.0 and the warning becomes unnecessary, we may want to unify these two functions again.
Signed-off-by: Junio C Hamano <gitster@pobox.com> --- builtin/add.c | 64 +++++++++++++++++++++++++++++++++++------------------------ 1 file changed, 38 insertions(+), 26 deletions(-)
diff --git a/builtin/add.c b/builtin/add.c index f8f6c9e..4242bce 100644 --- a/builtin/add.c +++ b/builtin/add.c @@ -26,6 +26,9 @@ static int take_worktree_changes; struct update_callback_data { int flags; int add_errors; + + /* only needed for 2.0 transition preparation */ + int warn_add_would_remove; }; static int fix_unmerged_status(struct diff_filepair *p, @@ -49,6 +52,17 @@ static int fix_unmerged_status(struct diff_filepair *p, return DIFF_STATUS_MODIFIED; } +static void warn_add_would_remove(const char *path) +{ + warning(_("In Git 2.0, 'git add <pathspec>...' will also update the\n" + "index for paths removed from the working tree that match\n" + "the given pathspec. If you want to 'add' only changed\n" + "or newly created paths, say 'git add --no-all <pathspec>...'" + " instead.\n\n" + "'%s' would be removed from the index without --no-all."), + path); +} + static void update_callback(struct diff_queue_struct *q, struct diff_options *opt, void *cbdata) { @@ -70,6 +84,10 @@ static void update_callback(struct diff_queue_struct *q, } 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)) @@ -81,20 +99,27 @@ static void update_callback(struct diff_queue_struct *q, } } -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; 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; - data.flags = flags; - data.add_errors = 0; - 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; } @@ -354,18 +379,6 @@ static void warn_pathless_add(const char *option_name, const char *short_name) { option_name, short_name); } -static int directory_given(int argc, const char **argv) -{ - struct stat st; - - while (argc--) { - if (!lstat(*argv, &st) && S_ISDIR(st.st_mode)) - return 1; - argv++; - } - return 0; -} - int cmd_add(int argc, const char **argv, const char *prefix) { int exit_status = 0; @@ -378,6 +391,7 @@ int cmd_add(int argc, const char **argv, const char *prefix) char *seen = NULL; const char *option_with_implicit_dot = NULL; const char *short_option_with_implicit_dot = NULL; + struct update_callback_data update_data; git_config(add_config, NULL); @@ -403,15 +417,11 @@ int cmd_add(int argc, const char **argv, const char *prefix) /* * Warn when "git add pathspec..." was given without "-u" or "-A" - * and pathspec... contains a directory name. + * and pathspec... covers a removed path. */ - if (!take_worktree_changes && addremove_explicit < 0 && - directory_given(argc, argv)) - warning(_("In Git 2.0, 'git add <pathspec>...' will also update the\n" - "index for paths removed from the working tree that match\n" - "the given pathspec. If you want to 'add' only changed\n" - "or newly created paths, say 'git add --no-all <pathspec>...'" - " instead.")); + 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) /* @@ -508,8 +518,10 @@ int cmd_add(int argc, const char **argv, const char *prefix) plug_bulk_checkin(); - exit_status |= add_files_to_cache(prefix, pathspec, flags); + update_data.flags = flags; + update_files_in_cache(prefix, pathspec, &update_data); + exit_status |= !!update_data.add_errors; if (add_new_files) exit_status |= add_files(&dir, flags);
-- 1.8.2.1-552-g964983e