Re: [PATCH v2 3/5] scalar: enable built-in FSMonitor on `register`

From: Derrick Stolee <derrickstolee@github.com>
Date: Aug 18, 2022, 13:19 UTC
Message-ID: <82716e5b-3522-68f5-7479-1b39811e0cb2@github.com>
In-Reply-To: <3a238691-c37f-39a1-f4f6-2b8f9b4c4dcb@github.com>

On 8/17/2022 7:47 PM, Victoria Dye wrote:

Show 7 quoted lines

> (warning: long-winded tangent mostly unrelated to FSMonitor)
> 
> In the process of testing FSMonitor behavior, I think found other issues
> with Scalar registration. Specifically, the test I wrote attempted to
> 'scalar register' a bare repo, since bare directories are incompatible with
> FSMonitor. After seeing that FSMonitor was *not* incompatible with the
> repository, I found that Scalar was 1) ignoring the bare repository and, as

This is interesting, that Scalar doesn't recognize a bare repo. There are definitely some config settings that it recommends that don't make sense in a bare repo, but it's interesting that it completely ignores it. Good find.

I'm not sure there is anything to 'fix' except maybe error out when the discovered Git repository is bare. Add a warning, at minimum.

> a result, 2) identifying my Git clone (way above GIT_CEILING_DIRECTORIES) as
> the "enlistment root". I think 1) might be fine as-is - uniformly ignoring
> bare repos seems like a reasonable choice - but 2) seems like more of a
> problem.

...

Show 15 quoted lines

> The current usage of GIT_CEILING_DIRECTORIES relies on the fact that, when
> invoking a normal 'git' command, 'setup_git_directory()' only searches
> upwards from the current working directory to find the repo root; it's a
> clear "yes" or "no" as to whether that search passes a ceiling directory.
> Scalar isn't as clear, since it searches for the repo root both "downwards"
> into '<dir>/src' *and* upwards through the parents of '<dir>'. It's not
> totally clear to me what the "right" behavior for Scalar is, but my current
> thought is to follow the same rules as 'setup_git_directory()', but for the
> *enlistment* root rather than the repository root. It's more restrictive
> than GIT_CEILING_DIRECTORIES on a normal git repo, e.g.:
> 
> 1. 'GIT_CEILING_DIRECTORIES=somedir/enlistment git -C somedir/enlistment/src status' 
>    is valid.
> 2. 'GIT_CEILING_DIRECTORIES=somedir/enlistment scalar register somedir/enlistment/src'
>    is not valid.

This is interesting, that we can't recognize the ceiling as the root.

> but since Scalar works on the entire enlistment (not just the repo inside of
> it), I think it makes sense to prevent it from crossing a ceiling directory
> boundary.

I think the enlistment root was something that was inherited from VFS for Git, and we can mostly abandon it. The things we need to do are all based on the Git repository itself, not the parent. The only thing we need to keep is to allow a user to specify the repo by pointing to the directory immediately above the 'src' directory.

Show 7 quoted lines

> 'scalar register somedir/enlistment/test/data' will search:
> 
>   * somedir/enlistment/test/data/src
>   * somedir/enlistment/test/data
>   * somedir/enlistment/test/src
>   * somedir/enlistment/test
>   * somedir/enlistment/src

Instead, we could do the following on a specified <dir>:

 * If <dir>/src exists, find the Git directory by finding the first Git
   repository containing <dir>/src.
 * Otherwise, find the first Git repository containing <dir>.

Is there an easy way to discover a Git repository at a specific directory? Or, do we do something simpler, like changing directories then calling setup_git_directory()? I think simplifying the logic that way should respect GIT_CEILING_DIRECTORIES correctly.

Thanks, -Stolee