On Mon, Nov 26, 2012 at 12:53 PM, Junio C Hamano <gits...@pobox.com> wrote:
> Chris Rorvick <ch...@rorvick.com> writes:
>> Pushes must already (by default) update to a commit-ish due the fast-
>> forward check in set_ref_status_for_push(). But rejecting for not being
>> a fast-forward suggests the situation can be resolved with a merge.
>> Flag these updates (i.e., to a blob or a tree) as not forwardable so the
>> user is presented with more appropriate advice.
>> Signed-off-by: Chris Rorvick <ch...@rorvick.com>
>> remote.c | 5 +++++
>> 1 file changed, 5 insertions(+)
>> diff --git a/remote.c b/remote.c
>> index f5bc4e7..ee0c1e5 100644
>> --- a/remote.c
>> +++ b/remote.c
>> @@ -1291,6 +1291,11 @@ static inline int is_forwardable(struct ref* ref)
>> if (!o || o->type != OBJ_COMMIT)
>> return 0;
>> + /* new object must be commit-ish */
>> + o = deref_tag(parse_object(ref->new_sha1), NULL, 0);
>> + if (!o || o->type != OBJ_COMMIT)
>> + return 0;
> I think the original code used ref_newer() which took commit-ish on
> both sides.
That is still called later in set_ref_status_for_push() to calculate
the nonfastforward flag. The only reason for even checking the new
here is to exclude trees and blobs now so they are flagged as
already-existing and thus avoid nonsensical fetch-and-merge advice.
Otherwise the behavior is unchanged by this last patch.
ref_newer() does end up redoing computation now done in the new
is_forwardable() function. I could probably factor this out of
ref_newer() into a commit_newer() function that could be reused in
set_ref_status_for_push() to avoid this overhead, but it didn't seem
like a big deal. Thoughts?
> With this code, the old must be a commit but new can be a tag that
> points at a commit? Why?
The old must not be a tag because fast-forwarding from it is
potentially destructive; a tag would likely be left dangling in this
case. This is not true for the new, though. I'm not sure
fast-forwarding from a commit to a tag is useful, but I didn't see a
reason to prevent it either. The refs/tags/ hierarchy is excluded
from fast-forwarding before this check, and refs/heads/ is already
protected against anything but commits. So it seems very unlikely
that someone would accidentally make use of this behavior.
So, fast-forwarding to a tag seemed fairly benign and unlikely to
cause confusion, so I leaned towards allowing it in case someone found
a use case for it.
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html