From: Viacelaus <vaceslavkozin619@xxxxxxxxx> 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@xxxxxxxxx> --- 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@xxxxxxxxx> ## 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@xxxxxxxxx> ## 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