From 300c0a2209b753da1075770f7b696b1814c010f7 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Fri, 8 Mar 2013 22:21:13 -0800 Subject: builtin/add.c: simplify boolean variables Do not to explicitly initialize static variables to 0 and instead let BSS take care of it. Also use OPT_BOOL() to let the command line arguments set these variables to 0 or 1, instead of the deprecated OPT_BOOLEAN() aka OPT_COUNTUP(). Signed-off-by: Junio C Hamano diff --git a/builtin/add.c b/builtin/add.c index 0dd014e..220321b 100644 --- a/builtin/add.c +++ b/builtin/add.c @@ -270,23 +270,23 @@ 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, addremove, intent_to_add, ignore_missing; 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, 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(), }; -- cgit v0.10.2-6-g49f6 From 45c45e300bcbe493a39533bb04f6bd548e8a3f19 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Tue, 19 Apr 2011 12:18:20 -0700 Subject: git add: start preparing for "git add ..." to default to "-A" When "git add subdir/" is run without "-u" or "-A" option, e.g. $ edit subdir/x $ create subdir/y $ rm subdir/z $ git add subdir/ the command does not notice removal of paths (e.g. subdir/z) from the working tree. This sometimes confuses new people, as arguably "git add" is told to record the current state of "subdir/" as a whole, not the current state of the paths that exist in the working tree that matches that pathspec (the latter by definition excludes the state of "subdir/z" because it does not exist in the working tree). Plan to eventually make "git add" pretend as if "-A" is given when there is a pathspec on the command line. When resolving a conflict to remove a path, the current code tells you to "git rm $path", but with such a change, you will be able to say "git add $path" (of course you can do "git add -A $path" today). That means that we can simplify the advice messages given by "git status". That all will be in Git 2.0 or later, if we are going to do so. For that transition to work, people need to learn either to say "git add --no-all subdir/" when they want to ignore the removed paths like "subdir/z", or to say "git add -A subdir/" when they want to take the state of the directory as a whole. "git add" without any argument will continue to be a no-op. Signed-off-by: Junio C Hamano diff --git a/Documentation/git-add.txt b/Documentation/git-add.txt index b0944e5..5c501a2 100644 --- a/Documentation/git-add.txt +++ b/Documentation/git-add.txt @@ -9,7 +9,7 @@ SYNOPSIS -------- [verse] 'git add' [-n] [-v] [--force | -f] [--interactive | -i] [--patch | -p] - [--edit | -e] [--all | [--update | -u]] [--intent-to-add | -N] + [--edit | -e] [--[no-]all | [--update | -u]] [--intent-to-add | -N] [--refresh] [--ignore-errors] [--ignore-missing] [--] [...] @@ -121,6 +121,18 @@ If no is given, the current version of Git defaults to and its subdirectories. This default will change in a future version of Git, hence the form without should not be used. +--no-all:: + Update the index by adding new files that are unknown to the + index and files modified in the working tree, but ignore + files that have been removed from the working tree. This + option is a no-op when no is used. ++ +This option is primarily to help the current users of Git, whose +"git add ..." ignores removed files. In future versions +of Git, "git add ..." will be a synonym to "git add -A +..." and "git add --no-all ..." will behave like +today's "git add ...", ignoring removed files. + -N:: --intent-to-add:: Record only the fact that the path will be added later. An entry diff --git a/builtin/add.c b/builtin/add.c index 220321b..f8f6c9e 100644 --- a/builtin/add.c +++ b/builtin/add.c @@ -271,7 +271,11 @@ static const char ignore_error[] = N_("The following paths are ignored by one of your .gitignore files:\n"); static int verbose, show_only, ignored_too, refresh_only; -static int ignore_add_errors, addremove, intent_to_add, ignore_missing; +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")), @@ -283,7 +287,7 @@ static struct option builtin_add_options[] = { OPT__FORCE(&ignored_too, N_("allow adding otherwise ignored files")), 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, N_("add changes from all tracked and untracked files")), + 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")), @@ -350,6 +354,18 @@ 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; @@ -377,8 +393,33 @@ int cmd_add(int argc, const char **argv, const char *prefix) 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... contains a directory name. + */ + if (!take_worktree_changes && addremove_explicit < 0 && + directory_given(argc, argv)) + warning(_("In Git 2.0, 'git add ...' 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 ...'" + " instead.")); + + 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) { diff --git a/t/t2200-add-update.sh b/t/t2200-add-update.sh index 4cdebda..32f932a 100755 --- a/t/t2200-add-update.sh +++ b/t/t2200-add-update.sh @@ -150,9 +150,9 @@ test_expect_success 'add -u resolves unmerged paths' ' 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. -- cgit v0.10.2-6-g49f6 From d226b14d47311d74e2d55059a54594c3fe474b25 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Wed, 17 Apr 2013 12:32:21 -0700 Subject: git add: rework the logic to warn "git add ..." default change The earlier logic to warn against "git add subdir" that is run without "-A" or "--no-all" was only to check any 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 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 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 ...' 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 ...'" + " 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 ...' 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 ...'" - " 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); -- cgit v0.10.2-6-g49f6 From ccc663bc24a3925b818df0ce97a5ba47e221f383 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Sun, 21 Apr 2013 21:04:35 -0700 Subject: git add: rephrase the "removal will cease to be ignored" warning Now the logic to decide when to warn has been tightened, we know the user is in a situation where the current and future behaviours will be different. Spell out what happens with these two versions and how to explicitly ask for the behaviour, and suggest "git status" as a way to inspect the current status. Signed-off-by: Junio C Hamano diff --git a/builtin/add.c b/builtin/add.c index 4242bce..20f459a 100644 --- a/builtin/add.c +++ b/builtin/add.c @@ -52,15 +52,22 @@ static int fix_unmerged_status(struct diff_filepair *p, 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 ', which is the current default, ignores\n" +" paths you removed from your working tree.\n" +"\n" +"* 'git add --all ' 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(_("In Git 2.0, 'git add ...' 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 ...'" - " instead.\n\n" - "'%s' would be removed from the index without --no-all."), - path); + warning(_(add_would_remove_warning), path); } static void update_callback(struct diff_queue_struct *q, -- cgit v0.10.2-6-g49f6