Re: [PATCH v2 07/11] maintenance: take a lock on the objects directory

From: Jonathan Tan <jonathantanmy@google.com>
Date: Aug 19, 2020, 00:04 UTC
Message-ID: <20200819000425.2840491-1-jonathantanmy@google.com>
In-Reply-To: <6f86cfaa94cfeaf7a2af417991ca07e41b6b0c3d.1597760589.git.gitgitgadget@gmail.com>

> If the lock file already exists, then fail silently. This will become

Maybe "skip all maintenance steps silently"?

Show 14 quoted lines

> +	if (hold_lock_file_for_update(&lk, lock_path, LOCK_NO_DEREF) < 0) {
> +		/*
> +		 * Another maintenance command is running.
> +		 *
> +		 * If --auto was provided, then it is likely due to a
> +		 * recursive process stack. Do not report an error in
> +		 * that case.
> +		 */
> +		if (!opts->auto_flag && !opts->quiet)
> +			error(_("lock file '%s' exists, skipping maintenance"),
> +			      lock_path);
> +		free(lock_path);
> +		return 0;
> +	}

As it is, this doesn't seem very silent. :-) If we do want a message to be printed maybe make it warning instead of error.

Other than that, the idea of having a lock file and the implementation in this patch look good to me.