[PATCH v2] hard reset safe mode

["Viaceslavus via GitGitGadget" <gitgitgadget@gmail.com>]

From: Viacelaus <vaceslavkozin619@gmail.com>

The power of hard reset may frequently be inconvinient for a common user. So
this is an implementation of safe mode for hard reset. It can be switched on
by setting 'reset.safe' config variable to true. When running 'reset --hard'
with 'reset.safe' enabled git will check if there are any staged changes
that may be discarded by this reset. If there is a chance of deleting the
changes, git will ask the user for a confirmation with Yes/No choice.

Signed-off-by: Viacelaus <vaceslavkozin619@gmail.com>
---
    Hard reset safe mode
    
    Preventing unexpected code-deletion with hard reset 'safe mode'.
    
    Considering the recomendations for patch v1 and in order to preserve the
    current robustness of hard reset, I have made the following
    modifications to the original version (which has completely disallowed
    hard reset on unborn branch with staged files):
    
    Changes since v1:
    
     * Described security measures aren't enabled by default. Safe mode can
       be turned on with 'reset.safe' config variable.
     * Replaced the resulting error with Yes/No choice so hard reset on
       unborn branch isn`t impossible now.
     * Detection of staged changes that are going to be deleted by the reset
       isn't limited to unborn-branch state now. Git will warn you and ask
       for a confirmation if there are commits on the branch also.

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1137%2FViaceslavus%2Fhard-reset-safety-on-empty-repo-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1137/Viaceslavus/hard-reset-safety-on-empty-repo-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/1137

Range-diff vs v1:

 1:  13cc01b7de7 ! 1:  e6eec1bfda2 forbid a hard reset before the initial commit
     @@ Metadata
      Author: Viacelaus <vaceslavkozin619@gmail.com>
      
       ## Commit message ##
     -    forbid a hard reset before the initial commit
     +    hard reset safe mode
      
     -    Performing 'git reset --hard' on empty repo with staged files
     -    may have the only one possible result - deleting all staged files.
     -    Such behaviour may be unexpected or even dangerous. With this
     -    commit, when running 'git reset --hard', git will check for the
     -    existence of commits in the repo; in case of absence of such, and
     -    also if there are any files staged, git will die with an error.
     +    The power of hard reset may frequently be inconvinient for a common user. So
     +    this is an implementation of safe mode for hard reset. It can be switched on
     +    by setting 'reset.safe' config variable to true. When running 'reset --hard'
     +    with 'reset.safe' enabled git will check if there are any staged changes
     +    that may be discarded by this reset. If there is a chance of deleting the
     +    changes, git will ask the user for a confirmation with Yes/No choice.
      
          Signed-off-by: Viacelaus <vaceslavkozin619@gmail.com>
      
       ## builtin/reset.c ##
     +@@
     + #include "submodule.h"
     + #include "submodule-config.h"
     + #include "dir.h"
     ++#include "wt-status.h"
     + 
     + #define REFRESH_INDEX_DELAY_WARNING_IN_MS (2 * 1000)
     + 
      @@ builtin/reset.c: static void die_if_unmerged_cache(int reset_type)
       
       }
     @@ builtin/reset.c: static void die_if_unmerged_cache(int reset_type)
      +{
      +	return is_branch(refname);
      +}
     ++
     ++static void accept_discarding_changes(void) {
     ++	int answer = getc(stdin);
     ++	printf(_("Some staged changes may be discarded by this reset. Continue? [Y/n]"));
     ++
     ++	if (answer != 'y' && answer != 'Y') {
     ++		printf(_("aborted\n"));
     ++		exit(1);
     ++	}
     ++}
     ++
     ++static void detect_risky_reset(int commits_exist) {
     ++	int cache = read_cache();
     ++	if(!commits_exist) {
     ++		if(cache == 1) {
     ++			accept_discarding_changes();
     ++		}
     ++	}
     ++	else {
     ++		if(has_uncommitted_changes(the_repository, 1)) {
     ++			accept_discarding_changes();
     ++		}
     ++	}
     ++}
      +
       static void parse_args(struct pathspec *pathspec,
       		       const char **argv, const char *prefix,
     @@ builtin/reset.c: int cmd_reset(int argc, const char **argv, const char *prefix)
       	}
      +
      +	if (reset_type == HARD) {
     -+		int commits_exist = for_each_fullref_in("refs/heads", check_commit_exists, NULL);
     -+		if(!commits_exist) {
     -+			if(read_cache() == 1)
     -+				die(_("Hard reset isn`t allowed before the first commit."));
     ++		int safe = 0;
     ++		git_config_get_bool("reset.safe", &safe);
     ++		if (safe) {
     ++			int commits_exist = for_each_fullref_in("refs/heads", check_commit_exists, NULL);
     ++			detect_risky_reset(commits_exist);
      +		}
      +	}
      +
     @@ t/t7104-reset-hard.sh: test_expect_success 'reset --hard did not corrupt index o
       
       '
       
     -+test_expect_success 'reset --hard on empty repo without staged changes works fine' '
     -+	git reset --hard
     ++test_expect_success 'reset --hard in safe mode on unborn branch with staged files results in a warning' '
     ++	git config reset.safe on &&
     ++	touch a &&
     ++	git add a &&
     ++	test_must_fail git reset --hard
     ++
      +'
      +
     -+test_expect_success 'reset --hard on empty repo with staged changes results in an error' '
     -+	touch n &&
     -+	git add n &&
     -+	test_must_fail git reet --hard
     ++test_expect_success 'reset --hard in safe mode after a commit without staged changes works fine' '
     ++	git config reset.safe on &&
     ++	touch b &&
     ++	git add b &&
     ++	git commit -m "initial" &&
     ++	git reset --hard
     ++
      +'
      +
     -+test_expect_success 'reset --hard after a commit works fine' '
     -+	touch new &&
     -+	git add new &&
     ++test_expect_success 'reset --hard in safe mode after a commit with staged changes results in a warning' '
     ++	git config reset.safe on &&
     ++	touch c d &&
     ++	git add c &&
      +	git commit -m "initial" &&
     -+	git reset --hard 2> error &&
     -+	test_must_be_empty error
     ++	git add d &&
     ++	test_must_fail git reset --hard
     ++
      +'
      +
       test_done
     -
     - ## t/t7106-reset-unborn-branch.sh ##
     -@@ t/t7106-reset-unborn-branch.sh: test_expect_success 'reset --soft is a no-op' '
     - 	test_cmp expect actual
     - '
     - 
     --test_expect_success 'reset --hard' '
     --	rm .git/index &&
     --	git add a &&
     --	test_when_finished "echo a >a" &&
     --	git reset --hard &&
     --
     --	git ls-files >actual &&
     --	test_must_be_empty actual &&
     --	test_path_is_missing a
     --'
     --
     - test_done


 builtin/reset.c       | 40 ++++++++++++++++++++++++++++++++++++++++
 t/t7104-reset-hard.sh | 27 +++++++++++++++++++++++++++
 2 files changed, 67 insertions(+)

diff --git a/builtin/reset.c b/builtin/reset.c
index b97745ee94e..997e2adf8d8 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -26,6 +26,7 @@
 #include "submodule.h"
 #include "submodule-config.h"
 #include "dir.h"
+#include "wt-status.h"
 
 #define REFRESH_INDEX_DELAY_WARNING_IN_MS (2 * 1000)
 
@@ -301,6 +302,35 @@ static void die_if_unmerged_cache(int reset_type)
 
 }
 
+static int check_commit_exists(const char *refname, const struct object_id *oid, int f, void *d)
+{
+	return is_branch(refname);
+}
+
+static void accept_discarding_changes(void) {
+	int answer = getc(stdin);
+	printf(_("Some staged changes may be discarded by this reset. Continue? [Y/n]"));
+
+	if (answer != 'y' && answer != 'Y') {
+		printf(_("aborted\n"));
+		exit(1);
+	}
+}
+
+static void detect_risky_reset(int commits_exist) {
+	int cache = read_cache();
+	if(!commits_exist) {
+		if(cache == 1) {
+			accept_discarding_changes();
+		}
+	}
+	else {
+		if(has_uncommitted_changes(the_repository, 1)) {
+			accept_discarding_changes();
+		}
+	}
+}
+
 static void parse_args(struct pathspec *pathspec,
 		       const char **argv, const char *prefix,
 		       int patch_mode,
@@ -474,6 +504,16 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
 			die(_("Cannot do %s reset with paths."),
 					_(reset_type_names[reset_type]));
 	}
+
+	if (reset_type == HARD) {
+		int safe = 0;
+		git_config_get_bool("reset.safe", &safe);
+		if (safe) {
+			int commits_exist = for_each_fullref_in("refs/heads", check_commit_exists, NULL);
+			detect_risky_reset(commits_exist);
+		}
+	}
+
 	if (reset_type == NONE)
 		reset_type = MIXED; /* by default */
 
diff --git a/t/t7104-reset-hard.sh b/t/t7104-reset-hard.sh
index cf9697eba9a..c962c706bed 100755
--- a/t/t7104-reset-hard.sh
+++ b/t/t7104-reset-hard.sh
@@ -44,4 +44,31 @@ test_expect_success 'reset --hard did not corrupt index or cache-tree' '
 
 '
 
+test_expect_success 'reset --hard in safe mode on unborn branch with staged files results in a warning' '
+	git config reset.safe on &&
+	touch a &&
+	git add a &&
+	test_must_fail git reset --hard
+
+'
+
+test_expect_success 'reset --hard in safe mode after a commit without staged changes works fine' '
+	git config reset.safe on &&
+	touch b &&
+	git add b &&
+	git commit -m "initial" &&
+	git reset --hard
+
+'
+
+test_expect_success 'reset --hard in safe mode after a commit with staged changes results in a warning' '
+	git config reset.safe on &&
+	touch c d &&
+	git add c &&
+	git commit -m "initial" &&
+	git add d &&
+	test_must_fail git reset --hard
+
+'
+
 test_done

base-commit: 5d01301f2b865aa8dba1654d3f447ce9d21db0b5
-- 
gitgitgadget