Re: [PATCH v2] lockfile: add PID file for debugging stale locks

From: Junio C Hamano <gitster@pobox.com>
Date: Dec 18, 2025, 00:32 UTC
Message-ID: <xmqqh5tozl1i.fsf@gitster.g>
In-Reply-To: <pull.2011.v2.git.1765997966593.gitgitgadget@gmail.com>

"Paulo Casaretto via GitGitGadget" <gitgitgadget@gmail.com> writes:

Show 21 quoted lines

>  /* Make sure errno contains a meaningful value on error */
>  static int lock_file(struct lock_file *lk, const char *path, int flags,
> -		     int mode)
> +		     int mode, enum lockfile_pid_component component)
> ...
>  }
>  
> @@ -102,7 +203,8 @@ static int lock_file(struct lock_file *lk, const char *path, int flags,
>   * exactly once. If timeout_ms is -1, try indefinitely.
>   */
>  static int lock_file_timeout(struct lock_file *lk, const char *path,
> -			     int flags, long timeout_ms, int mode)
> +			     int flags, long timeout_ms, int mode,
> +			     enum lockfile_pid_component component)
>  {
> ...
>  	if (timeout_ms == 0)
> -		return lock_file(lk, path, flags, mode);
> +		return lock_file(lk, path, flags, mode, component);
> -		fd = lock_file(lk, path, flags, mode);
> +		fd = lock_file(lk, path, flags, mode, component);

These are OK, but I expected these are rolled into an "unsigned flags" word, so that ...

Show 6 quoted lines

>  int hold_lock_file_for_update_timeout_mode(
> -		struct lock_file *lk, const char *path,
> -		int flags, long timeout_ms, int mode);
> +	struct lock_file *lk, const char *path,
> +	int flags, long timeout_ms, int mode,
> +	enum lockfile_pid_component component);

... things like this can be done without adding an extra parameter. Compared to "what should we do when we see an error?", ...

> -	fd = hold_lock_file_for_update_timeout(&lock, path.buf, LOCK_DIE_ON_ERROR, -1);
> +	fd = hold_lock_file_for_update_timeout(&lock, path.buf, LOCK_DIE_ON_ERROR, -1,
> +					       LOCKFILE_PID_OTHER);

... "how would we name the lockfile for this action?" is *not* all that special and should not occupy a separate parameter on its own.

Existing "flags" argument being "int" not "unsigned int" is a historical mistake, by the way.

But maybe it is just me?  I dunno.