Jeff King <peff@xxxxxxxx> writes: > I.e., we trigger the "!o" branch after the parse_object in your example. Heh, I didn't see this message until now (gmane seems to be lagging a bit). I am very tempted to do this. * Remove unnecessary not_forwardable from "struct ref"; it is only used inside set_ref_status_for_push(); * "refs/tags/" is the only hierarchy that cannot be replaced without --force; * Remove the misguided attempt to force that everything that updates an existing ref has to be a commit outside "refs/tags/" hierarchy. This code does not know what kind of objects the user wants to place in "refs/frotz/" hierarchy it knows nothing about. I feel moderately strongly about the last point. Defining special semantics for one hierarchy (e.g. "refs/tags/") and implementing a policy for enforcement is one thing, but a random policy that depends on object type that applies globally is simply insane. The user may want to do "refs/tested/" hierarchy that is meant to hold references to commit, with one annotated tag "refs/tested/latest" that points at the "latest tested version" with some commentary, and maintain the latter by keep pushing to it. If that is the semantics the user wanted to ahve in the "refs/tested/" hierarchy, it is not reasonable to require --force for such a workflow. The user knows better than Git in such a case. cache.h | 1 - remote.c | 24 +----------------------- t/t5516-fetch-push.sh | 21 --------------------- 3 files changed, 1 insertion(+), 45 deletions(-) diff --git a/cache.h b/cache.h index a32a0ea..a942bbd 100644 --- a/cache.h +++ b/cache.h @@ -1004,7 +1004,6 @@ struct ref { requires_force:1, merge:1, nonfastforward:1, - not_forwardable:1, update:1, deletion:1; enum { diff --git a/remote.c b/remote.c index aa6b719..2c747c4 100644 --- a/remote.c +++ b/remote.c @@ -1279,26 +1279,6 @@ int match_push_refs(struct ref *src, struct ref **dst, return 0; } -static inline int is_forwardable(struct ref* ref) -{ - struct object *o; - - if (!prefixcmp(ref->name, "refs/tags/")) - return 0; - - /* old object must be a commit */ - o = parse_object(ref->old_sha1); - 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; - - return 1; -} - void set_ref_status_for_push(struct ref *remote_refs, int send_mirror, int force_update) { @@ -1344,8 +1324,6 @@ void set_ref_status_for_push(struct ref *remote_refs, int send_mirror, * passing the --force argument */ - ref->not_forwardable = !is_forwardable(ref); - ref->update = !ref->deletion && !is_null_sha1(ref->old_sha1); @@ -1355,7 +1333,7 @@ void set_ref_status_for_push(struct ref *remote_refs, int send_mirror, !has_sha1_file(ref->old_sha1) || !ref_newer(ref->new_sha1, ref->old_sha1); - if (ref->not_forwardable) { + if (!prefixcmp(ref->name, "refs/tags/")) { ref->requires_force = 1; if (!force_ref_update) { ref->status = REF_STATUS_REJECT_ALREADY_EXISTS; diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh index 6009372..8f024a0 100755 --- a/t/t5516-fetch-push.sh +++ b/t/t5516-fetch-push.sh @@ -950,27 +950,6 @@ test_expect_success 'push requires --force to update lightweight tag' ' ) ' -test_expect_success 'push requires --force to update annotated tag' ' - mk_test heads/master && - mk_child child1 && - mk_child child2 && - ( - cd child1 && - git tag -a -m "message 1" Tag && - git push ../child2 Tag:refs/tmp/Tag && - git push ../child2 Tag:refs/tmp/Tag && - >file1 && - git add file1 && - git commit -m "file1" && - git tag -f -a -m "message 2" Tag && - test_must_fail git push ../child2 Tag:refs/tmp/Tag && - git push --force ../child2 Tag:refs/tmp/Tag && - git tag -f -a -m "message 3" Tag HEAD~ && - test_must_fail git push ../child2 Tag:refs/tmp/Tag && - git push --force ../child2 Tag:refs/tmp/Tag - ) -' - test_expect_success 'push --porcelain' ' mk_empty && echo >.git/foo "To testrepo" && -- 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