From: Larry D'Anna <larry@elder-gods.org>
Date: Fri, 05 Feb 2010 20:05:24 GMT
Subject: Re: [PATCH 3/3] make git push --dry-run --porcelain exit with status 0 even if updates will be rejected
Message-ID: <20100205200524.GA24027@cthulhu>
In-Reply-To: <20100205195644.GE24474@coredump.intra.peff.net>

* Jeff King (peff@peff.net) [100205 14:56]:
> On Fri, Feb 05, 2010 at 02:34:22PM -0500, Larry D'Anna wrote:
> 
> > diff --git a/builtin-send-pack.c b/builtin-send-pack.c
> > index 76c7206..dfd7470 100644
> > --- a/builtin-send-pack.c
> > +++ b/builtin-send-pack.c
> > @@ -478,6 +478,11 @@ int send_pack(struct send_pack_args *args,
> >  		return ret;
> >  	for (ref = remote_refs; ref; ref = ref->next) {
> >  		switch (ref->status) {
> > +		case REF_STATUS_REJECT_NONFASTFORWARD:
> > +		case REF_STATUS_REJECT_NODELETE:
> > +			if (args->porcelain && args->dry_run)
> > +				break;
> > +			return -1;
> >  		case REF_STATUS_NONE:
> >  		case REF_STATUS_UPTODATE:
> >  		case REF_STATUS_OK:
> 
> Why just these two status flags? Based on your reasoning elsewhere, I
> would assume the logic should be:
> 
>   - if we had some transport-related error, return failure
> 
>   - if not, then return success, as any ref's failure is already
>     indicated in the porcelain output
> 
> So shouldn't it just be:
> 
>   if (args->porcelain && args->dry_run)
>           return 0;
> 
> after we check for transport errors but before the loop that you are
> modifying.

I don't know what the deal is with REF_STATUS_EXPECTING_REPORT, so I didn't want
to modify the behavior in the case that ref->status was that.  What does
expecting report mean?

          --larry