Commit 4b136a94d948e62634633092c9d1052c4b074e6c

Russell Belfer 2012-03-23T09:26:09

Fix crash in new status and add recurse option This fixes the bug that @nulltoken found (thank you!) where if there were untracked directories alphabetically after the last tracked item, the diff implementation would deref a NULL pointer. The fix involved the code which decides if it is necessary to recurse into a directory in the working dir, so it was easy to add a new option `GIT_STATUS_OPT_RECURSE_UNTRACKED_DIRS` to control if the contents of untracked directories should be included in status.

diff --git a/include/git2/diff.h b/include/git2/diff.h
index cb3ef4e..0c9f620 100644
--- a/include/git2/diff.h
+++ b/include/git2/diff.h
@@ -40,7 +40,8 @@ enum {
 	GIT_DIFF_PATIENCE = (1 << 6),
 	GIT_DIFF_INCLUDE_IGNORED = (1 << 7),
 	GIT_DIFF_INCLUDE_UNTRACKED = (1 << 8),
-	GIT_DIFF_INCLUDE_UNMODIFIED = (1 << 9)
+	GIT_DIFF_INCLUDE_UNMODIFIED = (1 << 9),
+	GIT_DIFF_RECURSE_UNTRACKED_DIRS = (1 << 10),
 };
 
 /**
diff --git a/include/git2/status.h b/include/git2/status.h
index a24d39f..f5fc95f 100644
--- a/include/git2/status.h
+++ b/include/git2/status.h
@@ -87,19 +87,27 @@ typedef enum {
  *   should be made even on unmodified files.
  * - GIT_STATUS_OPT_EXCLUDE_SUBMODULES indicates that directories
  *   which appear to be submodules should just be skipped over.
+ * - GIT_STATUS_OPT_RECURSE_UNTRACKED_DIRS indicates that the
+ *   contents of untracked directories should be included in the
+ *   status.  Normally if an entire directory is new, then just
+ *   the top-level directory will be included (with a trailing
+ *   slash on the entry name).  Given this flag, the directory
+ *   itself will not be included, but all the files in it will.
  */
-#define GIT_STATUS_OPT_INCLUDE_UNTRACKED  (1 << 0)
-#define GIT_STATUS_OPT_INCLUDE_IGNORED    (1 << 1)
-#define GIT_STATUS_OPT_INCLUDE_UNMODIFIED (1 << 2)
-#define GIT_STATUS_OPT_EXCLUDE_SUBMODULES (1 << 3)
+#define GIT_STATUS_OPT_INCLUDE_UNTRACKED      (1 << 0)
+#define GIT_STATUS_OPT_INCLUDE_IGNORED        (1 << 1)
+#define GIT_STATUS_OPT_INCLUDE_UNMODIFIED     (1 << 2)
+#define GIT_STATUS_OPT_EXCLUDE_SUBMODULES     (1 << 3)
+#define GIT_STATUS_OPT_RECURSE_UNTRACKED_DIRS (1 << 4)
 
 /**
- * Options to control which callbacks will be made by
- * `git_status_foreach_ext()`
+ * Options to control how callbacks will be made by
+ * `git_status_foreach_ext()`.
  */
 typedef struct {
 	git_status_show_t show;
 	unsigned int flags;
+	git_strarray pathspec;
 } git_status_options;
 
 /**
diff --git a/src/diff.c b/src/diff.c
index 3f8041a..d5a841c 100644
--- a/src/diff.c
+++ b/src/diff.c
@@ -439,7 +439,12 @@ static int diff_from_iterators(
 			is_ignored = git_iterator_current_is_ignored(new);
 
 			if (S_ISDIR(nitem->mode)) {
-				if (git__prefixcmp(oitem->path, nitem->path) == 0) {
+				/* recurse into directory if explicitly requested or
+				 * if there are tracked items inside the directory
+				 */
+				if ((diff->opts.flags & GIT_DIFF_RECURSE_UNTRACKED_DIRS) ||
+					(oitem && git__prefixcmp(oitem->path, nitem->path) == 0))
+				{
 					if (is_ignored)
 						ignore_prefix = nitem->path;
 					if (git_iterator_advance_into_directory(new, &nitem) < 0)
diff --git a/src/status.c b/src/status.c
index 0c7a622..a0716e9 100644
--- a/src/status.c
+++ b/src/status.c
@@ -137,12 +137,16 @@ int git_status_foreach_ext(
 	}
 
 	memset(&diffopt, 0, sizeof(diffopt));
+	memcpy(&diffopt.pathspec, &opts->pathspec, sizeof(diffopt.pathspec));
+
 	if ((opts->flags & GIT_STATUS_OPT_INCLUDE_UNTRACKED) != 0)
 		diffopt.flags = diffopt.flags | GIT_DIFF_INCLUDE_UNTRACKED;
 	if ((opts->flags & GIT_STATUS_OPT_INCLUDE_IGNORED) != 0)
 		diffopt.flags = diffopt.flags | GIT_DIFF_INCLUDE_IGNORED;
 	if ((opts->flags & GIT_STATUS_OPT_INCLUDE_UNMODIFIED) != 0)
 		diffopt.flags = diffopt.flags | GIT_DIFF_INCLUDE_UNMODIFIED;
+	if ((opts->flags & GIT_STATUS_OPT_RECURSE_UNTRACKED_DIRS) != 0)
+		diffopt.flags = diffopt.flags | GIT_DIFF_RECURSE_UNTRACKED_DIRS;
 	/* TODO: support EXCLUDE_SUBMODULES flag */
 
 	if (show != GIT_STATUS_SHOW_WORKDIR_ONLY &&
diff --git a/tests-clar/status/status_data.h b/tests-clar/status/status_data.h
index 3957768..e60b67c 100644
--- a/tests-clar/status/status_data.h
+++ b/tests-clar/status/status_data.h
@@ -143,3 +143,60 @@ static const unsigned int entry_statuses3[] = {
 };
 
 static const size_t entry_count3 = 21;
+
+
+/* entries for a copy of tests/resources/status with some mods
+ * and different options to the status call
+ */
+
+static const char *entry_paths4[] = {
+	".new_file",
+	"current_file",
+	"current_file/current_file",
+	"current_file/modified_file",
+	"current_file/new_file",
+	"file_deleted",
+	"modified_file",
+	"new_file",
+	"staged_changes",
+	"staged_changes_file_deleted",
+	"staged_changes_modified_file",
+	"staged_delete_file_deleted",
+	"staged_delete_modified_file",
+	"staged_new_file",
+	"staged_new_file_deleted_file",
+	"staged_new_file_modified_file",
+	"subdir",
+	"subdir/current_file",
+	"subdir/deleted_file",
+	"subdir/modified_file",
+	"zzz_new_dir/new_file",
+	"zzz_new_file"
+};
+
+static const unsigned int entry_statuses4[] = {
+	GIT_STATUS_WT_NEW,
+	GIT_STATUS_WT_DELETED,
+	GIT_STATUS_WT_NEW,
+	GIT_STATUS_WT_NEW,
+	GIT_STATUS_WT_NEW,
+	GIT_STATUS_WT_DELETED,
+	GIT_STATUS_WT_MODIFIED,
+	GIT_STATUS_WT_NEW,
+	GIT_STATUS_INDEX_MODIFIED,
+	GIT_STATUS_WT_DELETED | GIT_STATUS_INDEX_MODIFIED,
+	GIT_STATUS_WT_MODIFIED | GIT_STATUS_INDEX_MODIFIED,
+	GIT_STATUS_INDEX_DELETED,
+	GIT_STATUS_WT_NEW | GIT_STATUS_INDEX_DELETED,
+	GIT_STATUS_INDEX_NEW,
+	GIT_STATUS_WT_DELETED | GIT_STATUS_INDEX_NEW,
+	GIT_STATUS_WT_MODIFIED | GIT_STATUS_INDEX_NEW,
+	GIT_STATUS_WT_NEW,
+	GIT_STATUS_WT_DELETED,
+	GIT_STATUS_WT_DELETED,
+	GIT_STATUS_WT_DELETED,
+	GIT_STATUS_WT_NEW,
+	GIT_STATUS_WT_NEW,
+};
+
+static const size_t entry_count4 = 22;
diff --git a/tests-clar/status/worktree.c b/tests-clar/status/worktree.c
index b9000bb..dbc2fee 100644
--- a/tests-clar/status/worktree.c
+++ b/tests-clar/status/worktree.c
@@ -174,6 +174,41 @@ void test_status_worktree__swap_subdir_and_file(void)
 
 }
 
+void test_status_worktree__swap_subdir_with_recurse_and_pathspec(void)
+{
+	struct status_entry_counts counts;
+	git_repository *repo = cl_git_sandbox_init("status");
+	git_status_options opts;
+
+	/* first alter the contents of the worktree */
+	cl_git_pass(p_rename("status/current_file", "status/swap"));
+	cl_git_pass(p_rename("status/subdir", "status/current_file"));
+	cl_git_pass(p_rename("status/swap", "status/subdir"));
+	cl_git_mkfile("status/.new_file", "dummy");
+	cl_git_pass(git_futils_mkdir_r("status/zzz_new_dir", NULL, 0777));
+	cl_git_mkfile("status/zzz_new_dir/new_file", "dummy");
+	cl_git_mkfile("status/zzz_new_file", "dummy");
+
+	/* now get status */
+	memset(&counts, 0x0, sizeof(struct status_entry_counts));
+	counts.expected_entry_count = entry_count4;
+	counts.expected_paths = entry_paths4;
+	counts.expected_statuses = entry_statuses4;
+
+	memset(&opts, 0, sizeof(opts));
+	opts.flags = GIT_STATUS_OPT_INCLUDE_UNTRACKED |
+		GIT_STATUS_OPT_RECURSE_UNTRACKED_DIRS;
+	/* TODO: set pathspec to "current_file" eventually */
+
+	cl_git_pass(
+		git_status_foreach_ext(repo, &opts, cb_status__normal, &counts)
+	);
+
+	cl_assert(counts.entry_count == counts.expected_entry_count);
+	cl_assert(counts.wrong_status_flags_count == 0);
+	cl_assert(counts.wrong_sorted_path == 0);
+}
+
 /* this test is equivalent to t18-status.c:singlestatus0 */
 void test_status_worktree__single_file(void)
 {