Re: [PATCH 1/1] maintenance: fix a SEGFAULT when no repository

From: Derrick Stolee <stolee@gmail.com>
Date: Nov 24, 2020, 21:59 UTC
Message-ID: <7fbe28ff-6d0f-f377-d86f-0b32f12ddee6@gmail.com>
In-Reply-To: <xmqqim9uifz8.fsf@gitster.c.googlers.com>

On 11/24/2020 4:48 PM, Junio C Hamano wrote:

Show 10 quoted lines

> Derrick Stolee <stolee@gmail.com> writes:
> 
>> The reason to use RUN_SETUP_GENTLY was probably due to some thought
>> of modifying the background maintenance schedule without being in a
>> Git repository. However, we currently run the [un]register logic
>> inside of the stop|start subcommands, so a GIT_DIR is required there,
>> too.
> 
> Meaning all the operations we currently support requires to be done
> in a repository?  If so, that may be acceptable.

That is the current case.

Show 6 quoted lines

> When a new operation that does not require to be in a repository is
> added, or when an existing operation is updated not to require to be
> in a repository, reverting the change and then checking in the
> implementation of each operation if we are in a repository instead
> should be easy enough---it pretty much should amount to Rafael's
> patch, right?

Yes, I agree.

> But then, being prepared for that future already is also OK, so I
> can go either way.  Please figure it out between you ;-)

The only thing I can think is that using RUN_SETUP protects all subcommands immediately. It is equally possible that we add a new subcommand that _also_ requires the GIT_DIR and we need to be sure to implement this check at the beginning of that method, too!

But again, I'm just happy that this was found and is being fixed.

Rafael: please feel free to take or ignore my suggestion at your
discretion. The existing review has also been illuminating.

Thanks, -Stolee