Re: [PATCH 3/5] setup_revisions: remember whether a ref was positive or not

[Junio C Hamano <gitster@pobox.com>]

Sverre Rabbelier <srabbelier@gmail.com> writes:

>  void add_pending_object(struct rev_info *revs, struct object *obj, const char *name)
>  {
> -	add_pending_object_with_mode(revs, obj, name, S_IFINVALID);
> +	add_pending_object_with_mode(revs, obj, name, S_IFINVALID, 0);
>  }

This seems utterly broken.  For example, fmt-merge-msg.c adds "^HEAD" and
of course the flags on the object is UNINTERESTING. Has all the callers of
add_pending_object() been verified? Why is it passing an unconditional 0
and not !!(obj->flags & UNINTERESTING) or something?

If the excuse is "this is only to help fast-export and other callers of
add_pending_object() does not care", that is a sloppy attitude that breaks
maintainability of the code (because it forgets to add "in the current
code nobody looks at the new 'flags' field" to that excuse, and also does
not have any comments around this code that says so); it is questionable
if such a hack belongs to a patch that touches object.h.

> @@ -1073,7 +1074,8 @@ int handle_revision_arg(const char *arg, struct rev_info *revs,
>  			} else
>  				a->object.flags |= flags_exclude;
>  			b->object.flags |= flags;
> -			add_pending_object(revs, &a->object, this);
> +			add_pending_object_with_mode(revs, &a->object, this,
> +						     S_IFINVALID, flags_exclude);
> ...
> @@ -1103,7 +1105,7 @@ int handle_revision_arg(const char *arg, struct rev_info *revs,
>  	if (!cant_be_filename)
>  		verify_non_filename(revs->prefix, arg);
>  	object = get_reference(revs, arg, sha1, flags ^ local_flags);
> -	add_pending_object_with_mode(revs, object, arg, mode);
> +	add_pending_object_with_mode(revs, object, arg, mode, local_flags);
>  	return 0;
>  }

Questionable.  Did the user mean to say Z is positive when he said

	$ git rev-list A B ^C ... --not G H ... ^Z