Re: [PATCH 05/23] fsmonitor-settings: bare repos are incompatible with FSMonitor

From: Jeff Hostetler <git@jeffhostetler.com>
Date: Mar 2, 2022, 21:09 UTC
Message-ID: <1fe07a1b-df7f-5230-5f49-4cb454c3df9f@jeffhostetler.com>
In-Reply-To: <220225.86ee3q7k2s.gmgdl@evledraar.gmail.com>

On 2/25/22 3:42 PM, Ævar Arnfjörð Bjarmason wrote:

Show 37 quoted lines

> 
> On Tue, Feb 15 2022, Jeff Hostetler via GitGitGadget wrote:
> 
>> +static void create_reason_message(struct repository *r,
>> +				  struct strbuf *buf_reason)
>> +{
>> +	struct fsmonitor_settings *s = r->settings.fsmonitor;
>> +
>> +	switch (s->reason) {
>> +	case FSMONITOR_REASON_ZERO:
>> +		return;
>> +
>> +	case FSMONITOR_REASON_BARE:
>> +		strbuf_addstr(buf_reason,
>> +			      _("bare repos are incompatible with fsmonitor"));
>> +		return;
>> +
>> +	default:
>> +		BUG("Unhandled case in create_reason_message '%d'", s->reason);
>> +	}
>> +}
>> +
>> +enum fsmonitor_reason fsm_settings__get_reason(struct repository *r,
>> +					       struct strbuf *buf_reason)
>> +{
>> +	lookup_fsmonitor_settings(r);
>> +
>> +	strbuf_reset(buf_reason);
>> +	if (r->settings.fsmonitor->mode == FSMONITOR_MODE_INCOMPATIBLE)
>> +		create_reason_message(r, buf_reason);
>> +
>> +	return r->settings.fsmonitor->reason;
>> +}
> 
> This API (just looking at one small bit discussed because related bits
> conflict with another series) seems to require a lot of ceremony just to
> get a const char * error.

My thought at the time was that the __get_reason() code might want to format a message and include details from the repo (such as the working directory root) or the kind of objection (such as a remote SMB mount). So I had it take a strbuf rather than a "const char *" return value.

But so far (in the successive commits) all of the reason messages have been constant and I think I'm OK with having the non-specific messages.

So yes, I could simplify it.

Thanks Jeff