Re: [PATCH v6] add --summary option to git-push and git-fetch

* Junio C Hamano (gitster@pobox.com) [100130 02:16]:

Larry D'Anna [off-list ref] writes:

quoted

+
+	object = parse_object(sha1); 
+	if (!object)
+	    die("bad object %s", arg);
+	
+	object_deref = deref_tag(object, NULL, 0); 
+	if (object_deref && object_deref->type == OBJ_COMMIT)
+	    if (flags_to_clear)
+		clear_commit_marks((struct commit *) object_deref, flags_to_clear); 
+
+	object->flags |= flags ^ local_flags; 

This smells somewhat fishy---what is the reason this "peel and mark" needs
to be done only in this codepath, and none of the other callers of
get_reference() need a similar logic, for example?

In general, why do you need to sprinkle clear-commit-marks all over the
place?  

My idea was to call call clear_commit_marks on the "roots" of the revision arg,
and since handle_revision_arg looks up those roots in several different places,
i had to put clear_commit_marks in each of those places.  the reason the patch
is particularly ugly in this spot is that the other places where i put
clear_commit_marks, I already had a struct commit *, but here i just had a
object that might be a tag.

This is not a rhetorical question (I haven't reviewed all the
codepath involved for quite some time), but naïvely it appears it would be
a lot simpler if you can let the existing code to do all the revision
parsing and preparation to add to the pending object array as usual, and
clear the flags from them before you let prepare_revision_walk() to start
traversing the commit, but you probably had some reason why that simpler
approach would not work and did it this way.  What am I missing?

The "existing code" being the caller of print_summary_for_push_or_fetch?  I
suppose I just wanted to keep the patches interference with update_local_ref to
a minimum, so I had it just grab the existing variable "quickref" out of that
function, because that was all the info I really needed to print the summary.

So i guess you're saying that it would be better for update_local_ref and
print_summary_for_push_or_fetch to clear the flags, and just pass a rev_info for
print_summary_for_push_or_fetch instead of quickref?

   --larry