Re: [PATCH] (trivial) add helpful "use --soft" for bare reset

2 messages, 2 authors, 2016-06-15 · open the first message on its own page

Re: [PATCH] (trivial) add helpful "use --soft" for bare reset

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:51:32

David Fries [off-list ref] writes:
quoted hunk
Add a helpful "use --soft" message for git-reset (mixed) in a bare
repository.  This tells the user what they can do, instead of just what
they can't.
...
diff --git a/builtin/reset.c b/builtin/reset.c
index 98bca04..dd0cc1e 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -332,7 +332,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
 		setup_work_tree();
 
 	if (reset_type == MIXED && is_bare_repository())
-		die(_("%s reset is not allowed in a bare repository"),
+		die(_("%s reset is not allowed in a bare repository, use --soft"),
 		    _(reset_type_names[reset_type]));
 
 	/* Soft reset does not touch the index file nor the working tree
If we want to give a short and concise advice in this codepath, it should
be that "git reset" of any variant must not be used in a bare repository,
which is what we currently say.

The _intent_ of "reset --soft" is to KEEP the current index and the
contents of the working tree, so that next commit using the index and the
working tree will be done immediately on top of the commit you are
resetting to. The implementation detail of how "reset --soft" keeps the
index and the working tree happens to be by not touching them and updating
the branch pointed by the HEAD pointer.

If we wanted to be strict, we would have at least checked that the index
and the working tree existed (iow, made sure we had something worth
keeping) before proceeding, but we were too lazy to check the sanity of
the end user here. An assumption made in early days of git where everybody
knew what was going on is that nobody sane would use "reset" of any flavor
in a bare repository as the command is about changing the base of the next
commit _made_ in that repository, and _making_ a new commit on top of the
updated HEAD would very much mean it is not a bare repository.

And that assumption was the only reason why the command would run inside a
bare repository as a substitute for "update-ref HEAD $the_right_commit".
IOW, "reset --soft" is allowed not because we thought that it made sense
to use the command in a bare repository, nor because we wanted to
encourage it as a way to flip the branch that happens to be the primary
one pointed at by HEAD pointer, but because we (as the implementors) could
get away with an implementation that does not forbid its use, as (1)
nobody sane would have done so anyway, and (2) the implementation happened
to be harmless.

Take a step back and think a bit _why_ you wanted to flip the branch to
point at a different commit. Did you push an incorrect commit to the
repository? The right way to fix that mistake is by pushing the correct
one, possibly with --force. You can also repoint a ref at a different
commit with "update-ref $the_ref $right_commit". It will let you correct a
branch that is not the primary branch pointed at with the HEAD pointer in
the bare repository, unlike "reset --soft".

Encouraging "reset --soft" as a way to run "update-ref HEAD" in a bare
repository leads new users in a wrong direction. The next person who reads
your updated error message would say "I cannot use reset --soft to update
a branch that is not the primary branch in a bare repository. Let's add an
option to name which branch to update, usable only when the command is
used in a bare repository with --soft option". Only because you did not
teach the right way "update-ref refs/heads/$that_branch $right_commit", we
would end up with yet another option to "reset" command that is applicable
only in a very narrow special case (i.e. "in bare and only with --soft").

I do not think we want to go that route.

Re: [PATCH] (trivial) add helpful "use --soft" for bare reset

From: David Fries <hidden>
Date: 2016-06-15 22:51:32

Instead of,
fatal: mixed reset is not allowed in a bare repository
print,
fatal: mixed reset is not allowed in a bare repository, see update-ref
To tell the user what they can do, instead of just what they can't.

Signed-off-by: David Fries <redacted>
---
 builtin/reset.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/builtin/reset.c b/builtin/reset.c
index 98bca04..d0d4d66 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -332,7 +332,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
 		setup_work_tree();
 
 	if (reset_type == MIXED && is_bare_repository())
-		die(_("%s reset is not allowed in a bare repository"),
+		die(_("%s reset is not allowed in a bare repository, see update-ref"),
 		    _(reset_type_names[reset_type]));
 
 	/* Soft reset does not touch the index file nor the working tree
-- 
1.7.2.3
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help