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

From: Derrick Stolee <stolee@gmail.com>
Date: Aug 19, 2020, 15:10 UTC
Message-ID: <2fb1fdf2-54b1-4597-4b58-96208bebeb6b@gmail.com>
In-Reply-To: <20200819000425.2840491-1-jonathantanmy@google.com>

On 8/18/2020 8:04 PM, Jonathan Tan wrote:

Show 21 quoted lines

>> If the lock file already exists, then fail silently. This will become
> 
> Maybe "skip all maintenance steps silently"?
> 
>> +	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.

Sorry, it is silent when "--auto" is specified. The commit message needs to reflect this properly. This could easily be downgraded to a warning.

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

Thanks, -Stolee