* Junio C Hamano (gitster@xxxxxxxxx) [100130 02:16]:
> Larry D'Anna <larry@xxxxxxxxxxxxxx> writes:
> > +
> > + 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
--
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