Re: [PATCH/RFCv3 2/2] receive-pack: don't pass non-existent refs to post-{receive,update} hooks in push deletions
- From
Pang Yan Han <pangyanhan@gmail.com>
- Date
- Sep 28, 2011, 23:08 UTC
- Message-ID
- <20110928230809.GA1798@myhost>
- In-Reply-To
- <7vsjngxphv.fsf@alter.siamese.dyndns.org>
On Wed, Sep 28, 2011 at 03:37:32PM -0700, Junio C Hamano wrote:
Show 10 quoted lines
> Pang Yan Han <pangyanhan@gmail.com> writes: > > > +/* For invalid refs */ > > +static struct command **invalid_delete; > > +static size_t invalid_delete_nr; > > +static size_t invalid_delete_alloc; > > Do you have to have these separately only to keep track of the corner case > errors? I would have expected that it would be more natural to mark them > by adding a single bitfield to "struct command".
Yes they are purely for keeping track of deleting non-existent refs. Ok I will add the bitfield to struct command.
Show 20 quoted lines
>
> > @@ -447,6 +467,8 @@ static const char *update(struct command *cmd)
> > if (!parse_object(old_sha1)) {
> > rp_warning("Allowing deletion of corrupt ref.");
> > old_sha1 = NULL;
> > + if (!ref_exists((char *) name))
> > + invalid_delete_append(cmd);
>
> This is not an "invalid" delete but deleting a non-existing ref. Perhaps
> you would want to move the warning and optionally reword it as well, e.g.
>
> if (!parse_object(old_sha1)) {
> old_sha1 = NULL;
> if (ref_exists(name)) {
> rp_warning("Allowing deletion of corrupt ref.");
> } else {
> rp_warning("Deleting a ref that does not exist.");
> cmd->did_not_exist = 1;
> }
> ...Sure thing.
I am unable to reply this until much later, but are the tests in this patch ok? It's the first time I'm writing test cases for git.
Thanks for the feedback!