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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Sverre Rabbelier <srabbelier@xxxxxxxxx> 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
--
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


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]