diff --git a/src/diff.c b/src/diff.c index 61fd18d89..7152683e7 100644 --- a/src/diff.c +++ b/src/diff.c @@ -572,7 +572,13 @@ static int maybe_modified( return -1; use_noid = &noid; } - if (omode == nmode && git_oid_equal(&oitem->oid, use_noid)) + + /* if oid matches, then mark unmodified (except submodules, where + * the filesystem content may be modified even if the oid still + * matches between the index and the workdir HEAD) + */ + if (omode == nmode && !S_ISGITLINK(omode) && + git_oid_equal(&oitem->oid, use_noid)) status = GIT_DELTA_UNMODIFIED; } @@ -725,14 +731,20 @@ int git_diff__from_iterators( recurse_into_dir = false; } - if (contains_oitem || recurse_into_dir) { - /* if this directory is ignored, remember it as the - * "ignore_prefix" for processing contained items - */ - if (delta_type == GIT_DELTA_UNTRACKED && - git_iterator_current_is_ignored(new_iter)) - git_buf_sets(&ignore_prefix, nitem->path); + /* if directory is ignored, remember ignore_prefix */ + if ((contains_oitem || recurse_into_dir) && + delta_type == GIT_DELTA_UNTRACKED && + git_iterator_current_is_ignored(new_iter)) + { + git_buf_sets(&ignore_prefix, nitem->path); + delta_type = GIT_DELTA_IGNORED; + /* skip recursion if we've just learned this is ignored */ + if (DIFF_FLAG_ISNT_SET(diff, GIT_DIFF_RECURSE_IGNORED_DIRS)) + recurse_into_dir = false; + } + + if (contains_oitem || recurse_into_dir) { /* advance into directory */ error = git_iterator_advance_into(&nitem, new_iter); diff --git a/tests-clar/diff/diff_helpers.c b/tests-clar/diff/diff_helpers.c index a1f75ce39..19c005e2e 100644 --- a/tests-clar/diff/diff_helpers.c +++ b/tests-clar/diff/diff_helpers.c @@ -42,6 +42,16 @@ int diff_file_cb( return 0; } +int diff_print_file_cb( + const git_diff_delta *delta, + float progress, + void *payload) +{ + fprintf(stderr, "%c %s\n", + git_diff_status_char(delta->status), delta->old_file.path); + return diff_file_cb(delta, progress, payload); +} + int diff_hunk_cb( const git_diff_delta *delta, const git_diff_range *range, diff --git a/tests-clar/diff/diff_helpers.h b/tests-clar/diff/diff_helpers.h index a43847b79..674fd8e19 100644 --- a/tests-clar/diff/diff_helpers.h +++ b/tests-clar/diff/diff_helpers.h @@ -30,6 +30,11 @@ extern int diff_file_cb( float progress, void *cb_data); +extern int diff_print_file_cb( + const git_diff_delta *delta, + float progress, + void *cb_data); + extern int diff_hunk_cb( const git_diff_delta *delta, const git_diff_range *range, diff --git a/tests-clar/diff/workdir.c b/tests-clar/diff/workdir.c index f67f09ae8..fc95cf8b4 100644 --- a/tests-clar/diff/workdir.c +++ b/tests-clar/diff/workdir.c @@ -953,16 +953,31 @@ void test_diff_workdir__submodules(void) cl_git_pass(git_diff_foreach( diff, diff_file_cb, diff_hunk_cb, diff_line_cb, &exp)); - /* the following differs from "git diff 873585" by two "untracked" file - * because the diff list includes the "not" and "not-submodule" dirs which - * are not displayed in the text diff. + /* so "git diff 873585" returns: + * M .gitmodules + * A just_a_dir/contents + * A just_a_file + * A sm_added_and_uncommited + * A sm_changed_file + * A sm_changed_head + * A sm_changed_index + * A sm_changed_untracked_file + * M sm_missing_commits + * A sm_unchanged + * which is a little deceptive because of the difference between the + * "git diff " results from "git_diff_tree_to_workdir". The + * only significant difference is that those Added items will show up + * as Untracked items in the pure libgit2 diff. + * + * Then add in the two extra untracked items "not" and "not-submodule" + * to get the 12 files reported here. */ - cl_assert_equal_i(11, exp.files); + cl_assert_equal_i(12, exp.files); cl_assert_equal_i(0, exp.file_status[GIT_DELTA_ADDED]); cl_assert_equal_i(0, exp.file_status[GIT_DELTA_DELETED]); - cl_assert_equal_i(1, exp.file_status[GIT_DELTA_MODIFIED]); + cl_assert_equal_i(2, exp.file_status[GIT_DELTA_MODIFIED]); cl_assert_equal_i(0, exp.file_status[GIT_DELTA_IGNORED]); cl_assert_equal_i(10, exp.file_status[GIT_DELTA_UNTRACKED]); diff --git a/tests-clar/status/ignore.c b/tests-clar/status/ignore.c index 65ab47267..2d3898ba4 100644 --- a/tests-clar/status/ignore.c +++ b/tests-clar/status/ignore.c @@ -199,12 +199,15 @@ void test_status_ignore__subdirectories(void) cl_git_pass(git_status_should_ignore(&ignored, g_repo, "ignore_me")); cl_assert(ignored); - /* So, interestingly, as per the comment in diff_from_iterators() the - * following file is ignored, but in a way so that it does not show up - * in status even if INCLUDE_IGNORED is used. This actually matches - * core git's behavior - if you follow these steps and try running "git - * status -uall --ignored" then the following file and directory will - * not show up in the output at all. + /* I've changed libgit2 so that the behavior here now differs from + * core git but seems to make more sense. In core git, the following + * items are skipped completed, even if --ignored is passed to status. + * It you mirror these steps and run "git status -uall --ignored" then + * you will not see "test/ignore_me/" in the results. + * + * However, we had a couple reports of this as a bug, plus there is a + * similar circumstance where we were differing for core git when you + * used a rooted path for an ignore, so I changed this behavior. */ cl_git_pass(git_futils_mkdir_r( "empty_standard_repo/test/ignore_me", NULL, 0775)); @@ -215,7 +218,7 @@ void test_status_ignore__subdirectories(void) memset(&st, 0, sizeof(st)); cl_git_pass(git_status_foreach(g_repo, cb_status__single, &st)); - cl_assert_equal_i(2, st.count); + cl_assert_equal_i(3, st.count); cl_git_pass(git_status_file(&st.status, g_repo, "test/ignore_me/file")); cl_assert(st.status == GIT_STATUS_IGNORED); @@ -230,26 +233,38 @@ void test_status_ignore__subdirectories_recursion(void) /* Let's try again with recursing into ignored dirs turned on */ git_status_options opts = GIT_STATUS_OPTIONS_INIT; status_entry_counts counts; - static const char *paths[] = { + static const char *paths_r[] = { ".gitignore", + "ignore_also/file", "ignore_me", "test/ignore_me/and_me/file", "test/ignore_me/file", "test/ignore_me/file2", }; - static const unsigned int statuses[] = { + static const unsigned int statuses_r[] = { GIT_STATUS_WT_NEW, GIT_STATUS_IGNORED, GIT_STATUS_IGNORED, GIT_STATUS_IGNORED, GIT_STATUS_IGNORED, + GIT_STATUS_IGNORED, + }; + static const char *paths_nr[] = { + ".gitignore", + "ignore_also/", + "ignore_me", + "test/ignore_me/", + }; + static const unsigned int statuses_nr[] = { + GIT_STATUS_WT_NEW, + GIT_STATUS_IGNORED, + GIT_STATUS_IGNORED, + GIT_STATUS_IGNORED, }; - - opts.flags = GIT_STATUS_OPT_DEFAULTS | GIT_STATUS_OPT_RECURSE_IGNORED_DIRS; g_repo = cl_git_sandbox_init("empty_standard_repo"); - cl_git_rewritefile("empty_standard_repo/.gitignore", "ignore_me\n"); + cl_git_rewritefile("empty_standard_repo/.gitignore", "ignore_me\n/ignore_also\n"); cl_git_mkfile( "empty_standard_repo/ignore_me", "I'm going to be ignored!"); @@ -263,11 +278,32 @@ void test_status_ignore__subdirectories_recursion(void) "empty_standard_repo/test/ignore_me/and_me", NULL, 0775)); cl_git_mkfile( "empty_standard_repo/test/ignore_me/and_me/file", "Deeply ignored"); + cl_git_pass(git_futils_mkdir_r( + "empty_standard_repo/ignore_also", NULL, 0775)); + cl_git_mkfile( + "empty_standard_repo/ignore_also/file", "I'm going to be ignored!"); memset(&counts, 0x0, sizeof(status_entry_counts)); - counts.expected_entry_count = 5; - counts.expected_paths = paths; - counts.expected_statuses = statuses; + counts.expected_entry_count = 6; + counts.expected_paths = paths_r; + counts.expected_statuses = statuses_r; + + opts.flags = GIT_STATUS_OPT_DEFAULTS | GIT_STATUS_OPT_RECURSE_IGNORED_DIRS; + + cl_git_pass(git_status_foreach_ext( + g_repo, &opts, cb_status__normal, &counts)); + + cl_assert_equal_i(counts.expected_entry_count, counts.entry_count); + cl_assert_equal_i(0, counts.wrong_status_flags_count); + cl_assert_equal_i(0, counts.wrong_sorted_path); + + + memset(&counts, 0x0, sizeof(status_entry_counts)); + counts.expected_entry_count = 4; + counts.expected_paths = paths_nr; + counts.expected_statuses = statuses_nr; + + opts.flags = GIT_STATUS_OPT_DEFAULTS; cl_git_pass(git_status_foreach_ext( g_repo, &opts, cb_status__normal, &counts)); diff --git a/tests-clar/status/status_helpers.c b/tests-clar/status/status_helpers.c index 3dbf43a5b..24546d45c 100644 --- a/tests-clar/status/status_helpers.c +++ b/tests-clar/status/status_helpers.c @@ -47,3 +47,51 @@ int cb_status__single(const char *p, unsigned int s, void *payload) return 0; } + +int cb_status__print( + const char *path, unsigned int status_flags, void *payload) +{ + char istatus = ' ', wstatus = ' '; + int icount = 0, wcount = 0; + + if (status_flags & GIT_STATUS_INDEX_NEW) { + istatus = 'A'; icount++; + } + if (status_flags & GIT_STATUS_INDEX_MODIFIED) { + istatus = 'M'; icount++; + } + if (status_flags & GIT_STATUS_INDEX_DELETED) { + istatus = 'D'; icount++; + } + if (status_flags & GIT_STATUS_INDEX_RENAMED) { + istatus = 'R'; icount++; + } + if (status_flags & GIT_STATUS_INDEX_TYPECHANGE) { + istatus = 'T'; icount++; + } + + if (status_flags & GIT_STATUS_WT_NEW) { + wstatus = 'A'; wcount++; + } + if (status_flags & GIT_STATUS_WT_MODIFIED) { + wstatus = 'M'; wcount++; + } + if (status_flags & GIT_STATUS_WT_DELETED) { + wstatus = 'D'; wcount++; + } + if (status_flags & GIT_STATUS_WT_TYPECHANGE) { + wstatus = 'T'; wcount++; + } + if (status_flags & GIT_STATUS_IGNORED) { + wstatus = 'I'; wcount++; + } + + fprintf(stderr, "%c%c %s (%d/%d%s)\n", + istatus, wstatus, path, icount, wcount, + (icount > 1 || wcount > 1) ? " INVALID COMBO" : ""); + + if (payload) + *((int *)payload) += 1; + + return 0; +} diff --git a/tests-clar/status/status_helpers.h b/tests-clar/status/status_helpers.h index 3f9c1f57d..1aa0263ee 100644 --- a/tests-clar/status/status_helpers.h +++ b/tests-clar/status/status_helpers.h @@ -30,4 +30,8 @@ typedef struct { extern int cb_status__single(const char *p, unsigned int s, void *payload); +/* cb_status__print takes optional payload of "int *" */ + +extern int cb_status__print(const char *p, unsigned int s, void *payload); + #endif diff --git a/tests-clar/status/submodules.c b/tests-clar/status/submodules.c index aa88f06a0..6a9bf75e8 100644 --- a/tests-clar/status/submodules.c +++ b/tests-clar/status/submodules.c @@ -166,3 +166,54 @@ void test_status_submodules__moved_head(void) git_status_foreach_ext(g_repo, &opts, cb_status__match, &counts)); cl_assert_equal_i(6, counts.entry_count); } + +void test_status_submodules__dirty_workdir_only(void) +{ + git_status_options opts = GIT_STATUS_OPTIONS_INIT; + status_entry_counts counts; + static const char *expected_files_with_sub[] = { + ".gitmodules", + "added", + "deleted", + "ignored", + "modified", + "testrepo", + "untracked" + }; + static unsigned int expected_status_with_sub[] = { + GIT_STATUS_WT_MODIFIED, + GIT_STATUS_INDEX_NEW, + GIT_STATUS_INDEX_DELETED, + GIT_STATUS_IGNORED, + GIT_STATUS_WT_MODIFIED, + GIT_STATUS_WT_MODIFIED, + GIT_STATUS_WT_NEW + }; + + cl_git_rewritefile("submodules/testrepo/README", "heyheyhey"); + cl_git_mkfile("submodules/testrepo/all_new.txt", "never seen before"); + + /* first do a normal status, which should now include the submodule */ + + memset(&counts, 0, sizeof(counts)); + counts.expected_paths = expected_files_with_sub; + counts.expected_statuses = expected_status_with_sub; + + opts.flags = GIT_STATUS_OPT_DEFAULTS; + + cl_git_pass( + git_status_foreach_ext(g_repo, &opts, cb_status__match, &counts)); + cl_assert_equal_i(7, counts.entry_count); + + /* try again with EXCLUDE_SUBMODULES which should skip it */ + + memset(&counts, 0, sizeof(counts)); + counts.expected_paths = expected_files; + counts.expected_statuses = expected_status; + + opts.flags = GIT_STATUS_OPT_DEFAULTS | GIT_STATUS_OPT_EXCLUDE_SUBMODULES; + + cl_git_pass( + git_status_foreach_ext(g_repo, &opts, cb_status__match, &counts)); + cl_assert_equal_i(6, counts.entry_count); +}