Merge pull request #1588 from arrbee/fixes-for-checkout-and-diff

Bug fixes for checkout and diff

include/git2/index.h

@@ -57,6 +57,8 @@ GIT_BEGIN_DECL
 
 #define GIT_IDXENTRY_EXTENDED_FLAGS (GIT_IDXENTRY_INTENT_TO_ADD | GIT_IDXENTRY_SKIP_WORKTREE)
 
+#define GIT_IDXENTRY_STAGE(E) (((E)->flags & GIT_IDXENTRY_STAGEMASK) >> GIT_IDXENTRY_STAGESHIFT)
+
 /** Time used in a git index entry */
 typedef struct {
 	git_time_t seconds;

src/checkout.c

@@ -189,6 +189,10 @@ static int checkout_action_common(
 			action = (action & ~CHECKOUT_ACTION__UPDATE_BLOB) |
 				CHECKOUT_ACTION__UPDATE_SUBMODULE;
 
+		/* to "update" a symlink, we must remove the old one first */
+		if (delta->new_file.mode == GIT_FILEMODE_LINK && wd != NULL)
+			action |= CHECKOUT_ACTION__REMOVE;
+
 		notify = GIT_CHECKOUT_NOTIFY_UPDATED;
 	}
 
@@ -764,7 +768,11 @@ cleanup:
 }
 
 static int blob_content_to_link(
-	struct stat *st, git_blob *blob, const char *path, mode_t dir_mode, int can_symlink)
+	struct stat *st,
+	git_blob *blob,
+	const char *path,
+	mode_t dir_mode,
+	int can_symlink)
 {
 	git_buf linktarget = GIT_BUF_INIT;
 	int error;
@@ -777,7 +785,7 @@ static int blob_content_to_link(
 
 	if (can_symlink) {
 		if ((error = p_symlink(git_buf_cstr(&linktarget), path)) < 0)
-			giterr_set(GITERR_CHECKOUT, "Could not create symlink %s\n", path);
+			giterr_set(GITERR_OS, "Could not create symlink %s\n", path);
 	} else {
 		error = git_futils_fake_symlink(git_buf_cstr(&linktarget), path);
 	}
@@ -812,6 +820,31 @@ static int checkout_update_index(
 	return git_index_add(data->index, &entry);
 }
 
+static int checkout_submodule_update_index(
+	checkout_data *data,
+	const git_diff_file *file)
+{
+	struct stat st;
+
+	/* update the index unless prevented */
+	if ((data->strategy & GIT_CHECKOUT_DONT_UPDATE_INDEX) != 0)
+		return 0;
+
+	git_buf_truncate(&data->path, data->workdir_len);
+	if (git_buf_puts(&data->path, file->path) < 0)
+		return -1;
+
+	if (p_stat(git_buf_cstr(&data->path), &st) < 0) {
+		giterr_set(
+			GITERR_CHECKOUT, "Could not stat submodule %s\n", file->path);
+		return GIT_ENOTFOUND;
+	}
+
+	st.st_mode = GIT_FILEMODE_COMMIT;
+
+	return checkout_update_index(data, file, &st);
+}
+
 static int checkout_submodule(
 	checkout_data *data,
 	const git_diff_file *file)
@@ -828,8 +861,17 @@ static int checkout_submodule(
 			data->opts.dir_mode, GIT_MKDIR_PATH)) < 0)
 		return error;
 
-	if ((error = git_submodule_lookup(&sm, data->repo, file->path)) < 0)
+	if ((error = git_submodule_lookup(&sm, data->repo, file->path)) < 0) {
+		/* I've observed repos with submodules in the tree that do not
+		 * have a .gitmodules - core Git just makes an empty directory
+		 */
+		if (error == GIT_ENOTFOUND) {
+			giterr_clear();
+			return checkout_submodule_update_index(data, file);
+		}
+
 		return error;
+	}
 
 	/* TODO: Support checkout_strategy options.  Two circumstances:
 	 * 1 - submodule already checked out, but we need to move the HEAD
@@ -840,26 +882,7 @@ static int checkout_submodule(
 	 * command should probably be able to.  Do we need a submodule callback?
 	 */
 
-	/* update the index unless prevented */
-	if ((data->strategy & GIT_CHECKOUT_DONT_UPDATE_INDEX) == 0) {
-		struct stat st;
-
-		git_buf_truncate(&data->path, data->workdir_len);
-		if (git_buf_puts(&data->path, file->path) < 0)
-			return -1;
-
-		if ((error = p_stat(git_buf_cstr(&data->path), &st)) < 0) {
-			giterr_set(
-				GITERR_CHECKOUT, "Could not stat submodule %s\n", file->path);
-			return error;
-		}
-
-		st.st_mode = GIT_FILEMODE_COMMIT;
-
-		error = checkout_update_index(data, file, &st);
-	}
-
-	return error;
+	return checkout_submodule_update_index(data, file);
 }
 
 static void report_progress(

src/diff.c

@@ -747,7 +747,8 @@ static int diff_scan_inside_untracked_dir(
 	}
 
 	/* look for actual untracked file */
-	while (!diff->pfxcomp(info->nitem->path, git_buf_cstr(&base))) {
+	while (info->nitem != NULL &&
+		   !diff->pfxcomp(info->nitem->path, git_buf_cstr(&base))) {
 		is_ignored = git_iterator_current_is_ignored(info->new_iter);
 
 		/* need to recurse into non-ignored directories */
@@ -769,7 +770,8 @@ static int diff_scan_inside_untracked_dir(
 	}
 
 	/* finish off scan */
-	while (!diff->pfxcomp(info->nitem->path, git_buf_cstr(&base))) {
+	while (info->nitem != NULL &&
+		   !diff->pfxcomp(info->nitem->path, git_buf_cstr(&base))) {
 		if ((error = git_iterator_advance(&info->nitem, info->new_iter)) < 0)
 			break;
 	}

src/diff_tform.c

@@ -386,8 +386,12 @@ static int similarity_calc(
 
 		/* TODO: apply wd-to-odb filters to file data if necessary */
 
-		if (!(error = git_buf_joinpath(
-				&path, git_repository_workdir(diff->repo), file->path)))
+		if ((error = git_buf_joinpath(
+				 &path, git_repository_workdir(diff->repo), file->path)) < 0)
+			return error;
+
+		/* if path is not a regular file, just skip this item */
+		if (git_path_isfile(path.ptr))
 			error = opts->metric->file_signature(
 				&cache[file_idx], file, path.ptr, opts->metric->payload);
 
@@ -398,8 +402,11 @@ static int similarity_calc(
 
 		/* TODO: add max size threshold a la diff? */
 
-		if ((error = git_blob_lookup(&blob, diff->repo, &file->oid)) < 0)
-			return error;
+		if (git_blob_lookup(&blob, diff->repo, &file->oid) < 0) {
+			/* if lookup fails, just skip this item in similarity calc */
+			giterr_clear();
+			return 0;
+		}
 
 		blobsize = git_blob_rawsize(blob);
 		if (!git__is_sizet(blobsize)) /* ? what to do ? */

src/index.c

@@ -104,11 +104,6 @@ static int index_find(size_t *at_pos, git_index *index, const char *path, int st
 static void index_entry_free(git_index_entry *entry);
 static void index_entry_reuc_free(git_index_reuc_entry *reuc);
 
-GIT_INLINE(int) index_entry_stage(const git_index_entry *entry)
-{
-	return (entry->flags & GIT_IDXENTRY_STAGEMASK) >> GIT_IDXENTRY_STAGESHIFT;
-}
-
 static int index_srch(const void *key, const void *array_member)
 {
 	const struct entry_srch_key *srch_key = key;
@@ -118,7 +113,7 @@ static int index_srch(const void *key, const void *array_member)
 	ret = strcmp(srch_key->path, entry->path);
 
 	if (ret == 0)
-		ret = srch_key->stage - index_entry_stage(entry);
+		ret = srch_key->stage - GIT_IDXENTRY_STAGE(entry);
 
 	return ret;
 }
@@ -132,7 +127,7 @@ static int index_isrch(const void *key, const void *array_member)
 	ret = strcasecmp(srch_key->path, entry->path);
 
 	if (ret == 0)
-		ret = srch_key->stage - index_entry_stage(entry);
+		ret = srch_key->stage - GIT_IDXENTRY_STAGE(entry);
 
 	return ret;
 }
@@ -170,7 +165,7 @@ static int index_cmp(const void *a, const void *b)
 	diff = strcmp(entry_a->path, entry_b->path);
 
 	if (diff == 0)
-		diff = (index_entry_stage(entry_a) - index_entry_stage(entry_b));
+		diff = (GIT_IDXENTRY_STAGE(entry_a) - GIT_IDXENTRY_STAGE(entry_b));
 
 	return diff;
 }
@@ -184,7 +179,7 @@ static int index_icmp(const void *a, const void *b)
 	diff = strcasecmp(entry_a->path, entry_b->path);
 
 	if (diff == 0)
-		diff = (index_entry_stage(entry_a) - index_entry_stage(entry_b));
+		diff = (GIT_IDXENTRY_STAGE(entry_a) - GIT_IDXENTRY_STAGE(entry_b));
 
 	return diff;
 }
@@ -721,7 +716,7 @@ static int index_insert(git_index *index, git_index_entry *entry, int replace)
 		entry->flags |= GIT_IDXENTRY_NAMEMASK;
 
 	/* look if an entry with this path already exists */
-	if (!index_find(&position, index, entry->path, index_entry_stage(entry))) {
+	if (!index_find(&position, index, entry->path, GIT_IDXENTRY_STAGE(entry))) {
 		existing = (git_index_entry **)&index->entries.contents[position];
 
 		/* update filemode to existing values if stat is not trusted */
@@ -869,7 +864,7 @@ int git_index_remove_directory(git_index *index, const char *dir, int stage)
 		if (!entry || git__prefixcmp(entry->path, pfx.ptr) != 0)
 			break;
 
-		if (index_entry_stage(entry) != stage) {
+		if (GIT_IDXENTRY_STAGE(entry) != stage) {
 			++pos;
 			continue;
 		}
@@ -1008,7 +1003,7 @@ int git_index_conflict_get(git_index_entry **ancestor_out,
 		if (index->entries_cmp_path(conflict_entry->path, path) != 0)
 			break;
 
-		stage = index_entry_stage(conflict_entry);
+		stage = GIT_IDXENTRY_STAGE(conflict_entry);
 
 		switch (stage) {
 		case 3:
@@ -1050,7 +1045,7 @@ int git_index_conflict_remove(git_index *index, const char *path)
 		if (index->entries_cmp_path(conflict_entry->path, path) != 0)
 			break;
 
-		if (index_entry_stage(conflict_entry) == 0) {
+		if (GIT_IDXENTRY_STAGE(conflict_entry) == 0) {
 			pos++;
 			continue;
 		}
@@ -1069,7 +1064,7 @@ static int index_conflicts_match(const git_vector *v, size_t idx)
 {
 	git_index_entry *entry = git_vector_get(v, idx);
 
-	if (index_entry_stage(entry) > 0) {
+	if (GIT_IDXENTRY_STAGE(entry) > 0) {
 		index_entry_free(entry);
 		return 1;
 	}
@@ -1091,7 +1086,7 @@ int git_index_has_conflicts(const git_index *index)
 	assert(index);
 
 	git_vector_foreach(&index->entries, i, entry) {
-		if (index_entry_stage(entry) > 0)
+		if (GIT_IDXENTRY_STAGE(entry) > 0)
 			return 1;
 	}
 
@@ -1858,7 +1853,7 @@ static int write_index(git_index *index, git_filebuf *file)
 
 int git_index_entry_stage(const git_index_entry *entry)
 {
-	return index_entry_stage(entry);
+	return GIT_IDXENTRY_STAGE(entry);
 }
 
 typedef struct read_tree_data {

tests-clar/diff/workdir.c

@@ -1220,3 +1220,28 @@ void test_diff_workdir__untracked_directory_scenarios(void)
 
 	git_diff_list_free(diff);
 }
+
+
+void test_diff_workdir__untracked_directory_comes_last(void)
+{
+	git_diff_options opts = GIT_DIFF_OPTIONS_INIT;
+	git_diff_list *diff = NULL;
+
+	g_repo = cl_git_sandbox_init("renames");
+
+	cl_git_mkfile("renames/.gitignore", "*.ign\n");
+	cl_git_pass(p_mkdir("renames/zzz_untracked", 0777));
+	cl_git_mkfile("renames/zzz_untracked/an.ign", "ignore me please");
+	cl_git_mkfile("renames/zzz_untracked/skip.ign", "ignore me really");
+	cl_git_mkfile("renames/zzz_untracked/test.ign", "ignore me now");
+
+	opts.context_lines = 3;
+	opts.interhunk_lines = 1;
+	opts.flags |= GIT_DIFF_INCLUDE_IGNORED | GIT_DIFF_INCLUDE_UNTRACKED;
+
+	cl_git_pass(git_diff_index_to_workdir(&diff, g_repo, NULL, &opts));
+
+	cl_assert(diff != NULL);
+
+	git_diff_list_free(diff);
+}