Re: [PATCH v2 02/11] refs/files: remove unused REF_DELETING in lock_ref_oid_basic()

From: Jeff King <peff@peff.net>
Date: Jul 20, 2021, 07:19 UTC
Message-ID: <YPZ5ASyrdoCaXHzZ@coredump.intra.peff.net>
In-Reply-To: <xmqqk0lmnury.fsf@gitster.g>

On Mon, Jul 19, 2021 at 09:16:49AM -0700, Junio C Hamano wrote:

Show 24 quoted lines

> > None of those callers pass REF_DELETING, the last such caller went
> > away in 8df4e511387 (struct ref_update: move "have_old" into "flags",
> > 2015-02-17). This is the start of even more removal of unused code in
> > and around this function.
> 
> While I agree that no existing calls to lock_ref_oid_basic() pass
> REF_DELETING to it (hence this patch is a benign no-op), inside
> ref_transaction_commit(), 8df4e511387 still used REF_DELETING, I
> think, like so:
> 
> 	/* Acquire all locks while verifying old values */
> 	for (i = 0; i < n; i++) {
> 		struct ref_update *update = updates[i];
> 		unsigned int flags = update->flags;
> 
> 		if (is_null_sha1(update->new_sha1))
> 			flags |= REF_DELETING;
> 		update->lock = lock_ref_sha1_basic(
> 				update->refname,
> 				((update->flags & REF_HAVE_OLD) ?
> 				 update->old_sha1 : NULL),
> 				NULL,
> 				flags,
> 				&update->type);

Good catch. That code got moved to refs/files-backend.c in 7bd9bcf372 (refs: split filesystem-based refs code into a new file, 2015-11-09), and then pulled into a function in 165056b2fc (lock_ref_for_update(): new function, 2016-04-24). And then finally in 92b1551b1d (refs: resolve symbolic refs first, 2016-04-25), we replaced the call to lock_ref_sha1_basic() with lock_raw_ref().

So I think that final one is when the parameter actually became obsolete (so the patch is still good, just a minor history inaccuracy).

-Peff