git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: "Viaceslavus via GitGitGadget" <gitgitgadget@gmail.com>
Cc: git@vger.kernel.org, Viaceslavus <vaceslavkozin619@gmail.com>
Subject: Re: [PATCH] forbid a hard reset before the initial commit
Date: Wed, 02 Feb 2022 13:05:01 -0800	[thread overview]
Message-ID: <xmqq4k5hyo76.fsf@gitster.g> (raw)
In-Reply-To: <pull.1137.git.1643802721612.gitgitgadget@gmail.com> (Viaceslavus via GitGitGadget's message of "Wed, 02 Feb 2022 11:52:01 +0000")

"Viaceslavus via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: Viacelaus <vaceslavkozin619@gmail.com>
>
> Performing 'git reset --hard' on empty repo with staged files
> may have the only one possible result - deleting all staged files.

Sure.  It has the only one possible result, which is a sign that the
command is well designed to give a robust and predictable end user
experience.

I know you wanted to say "there is only one possible result, and
that result cannot be anything but bad. You Git folks are stupid to
design a command that only can have a bad result, so I'll fix that
stupidity for you".

But the thing is, not everybody agrees with your "deleting all files
that added to the index when asked to 'reset --hard' is bad".  It is
the most obvious way to go back to the "pristine" state, and after
all, that is what "reset --hard" is about.

Many readers on the list are non-native speakers.  You must be
careful with your rhetorics, because they often will not be taken in
the way you meant them to be taken by them.  When you can say "doing
X does Y" and convey the core of what you want to say, do so,
instead of saying "doing X has only one possible result, which is
Y". You may lose the "you Git folks are stupid" part of the message,
but you're better off not to sound rude anyway ;-)

> 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.

This directly contradicts with, and likely will regress the fix made
by, what 166ec2e9 (reset: allow reset on unborn branch, 2013-01-14)
wanted to do.  I do not think we want this change in its current
form.

When starting a new project on a hosting provider like GitHub these
days, you can have them create the initial commit that records the
copy of the license file, and the first thing you do on your local
machine after leaving the browser to create the repository over
there is to clone from it.  After that, you'd populate the working
tree with the rest of the project files, and record the result.  If
you say "reset --hard" before committing, you'll equally lose all
the newly added files, but because the history is not empty, the
approach taken by this patch would not work to protect you, I
suspect.  It almost always is a mistake to special case an empty
repository or an empty history.

Having said all that, I am sympathetic to the cause to make it
harder to discard a lot of work by mistake.  It is just that
disabling "reset --hard" only when it is trying to go back to an
empty tree is not an effective way to do so.  It is even less so
when you do not give any escape hatch in case the user knew what
they were doing and really meant to go back to the pristine state.

    Side note.  Yes, "git diff --cached | git apply -R --index" or
    "git rm --cached -r ." as a workaround, but when the user wanted
    to do "reset --hard", we should have a way to let them do so.

Off the top of my head, here are a couple of possible ways to
improve the design of this change (note: I am not saying that I'll
unconditionally take such a patch that implements any of these):

 * Detect if we are being interactive, and offer Yes/No choice to
   give an interactive user a chance to abort when we detect a
   "risky" situation.  Don't do anything if we are not interactive,
   and don't make it impossible to do things that we may (mis)detect
   as risky.

 * Instead of "we are going back to the state without any commit
   yet", use a better heuristics, such as "we'd lose a newly added
   path (i.e. the path exists in the index and in the working tree
   but does not exist in HEAD)" as a sign to flag the situation as
   possibly risky.  Or limit that further to protect only when we'd
   lose more than N-percent of the paths in the index that way.

But both are hard problems.

Many existing scripts do rely on "reset --hard" to be a robust and
predictable way to go back to the pristine state, and they will be
very upset if we misdetect and prompt the user who is not sitting in
front of the keyboard.

  reply	other threads:[~2022-02-02 21:05 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 [this message]
2022-02-11 22:26 ` [PATCH v2] hard reset safe mode Viaceslavus via GitGitGadget
2022-02-11 23:03   ` 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=xmqq4k5hyo76.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --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).