Re: [PATCH v5 1/7] reset: do not accept a mixed reset in a .git dir

From: Christian Couder <chriscool@tuxfamily.org>
Date: Dec 16, 2009, 06:36 UTC
Message-ID: <200912160736.46826.chriscool@tuxfamily.org>
In-Reply-To: <7vskbcyot5.fsf@alter.siamese.dyndns.org>

On mardi 15 décembre 2009, Junio C Hamano wrote:

Show 16 quoted lines

> Christian Couder <chriscool@tuxfamily.org> writes:
> >     While at it, instead of disallowing "git reset --option" outside
> >     a work tree only when option is "hard" or "merge", we now disallow
> >     it except when option is "soft" or "mixed", as it is safer if we
> >     ever add options to "git reset".
>
> I fail to see any sane logic behind this reasoning; you cannot decide if
> you need to allow or disallow the new --option with unspecified semantics
> until you have that --option, and you are saying
>
> Hmm, "reset --option" that does not work when it should work is a bug,
> just like "reset --option" that does not refuse to work when it should
> refuse is, and you cannot decide if you should allow a new --option until
> you have it.  Your "disallowing everything the code does not know about
> by default" doesn't particularly sound safer to me.  I'd suggest dropping
> it from this patch.

Ok, I will drop it.

> It is perfectly fine to have a change like that, if it makes the logic
> easier to follow with the updated repertoire when a new --option is
> added, but not before.

Ok.

[...]

Show 5 quoted lines

> By "after the next patch, it will not fail in a bare repository",
> did you mean "if the next patch blindly replaced an external call to
> read-tree with an internal call to unpack_trees(), it will change the
> behaviour, and we will end up allowing '--mixed in bare'.  To prevent it
> from happening, cmd_reset() should check that condition upfront"?

Yes, that's what I meant.

> Then you were not trying to hide regressions (which makes me happier).
> But then doesn't the change belong to the next patch, not this one?

I can put it in the patch that calls unpack_trees() directly, but on the other hand it can also be seen as an improvement that could be applied to "maint" as it improves the error message.

Best regards, Christian.