Commit bfe7d7de226e91c7ed99b68fc447aa0bcd5182ab

Russell Belfer 2012-11-26T17:24:02

Reorder operations in git reset This corrects the order of operations in git reset so that the checkout to reset the working directory content is done before the HEAD is moved. This allows us to use the HEAD and the index content to know what files can / should safely be reset. Unfortunately, there are still some cases where the behavior of this revision differs from core git. Notable, a file which has been added to the index but is not present in the HEAD is considered to be tracked by core git (and thus removable by a reset command) whereas since this loads the target state into the index prior to resetting, it will consider such a file to be untracked and won't touch it. That is a larger fix that I'll defer to a future commit.

diff --git a/src/reset.c b/src/reset.c
index 04b0863..f5daa8f 100644
--- a/src/reset.c
+++ b/src/reset.c
@@ -62,13 +62,10 @@ int git_reset(
 	git_object *commit = NULL;
 	git_index *index = NULL;
 	git_tree *tree = NULL;
-	int error;
+	int error = 0;
 	git_checkout_opts opts = GIT_CHECKOUT_OPTS_INIT;
 
 	assert(repo && target);
-	assert(reset_type == GIT_RESET_SOFT
-		|| reset_type == GIT_RESET_MIXED
-		|| reset_type == GIT_RESET_HARD);
 
 	if (git_object_owner(target) != repo) {
 		giterr_set(GITERR_OBJECT,
@@ -76,56 +73,50 @@ int git_reset(
 		return -1;
 	}
 
-	if (reset_type != GIT_RESET_SOFT
-		&& (error = git_repository__ensure_not_bare(
-			repo,
+	if (reset_type != GIT_RESET_SOFT &&
+		(error = git_repository__ensure_not_bare(repo,
 			reset_type == GIT_RESET_MIXED ? "reset mixed" : "reset hard")) < 0)
-				return error;
-
-	if ((error = git_object_peel(&commit, target, GIT_OBJ_COMMIT)) < 0)
-		goto cleanup;
+		return error;
 
-	if ((error = git_repository_index(&index, repo)) < 0)
+	if ((error = git_object_peel(&commit, target, GIT_OBJ_COMMIT)) < 0 ||
+		(error = git_repository_index(&index, repo)) < 0 ||
+		(error = git_commit_tree(&tree, (git_commit *)commit)) < 0)
 		goto cleanup;
 
-	if (reset_type == GIT_RESET_SOFT && 
+	if (reset_type == GIT_RESET_SOFT &&
 		(git_repository_state(repo) == GIT_REPOSITORY_STATE_MERGE ||
-		git_index_has_conflicts(index))) {
-			giterr_set(GITERR_OBJECT, "%s (soft) while in the middle of a merge.", ERROR_MSG);
-			error = GIT_EUNMERGED;
-			goto cleanup;
-	}
-
-	if ((error = update_head(repo, commit)) < 0)
-		goto cleanup;
-
-	if (reset_type == GIT_RESET_SOFT) {
-		error = 0;
+		 git_index_has_conflicts(index)))
+	{
+		giterr_set(GITERR_OBJECT, "%s (soft) in the middle of a merge.", ERROR_MSG);
+		error = GIT_EUNMERGED;
 		goto cleanup;
 	}
 
-	if ((error = git_commit_tree(&tree, (git_commit *)commit)) < 0)
-		goto cleanup;
-
-	if ((error = git_index_read_tree(index, tree)) < 0)
+	/* move HEAD to the new target */
+	if ((error = update_head(repo, commit)) < 0)
 		goto cleanup;
 
-	if ((error = git_index_write(index)) < 0)
-		goto cleanup;
+	if (reset_type == GIT_RESET_HARD) {
+		/* overwrite working directory with HEAD */
+		opts.checkout_strategy = GIT_CHECKOUT_FORCE;
 
-	if ((error = git_repository_merge_cleanup(repo)) < 0) {
-		giterr_set(GITERR_REPOSITORY, "%s - Failed to clean up merge data.", ERROR_MSG);
-		goto cleanup;
+		if ((error = git_checkout_tree(repo, (git_object *)tree, &opts)) < 0)
+			goto cleanup;
 	}
 
-	if (reset_type == GIT_RESET_MIXED) {
-		error = 0;
-		goto cleanup;
-	}
+	if (reset_type > GIT_RESET_SOFT) {
+		/* reset index to the target content */
 
-	opts.checkout_strategy = GIT_CHECKOUT_FORCE;
+		if ((error = git_repository_index(&index, repo)) < 0 ||
+			(error = git_index_read_tree(index, tree)) < 0 ||
+			(error = git_index_write(index)) < 0)
+			goto cleanup;
 
-	error = git_checkout_index(repo, NULL, &opts);
+		if ((error = git_repository_merge_cleanup(repo)) < 0) {
+			giterr_set(GITERR_INDEX, "%s - failed to clean up merge data", ERROR_MSG);
+			goto cleanup;
+		}
+	}
 
 cleanup:
 	git_object_free(commit);