On Sat, Jan 31, 2009 at 05:27:45PM -0800, Junio C Hamano wrote:
> I haven't manged to convince myself about the "git init" change (I have
> the code and also I've looked at the extent of damage the change causes to
I think the "git init" change doesn't make sense. In fact, I don't think
such a proposal ever really makes sense (and I have even proposed it in
the past, but others arguments have changed my way of thinking).
The reason is that you are just moving the breakage to a different point
in their workflow. The claim is that it's not OK for this to break:
cd foo && git init
git push ;# ok
... time passes, git upgrade ...
git push ;# broken
but it somehow _is_ OK for this to break:
cd foo && git init
git push ;# ok
... time passes, git upgrade ...
cd bar && git init
git push ;# broken
In both cases, you have a sequence of commands that does one thing with
one git version, and something else with another git version. The only
difference is whether your sequence includes git init. So while you
don't break people with existing repositories, you are still breaking
anybody who creates a new one and gets confused when there is new
behavior (or even has scripts which involve repository creation).
So in my opinion either the breakage is serious enough not to allow the
change, or minor enough (compared to the benefit) to allow it. But
changing default config in git init is:
- a half-way solution that leaves some workflows broken and some not
- possibly even _worse_, since now we have sacrificed consistency. So
now users wonder why some of their repos show breakage and some
don't. Or why a particular behavior goes away when they try to write
a test case that involves creating a new test repo.
And note that it doesn't matter whether you think the right path is to
make the change or not: I am only arguing here against this sort of
half-way technique.
> -- >8 --
> Subject: [PATCH] receive-pack: explain what to do when push updates the current branch
>
> This makes "git push" issue a more detailed instruction when a user pushes
> into the current branch of a non-bare repository without having an
> explicit configuration set to receive.denycurrentbranch. In such a case,
> it will also tell the user that the default will change to refusal in a
> future version of git.
I think this is a definite improvement over the current behavior. As I
said before, I am not sure what is the right path (though I think I am
leaning towards leaving the warning longer based on the recent
discussion), but if we are to leave the default to warn and not refuse,
I think this should definitely be applied.
A few comments on the specific message:
> }
>
> +static char *warn_unconfigured_deny_msg[] = {
> + "Updating the currently checked out branch may cause confusion,",
> + "as the index and work tree do not reflect changes that are in HEAD."
> + "As a result, you may see the changes you just pushed into it",
Missing comma between lines 2 and 3, which results in an overly long
line in the output.
> + "You can set 'receive.denyCurrentBranch' configuration variable to",
> + "'refuse' in the repository to forbid pushing into the current branch",
> + "of it."
Maybe this should specifically say "remote repository". If you
understand how the feature works, it is obvious that you must do it that
way, but for less advanced users it is not even clear that this text is
being generated by the remote end.
> + "To allow pushing into the current branch, you can set it to 'ignore';",
> + "but this is not recommended unless you really know what you are doing."
I thought somebody (you?) argued against the phrase "unless you really
know what you are doing". And it is better here in context explaining
the general issue. But as a user, now I have to ask: do I know what I am
doing, and if not, how do I find out?
The two obvious solutions for people who "know what they are doing" are
running "git reset --hard", and installing a hook that does something
sensible. I don't know if it is worth mentioning them here (the former
is mentioned earlier in the message, but that doesn't necessarily mean
the user understands all the implications). Since there are so many
subtleties to explain, maybe it make sense to simply put in a pointer to
an expanded discussion in the "git push" manpage?
-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
