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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Christian Couder <chriscool@xxxxxxxxxxxxx> writes:

>     This patch is also needed to speed up "git reset" by using
>     unpack_tree() directly (instead of execing "git read-tree"). A
>     following patch will do just that.

This still doesn't seem to explain anything that the part you added after
your patch.

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

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.

>> An honest justification might have been "This change to disallow a mixed
>> reset in $GIT_DIR of a repository with a work tree will break existing
>> scripts, but I think it is not widely used _for such and such reasons_,
>> and can easily be worked around.  On the other hand, this change vastly
>> simplifies the reimplementation of 'reset' _because X and Y and Z_".
>
> My opinion is that it works this way just by accident not by design (that's 
> why I said "fragile").

And do you still think it is accident after I explained the difference
between the two for you (or perhaps you didn't read it)?

>> > This patch is also needed to speed up "git reset" by using
>> > unpack_tree() directly (instead of execing "git read-tree").
>>
>> It is very unclear _why_ it is "needed" from this description.
>
> The reason is that after the next patch, it will not fail in a bare 
> repository,...

That sounds as if you want to change the definition of what the expected
behaviour is early, because you want to claim a regression you will later
introduce is not a regression.  I hope that is not the case.
--
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

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]