{"thread":{"id":"27705","subject":"[PATCH] (trivial) add helpful \"use --soft\" for bare reset","startedAt":"2011-06-26T22:14:28Z","lastAt":"2011-07-01T16:39:50Z","messageCount":6,"participants":["David Fries","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"170561","messageId":"20110626221428.GA26780@spacedout.fries.net","threadId":"27705","inReplyTo":null,"subject":"[PATCH] (trivial) add helpful \"use --soft\" for bare reset","fromName":"David Fries","fromEmail":"david@fries.net","sentAt":"2011-06-26T22:14:28Z","receivedAt":"2011-06-26T22:14:28Z","isPatch":true,"sender":{"key":"david@fries.net","avatar":"https://avatars.githubusercontent.com/u/2767875?v=4"},"body":"Add a helpful \"use --soft\" message for git-reset (mixed) in a bare\nrepository.  This tells the user what they can do, instead of just what\nthey can't.\n\nSigned-off-by: David Fries <David@Fries.net>\n---\nWhen I was first learning how to use git and I needed to reset my bare\nrepository I would make it a full repository just so I could use\ngit-reset, a message like the above would have saved me a lot of\neffort back then.\n\nI'm not the only one, I received an e-mail yesterday confused on what\nto do, found my patch via google \"it saved my bacon.\"\n\n builtin/reset.c |    2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/builtin/reset.c b/builtin/reset.c\nindex 98bca04..dd0cc1e 100644\n--- a/builtin/reset.c\n+++ b/builtin/reset.c\n@@ -332,7 +332,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n \t\tsetup_work_tree();\n \n \tif (reset_type == MIXED && is_bare_repository())\n-\t\tdie(_(\"%s reset is not allowed in a bare repository\"),\n+\t\tdie(_(\"%s reset is not allowed in a bare repository, use --soft\"),\n \t\t    _(reset_type_names[reset_type]));\n \n \t/* Soft reset does not touch the index file nor the working tree\n-- \n1.7.2.3\n"},{"id":"170714","messageId":"7vk4c3qlqa.fsf@alter.siamese.dyndns.org","threadId":"27705","inReplyTo":"20110626221428.GA26780@spacedout.fries.net","subject":"Re: [PATCH] (trivial) add helpful \"use --soft\" for bare reset","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-06-30T17:21:01Z","receivedAt":"2011-06-30T17:21:01Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"David Fries <david@fries.net> writes:\n\n> Add a helpful \"use --soft\" message for git-reset (mixed) in a bare\n> repository.  This tells the user what they can do, instead of just what\n> they can't.\n> ...\n> diff --git a/builtin/reset.c b/builtin/reset.c\n> index 98bca04..dd0cc1e 100644\n> --- a/builtin/reset.c\n> +++ b/builtin/reset.c\n> @@ -332,7 +332,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n>  \t\tsetup_work_tree();\n>  \n>  \tif (reset_type == MIXED && is_bare_repository())\n> -\t\tdie(_(\"%s reset is not allowed in a bare repository\"),\n> +\t\tdie(_(\"%s reset is not allowed in a bare repository, use --soft\"),\n>  \t\t    _(reset_type_names[reset_type]));\n>  \n>  \t/* Soft reset does not touch the index file nor the working tree\n\nIf we want to give a short and concise advice in this codepath, it should\nbe that \"git reset\" of any variant must not be used in a bare repository,\nwhich is what we currently say.\n\nThe _intent_ of \"reset --soft\" is to KEEP the current index and the\ncontents of the working tree, so that next commit using the index and the\nworking tree will be done immediately on top of the commit you are\nresetting to. The implementation detail of how \"reset --soft\" keeps the\nindex and the working tree happens to be by not touching them and updating\nthe branch pointed by the HEAD pointer.\n\nIf we wanted to be strict, we would have at least checked that the index\nand the working tree existed (iow, made sure we had something worth\nkeeping) before proceeding, but we were too lazy to check the sanity of\nthe end user here. An assumption made in early days of git where everybody\nknew what was going on is that nobody sane would use \"reset\" of any flavor\nin a bare repository as the command is about changing the base of the next\ncommit _made_ in that repository, and _making_ a new commit on top of the\nupdated HEAD would very much mean it is not a bare repository.\n\nAnd that assumption was the only reason why the command would run inside a\nbare repository as a substitute for \"update-ref HEAD $the_right_commit\".\nIOW, \"reset --soft\" is allowed not because we thought that it made sense\nto use the command in a bare repository, nor because we wanted to\nencourage it as a way to flip the branch that happens to be the primary\none pointed at by HEAD pointer, but because we (as the implementors) could\nget away with an implementation that does not forbid its use, as (1)\nnobody sane would have done so anyway, and (2) the implementation happened\nto be harmless.\n\nTake a step back and think a bit _why_ you wanted to flip the branch to\npoint at a different commit. Did you push an incorrect commit to the\nrepository? The right way to fix that mistake is by pushing the correct\none, possibly with --force. You can also repoint a ref at a different\ncommit with \"update-ref $the_ref $right_commit\". It will let you correct a\nbranch that is not the primary branch pointed at with the HEAD pointer in\nthe bare repository, unlike \"reset --soft\".\n\nEncouraging \"reset --soft\" as a way to run \"update-ref HEAD\" in a bare\nrepository leads new users in a wrong direction. The next person who reads\nyour updated error message would say \"I cannot use reset --soft to update\na branch that is not the primary branch in a bare repository. Let's add an\noption to name which branch to update, usable only when the command is\nused in a bare repository with --soft option\". Only because you did not\nteach the right way \"update-ref refs/heads/$that_branch $right_commit\", we\nwould end up with yet another option to \"reset\" command that is applicable\nonly in a very narrow special case (i.e. \"in bare and only with --soft\").\n\nI do not think we want to go that route.\n"},{"id":"170723","messageId":"20110630190608.GB26701@spacedout.fries.net","threadId":"27705","inReplyTo":"7vk4c3qlqa.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] (trivial) add helpful \"use --soft\" for bare reset","fromName":"David Fries","fromEmail":"david@fries.net","sentAt":"2011-06-30T19:06:08Z","receivedAt":"2011-06-30T19:06:08Z","isPatch":true,"sender":{"key":"david@fries.net","avatar":"https://avatars.githubusercontent.com/u/2767875?v=4"},"body":"Instead of,\nfatal: mixed reset is not allowed in a bare repository\nprint,\nfatal: mixed reset is not allowed in a bare repository, see update-ref\nTo tell the user what they can do, instead of just what they can't.\n\nSigned-off-by: David Fries <David@Fries.net>\n---\n builtin/reset.c |    2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/builtin/reset.c b/builtin/reset.c\nindex 98bca04..d0d4d66 100644\n--- a/builtin/reset.c\n+++ b/builtin/reset.c\n@@ -332,7 +332,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n \t\tsetup_work_tree();\n \n \tif (reset_type == MIXED && is_bare_repository())\n-\t\tdie(_(\"%s reset is not allowed in a bare repository\"),\n+\t\tdie(_(\"%s reset is not allowed in a bare repository, see update-ref\"),\n \t\t    _(reset_type_names[reset_type]));\n \n \t/* Soft reset does not touch the index file nor the working tree\n-- \n1.7.2.3\n"},{"id":"170728","messageId":"7vwrg3ozi5.fsf@alter.siamese.dyndns.org","threadId":"27705","inReplyTo":"20110630190608.GB26701@spacedout.fries.net","subject":"Re: [PATCH] (trivial) add helpful \"use --soft\" for bare reset","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-06-30T20:06:26Z","receivedAt":"2011-06-30T20:06:26Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"David Fries <david@fries.net> writes:\n\n> diff --git a/builtin/reset.c b/builtin/reset.c\n> index 98bca04..d0d4d66 100644\n> --- a/builtin/reset.c\n> +++ b/builtin/reset.c\n> @@ -332,7 +332,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n>  \t\tsetup_work_tree();\n>  \n>  \tif (reset_type == MIXED && is_bare_repository())\n> -\t\tdie(_(\"%s reset is not allowed in a bare repository\"),\n> +\t\tdie(_(\"%s reset is not allowed in a bare repository, see update-ref\"),\n>  \t\t    _(reset_type_names[reset_type]));\n\nThere still is one thing that worries me remains here, even though I may\nbe worried too much. I tend to think that giving an incorrect advice is\nfar worse than not giving one.\n\nAre you absolutely sure that the user wanted to update only the tip of a\nref without affecting the index nor the working tree, when a mixed reset\nis issued in a bare repository, and there is _no other_ explanation why\nthe user issued a forbidden command?\n\nFor example, I have ~/git.git and /pub/scm/git/git.git on a single machine\nsomewhere. The former is with a working tree and the latter is a bare\nrepository. I usually live in ~/git.git/ on that machine, but sometimes I\nwould do things like:\n\n\t$ git repack -a -d ;# working area\n\t$ cd /pub/scm/git/git.git ;# clean up from time to time ...\n        $ git repack -a -d ;# ... the public one as well\n\nI may disconnect from my screen session to the machine after I do this,\nand I may have forgotten where I was the next time I come back to the\nmachine.  After I reconnect to the same screen session, I may say \"git\nreset\" to get back to a known state, which is what I often want to do in\nthe working area repository ~/git.git, mistakenly thinking that I am in my\nusual ~/git.git directory.\n\nIn such a scenario, the mistake is not that I used a wrong command \"reset\"\nin an attempt to update the tip of the branch. The mistake is that I tried\nto use the right command to update the index, but I did it in a wrong\nplace. \"Did you mean to do that somewhere else?\" would be a much more\nappropriate advice in that case.\n"},{"id":"170732","messageId":"20110630220843.GC26701@spacedout.fries.net","threadId":"27705","inReplyTo":"7vwrg3ozi5.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] (trivial) add helpful \"use --soft\" for bare reset","fromName":"David Fries","fromEmail":"david@fries.net","sentAt":"2011-06-30T22:08:43Z","receivedAt":"2011-06-30T22:08:43Z","isPatch":true,"sender":{"key":"david@fries.net","avatar":"https://avatars.githubusercontent.com/u/2767875?v=4"},"body":"On Thu, Jun 30, 2011 at 01:06:26PM -0700, Junio C Hamano wrote:\n> David Fries <david@fries.net> writes:\n> \n> > diff --git a/builtin/reset.c b/builtin/reset.c\n> > index 98bca04..d0d4d66 100644\n> > --- a/builtin/reset.c\n> > +++ b/builtin/reset.c\n> > @@ -332,7 +332,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n> >  \t\tsetup_work_tree();\n> >  \n> >  \tif (reset_type == MIXED && is_bare_repository())\n> > -\t\tdie(_(\"%s reset is not allowed in a bare repository\"),\n> > +\t\tdie(_(\"%s reset is not allowed in a bare repository, see update-ref\"),\n> >  \t\t    _(reset_type_names[reset_type]));\n\n\nLet me go back and respond to your first reply,\n\n> Take a step back and think a bit _why_ you wanted to flip the branch to\n> point at a different commit. Did you push an incorrect commit to the\n> repository? The right way to fix that mistake is by pushing the correct\n> one, possibly with --force. You can also repoint a ref at a different\n> commit with \"update-ref $the_ref $right_commit\". It will let you correct a\n> branch that is not the primary branch pointed at with the HEAD pointer in\n> the bare repository, unlike \"reset --soft\".\n\nBack when I knew less it was a push, rebase, push failed, reset the\nbare repository so it would work.  I didn't know about push -f, but\nthat's what you call a learning curve, learn something by figuring out\nhow to get something done, then later figure out there's a much easier\nway.\n\nBut at work push -f no longer works, it's administratively denied from\nremote for certain branches, the kind that you generally never want to\nrewind.  But on occasion we do.  The options are to administratively\nchange permissions, push -f, change back, or login to the server,\nclone, push -f, or manipulate the bare repository directly.  Modifying\nthe bare repository is the quickest and git-update-ref works, it just\nisn't in the porcelain commands, so less likely to be known.\n\n> There still is one thing that worries me remains here, even though I may\n> be worried too much. I tend to think that giving an incorrect advice is\n> far worse than not giving one.\n> \n> Are you absolutely sure that the user wanted to update only the tip of a\n> ref without affecting the index nor the working tree, when a mixed reset\n> is issued in a bare repository, and there is _no other_ explanation why\n> the user issued a forbidden command?\n\nI guess I think of reset as moving the current branch and needing to\nknow how much collateral damage it can do, modify the branch location,\nmodify index, modify working files.  I'm not concerned about a user\nactually being in a bare repository thinking they're not, because\nresetting the index or working directory can loose information that\nyou can't get back by looking at the ref-log until gc runs, and\nnothing woring on the index or working directory will work so they'll\nfigure it out soon enough.\n\n> For example, I have ~/git.git and /pub/scm/git/git.git on a single machine\n> somewhere. The former is with a working tree and the latter is a bare\n> repository. I usually live in ~/git.git/ on that machine, but sometimes I\n> would do things like:\n> \n> \t$ git repack -a -d ;# working area\n> \t$ cd /pub/scm/git/git.git ;# clean up from time to time ...\n>         $ git repack -a -d ;# ... the public one as well\n> \n> I may disconnect from my screen session to the machine after I do this,\n> and I may have forgotten where I was the next time I come back to the\n> machine.  After I reconnect to the same screen session, I may say \"git\n> reset\" to get back to a known state, which is what I often want to do in\n> the working area repository ~/git.git, mistakenly thinking that I am in my\n> usual ~/git.git directory.\n\nBut you are doing a `git-reset some_ref` to move to some other commit,\nor `git-reset` to discard any index changes and leave the branch where\nit is?  The first makes it clear you want to move where the branch is\npointing, the second does nothing as far as the commit the branch is\non.  For the first case they are either in the repository they\nintended to or not, but either way they are intending to move the\ncurrent branch.  In the second case they're not even trying to move\nthe branch.\n\n> In such a scenario, the mistake is not that I used a wrong command \"reset\"\n> in an attempt to update the tip of the branch. The mistake is that I tried\n> to use the right command to update the index, but I did it in a wrong\n> place. \"Did you mean to do that somewhere else?\" would be a much more\n> appropriate advice in that case.\n\nYes, your message would be appropriate in that case, but there's no\nway for git to guess that.  Besides if it's a knowledgeable user just\nexecuting the wrong command in the wrong repository, any error message\nis enough for them pause, pay attention, figure out that's not what\nthey expected, realize what they did, and go on their merry way.  The\nmessage is for someone who's used to resetting their non-bare\nrepository, goes to the bare repository and left to hang as it doesn't\nwork and man git-reset(1) doesn't offer any hints (if they look).\n\nMy worry is git-update-ref's behavior is unexpected when someone's\nused to git-commit, git-branch etc, as in\n`git-branch new_branch old_branch^` vs\n`git-update-ref new_branch old_branch^` \nas someone has to know to add refs/heads to git-update-ref where they\ndon't with git-branch.\n\nThe point of the patch is trying to help out a user who is learning,\nand provide something to point them in the right direction.  \n\n-- \nDavid Fries <david@fries.net>    PGP pub CB1EE8F0\nhttp://fries.net/~david/\n"},{"id":"170743","messageId":"7vei2aosyx.fsf@alter.siamese.dyndns.org","threadId":"27705","inReplyTo":"20110630220843.GC26701@spacedout.fries.net","subject":"Re: [PATCH] (trivial) add helpful \"use --soft\" for bare reset","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-07-01T16:39:50Z","receivedAt":"2011-07-01T16:39:50Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"David Fries <david@fries.net> writes:\n\n> But at work push -f no longer works, it's administratively denied from\n> remote for certain branches, the kind that you generally never want to\n> rewind.  But on occasion we do.  The options are to administratively\n> change permissions, push -f, change back, or login to the server,\n> clone, push -f, or manipulate the bare repository directly.  Modifying\n> the bare repository is the quickest and git-update-ref works, it just\n> isn't in the porcelain commands, so less likely to be known.\n\nThat is expected. Whoever denies \"administratively\" a push that rewinds\nthe history would have the authority to grant exception to the policy, and\nability to help you rewind the history. You would need to work with\nthem. If \"them\" happens to be \"you\", then you are expected to have that\nauthority and ability, which may include to have direct write access to\nthe repository and to know update-ref.\n\n> ....  I'm not concerned about a user\n> actually being in a bare repository thinking they're not, because\n> resetting the index or working directory can loose information that\n> you can't get back by looking at the ref-log until gc runs, and\n> nothing woring on the index or working directory will work so they'll\n> figure it out soon enough.\n\nYou may not be concerned, but I am; otherwise the message you are\nresponding to wouldn't have been written. And I would agree they'll figure\nit out soon enough with the current error message, that does not have an\nadvice to look at update-ref, which is irrelevant. I am mostly worried\nabout the additional wrong (for their situation) advice throwing them off\ntrack.\n\n>> In such a scenario, the mistake is not that I used a wrong command \"reset\"\n>> in an attempt to update the tip of the branch. The mistake is that I tried\n>> to use the right command to update the index, but I did it in a wrong\n>> place. \"Did you mean to do that somewhere else?\" would be a much more\n>> appropriate advice in that case.\n>\n> Yes, your message would be appropriate in that case, but there's no\n> way for git to guess that.\n\nThat is exactly my point. If we cannot guess reliably, I do not want to\nsee us giving an inappropriate advice that does not apply to the user's\nsituation, potentially leading the user in the wrong direction.\n"}]}