@matheusbsbs / matheus-bagliano / commits / d226b14d47

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>

Junio C Hamano committed Apr 17, 2013 at 12:32 UTC d226b14d47311d74e2d55059a54594c3fe474b25

1 file changed +38 -26

builtin/add.c

+38 -26

@@ -26,6 +26,9 @@ static int take_worktree_changes;

26 struct update_callback_data {

27 int flags;

28 int add_errors;

29 +

30 + /* only needed for 2.0 transition preparation */

31 + int warn_add_would_remove;

32 };

34 static int fix_unmerged_status(struct diff_filepair *p,

@@ -49,6 +52,17 @@ static int fix_unmerged_status(struct diff_filepair *p,

52 return DIFF_STATUS_MODIFIED;

53 }

55 +static void warn_add_would_remove(const char *path)

56 +{

57 + warning(_("In Git 2.0, 'git add <pathspec>...' will also update the\n"

58 + "index for paths removed from the working tree that match\n"

59 + "the given pathspec. If you want to 'add' only changed\n"

60 + "or newly created paths, say 'git add --no-all <pathspec>...'"

61 + " instead.\n\n"

62 + "'%s' would be removed from the index without --no-all."),

63 + path);

64 +}

65 +

66 static void update_callback(struct diff_queue_struct *q,

67 struct diff_options *opt, void *cbdata)

68 {

@@ -70,6 +84,10 @@ static void update_callback(struct diff_queue_struct *q,

84 }

85 break;

86 case DIFF_STATUS_DELETED:

87 + if (data->warn_add_would_remove) {

88 + warn_add_would_remove(path);

89 + data->warn_add_would_remove = 0;

90 + }

91 if (data->flags & ADD_CACHE_IGNORE_REMOVAL)

92 break;

93 if (!(data->flags & ADD_CACHE_PRETEND))

@@ -81,20 +99,27 @@ static void update_callback(struct diff_queue_struct *q,

99 }

100 }

101

84 -int add_files_to_cache(const char *prefix, const char **pathspec, int flags)

102 +static void update_files_in_cache(const char *prefix, const char **pathspec,

103 + struct update_callback_data *data)

104 {

86 - struct update_callback_data data;

105 struct rev_info rev;

106 init_revisions(&rev, prefix);

107 setup_revisions(0, NULL, &rev, NULL);

108 init_pathspec(&rev.prune_data, pathspec);

109 rev.diffopt.output_format = DIFF_FORMAT_CALLBACK;

110 rev.diffopt.format_callback = update_callback;

93 - data.flags = flags;

94 - data.add_errors = 0;

95 - rev.diffopt.format_callback_data = &data;

111 + rev.diffopt.format_callback_data = data;

112 rev.max_count = 0; /* do not compare unmerged paths with stage #2 */

113 run_diff_files(&rev, DIFF_RACY_IS_MODIFIED);

114 +}

115 +

116 +int add_files_to_cache(const char *prefix, const char **pathspec, int flags)

117 +{

118 + struct update_callback_data data;

119 +

120 + memset(&data, 0, sizeof(data));

121 + data.flags = flags;

122 + update_files_in_cache(prefix, pathspec, &data);

123 return !!data.add_errors;

124 }

125

@@ -354,18 +379,6 @@ static void warn_pathless_add(const char *option_name, const char *short_name) {

379 option_name, short_name);

380 }

381

357 -static int directory_given(int argc, const char **argv)

358 -{

359 - struct stat st;

360 -

361 - while (argc--) {

362 - if (!lstat(*argv, &st) && S_ISDIR(st.st_mode))

363 - return 1;

364 - argv++;

365 - }

366 - return 0;

367 -}

368 -

382 int cmd_add(int argc, const char **argv, const char *prefix)

383 {

384 int exit_status = 0;

@@ -378,6 +391,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)

391 char *seen = NULL;

392 const char *option_with_implicit_dot = NULL;

393 const char *short_option_with_implicit_dot = NULL;

394 + struct update_callback_data update_data;

395

396 git_config(add_config, NULL);

397

@@ -403,15 +417,11 @@ int cmd_add(int argc, const char **argv, const char *prefix)

417

418 /*

419 * Warn when "git add pathspec..." was given without "-u" or "-A"

406 - * and pathspec... contains a directory name.

420 + * and pathspec... covers a removed path.

421 */

408 - if (!take_worktree_changes && addremove_explicit < 0 &&

409 - directory_given(argc, argv))

410 - warning(_("In Git 2.0, 'git add <pathspec>...' will also update the\n"

411 - "index for paths removed from the working tree that match\n"

412 - "the given pathspec. If you want to 'add' only changed\n"

413 - "or newly created paths, say 'git add --no-all <pathspec>...'"

414 - " instead."));

422 + memset(&update_data, 0, sizeof(update_data));

423 + if (!take_worktree_changes && addremove_explicit < 0)

424 + update_data.warn_add_would_remove = 1;

425

426 if (!take_worktree_changes && addremove_explicit < 0 && argc)

427 /*

@@ -508,8 +518,10 @@ int cmd_add(int argc, const char **argv, const char *prefix)

518

519 plug_bulk_checkin();

520

511 - exit_status |= add_files_to_cache(prefix, pathspec, flags);

521 + update_data.flags = flags;

522 + update_files_in_cache(prefix, pathspec, &update_data);

523

524 + exit_status |= !!update_data.add_errors;

525 if (add_new_files)

526 exit_status |= add_files(&dir, flags);

527