Merge pull request #3233 from ethomson/status_typechange

Don't propagate workdir's mode to the index during diff's update index

src/checkout.c

@@ -211,7 +211,7 @@ static bool checkout_is_workdir_modified(
 	if (baseitem->size && wditem->file_size != baseitem->size)
 		return true;
 
-	if (git_diff__oid_for_entry(&oid, data->diff, wditem, NULL) < 0)
+	if (git_diff__oid_for_entry(&oid, data->diff, wditem, wditem->mode, NULL) < 0)
 		return false;
 
 	/* Allow the checkout if the workdir is not modified *or* if the checkout

src/diff.c

@@ -570,7 +570,7 @@ int git_diff__oid_for_file(
 	git_oid *out,
 	git_diff *diff,
 	const char *path,
-	uint16_t  mode,
+	uint16_t mode,
 	git_off_t size)
 {
 	git_index_entry entry;
@@ -580,13 +580,14 @@ int git_diff__oid_for_file(
 	entry.file_size = size;
 	entry.path = (char *)path;
 
-	return git_diff__oid_for_entry(out, diff, &entry, NULL);
+	return git_diff__oid_for_entry(out, diff, &entry, mode, NULL);
 }
 
 int git_diff__oid_for_entry(
 	git_oid *out,
 	git_diff *diff,
 	const git_index_entry *src,
+	uint16_t mode,
 	const git_oid *update_match)
 {
 	int error = 0;
@@ -600,7 +601,7 @@ int git_diff__oid_for_entry(
 		&full_path, git_repository_workdir(diff->repo), entry.path) < 0)
 		return -1;
 
-	if (!entry.mode) {
+	if (!mode) {
 		struct stat st;
 
 		diff->perf.stat_calls++;
@@ -616,7 +617,7 @@ int git_diff__oid_for_entry(
 	}
 
 	/* calculate OID for file if possible */
-	if (S_ISGITLINK(entry.mode)) {
+	if (S_ISGITLINK(mode)) {
 		git_submodule *sm;
 
 		if (!git_submodule_lookup(&sm, diff->repo, entry.path)) {
@@ -630,7 +631,7 @@ int git_diff__oid_for_entry(
 			 */
 			giterr_clear();
 		}
-	} else if (S_ISLNK(entry.mode)) {
+	} else if (S_ISLNK(mode)) {
 		error = git_odb__hashlink(out, full_path.ptr);
 		diff->perf.oid_calculations++;
 	} else if (!git__is_sizet(entry.file_size)) {
@@ -657,11 +658,14 @@ int git_diff__oid_for_entry(
 	/* update index for entry if requested */
 	if (!error && update_match && git_oid_equal(out, update_match)) {
 		git_index *idx;
+		git_index_entry updated_entry;
 
-		if (!(error = git_repository_index__weakptr(&idx, diff->repo))) {
+		memcpy(&updated_entry, &entry, sizeof(git_index_entry));
-			git_oid_cpy(&entry.id, out);
+		updated_entry.mode = mode;
-			error = git_index_add(idx, &entry);
+		git_oid_cpy(&updated_entry.id, out);
-		}
+
+		if (!(error = git_repository_index__weakptr(&idx, diff->repo)))
+			error = git_index_add(idx, &updated_entry);
  	}
 
 	git_buf_free(&full_path);
@@ -856,7 +860,7 @@ static int maybe_modified(
 			&oitem->id : NULL;
 
 		if ((error = git_diff__oid_for_entry(
-				&noid, diff, nitem, update_check)) < 0)
+				&noid, diff, nitem, nmode, update_check)) < 0)
 			return error;
 
 		/* if oid matches, then mark unmodified (except submodules, where

src/diff.h

@@ -94,7 +94,7 @@ extern int git_diff_delta__format_file_header(
 extern int git_diff__oid_for_file(
 	git_oid *out, git_diff *, const char *, uint16_t, git_off_t);
 extern int git_diff__oid_for_entry(
-	git_oid *out, git_diff *, const git_index_entry *, const git_oid *update);
+	git_oid *out, git_diff *, const git_index_entry *, uint16_t, const git_oid *update);
 
 extern int git_diff__from_iterators(
 	git_diff **diff_ptr,

tests/status/worktree.c

@@ -1096,3 +1096,51 @@ void test_status_worktree__unreadable_as_untracked(void)
 	cl_assert_equal_i(0, counts.wrong_sorted_path);
 }
 
+void test_status_worktree__update_index_with_symlink_doesnt_change_mode(void)
+{
+	git_repository *repo = cl_git_sandbox_init("testrepo");
+	git_reference *head;
+	git_object *head_object;
+	git_index *index;
+	const git_index_entry *idx_entry;
+	git_status_options opts = GIT_STATUS_OPTIONS_INIT;
+	status_entry_counts counts = {0};
+	const char *expected_paths[] = { "README" };
+	const unsigned int expected_statuses[] = {GIT_STATUS_WT_NEW};
+
+	opts.show = GIT_STATUS_SHOW_INDEX_AND_WORKDIR;
+	opts.flags = GIT_STATUS_OPT_DEFAULTS | GIT_STATUS_OPT_UPDATE_INDEX;
+
+	cl_git_pass(git_repository_head(&head, repo));
+	cl_git_pass(git_reference_peel(&head_object, head, GIT_OBJ_COMMIT));
+
+	cl_git_pass(git_reset(repo, head_object, GIT_RESET_HARD, NULL));
+
+	cl_git_rewritefile("testrepo/README", "This was rewritten.");
+
+	/* this status rewrites the index because we have changed the
+	 * contents of a tracked file
+	 */
+	counts.expected_entry_count = 1;
+	counts.expected_paths = expected_paths;
+	counts.expected_statuses = expected_statuses;
+
+	cl_git_pass(
+		git_status_foreach_ext(repo, &opts, cb_status__normal, &counts));
+	cl_assert_equal_i(1, counts.entry_count);
+
+	/* now ensure that the status's rewrite of the index did not screw
+	 * up the mode of the symlink `link_to_new.txt`, particularly
+	 * on platforms that don't support symlinks
+	 */
+	cl_git_pass(git_repository_index(&index, repo));
+	cl_git_pass(git_index_read(index, true));
+
+	cl_assert(idx_entry = git_index_get_bypath(index, "link_to_new.txt", 0));
+	cl_assert(S_ISLNK(idx_entry->mode));
+
+	git_index_free(index);
+	git_object_free(head_object);
+	git_reference_free(head);
+}
+