From: "Viaceslavus via GitGitGadget" <gitgitgadget@gmail.com>
To: git@vger.kernel.org
Cc: Viaceslavus <vaceslavkozin619@gmail.com>,
Viacelaus <vaceslavkozin619@gmail.com>
Subject: [PATCH v2] hard reset safe mode
Date: Fri, 11 Feb 2022 22:26:44 +0000 [thread overview]
Message-ID: <pull.1137.v2.git.1644618404948.gitgitgadget@gmail.com> (raw)
In-Reply-To: <pull.1137.git.1643802721612.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
next prev parent reply other threads:[~2022-02-11 22:27 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-02-02 11:52 [PATCH] forbid a hard reset before the initial commit Viaceslavus via GitGitGadget
2022-02-02 21:05 ` Junio C Hamano
2022-02-11 22:26 ` Viaceslavus via GitGitGadget [this message]
2022-02-11 23:03 ` [PATCH v2] hard reset safe mode Junio C Hamano
2022-02-14 3:14 ` Bagas Sanjaya
2022-02-14 11:44 ` Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=pull.1137.v2.git.1644618404948.gitgitgadget@gmail.com \
--to=gitgitgadget@gmail.com \
--cc=git@vger.kernel.org \
--cc=vaceslavkozin619@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).