git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Derrick Stolee <derrickstolee@github.com>
To: Victoria Dye <vdye@github.com>,
	Matthew John Cheetham via GitGitGadget  <gitgitgadget@gmail.com>,
	git@vger.kernel.org
Cc: johannes.schindelin@gmx.de, mjcheetham@outlook.com, gitster@pobox.com
Subject: Re: [PATCH v2 3/5] scalar: enable built-in FSMonitor on `register`
Date: Thu, 18 Aug 2022 09:19:24 -0400	[thread overview]
Message-ID: <82716e5b-3522-68f5-7479-1b39811e0cb2@github.com> (raw)
In-Reply-To: <3a238691-c37f-39a1-f4f6-2b8f9b4c4dcb@github.com>

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

> (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. 

...

> 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.

> '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

  reply	other threads:[~2022-08-18 13:20 UTC|newest]

Thread overview: 39+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-08-16 18:07 [PATCH 0/3] scalar: enable built-in FSMonitor Victoria Dye via GitGitGadget
2022-08-16 18:07 ` [PATCH 1/3] scalar: enable built-in FSMonitor on `register` Matthew John Cheetham via GitGitGadget
2022-08-16 20:49   ` Junio C Hamano
2022-08-16 21:57     ` Victoria Dye
2022-08-16 22:15       ` Junio C Hamano
2022-08-16 18:07 ` [PATCH 2/3] scalar unregister: stop FSMonitor daemon Johannes Schindelin via GitGitGadget
2022-08-16 18:07 ` [PATCH 3/3] scalar: update technical doc roadmap with FSMonitor support Victoria Dye via GitGitGadget
2022-08-16 18:21 ` [PATCH 0/3] scalar: enable built-in FSMonitor Junio C Hamano
2022-08-16 18:42   ` Victoria Dye
2022-08-16 18:44     ` Junio C Hamano
2022-08-16 23:58 ` [PATCH v2 0/5] " Victoria Dye via GitGitGadget
2022-08-16 23:58   ` [PATCH v2 1/5] scalar-unregister: handle error codes greater than 0 Victoria Dye via GitGitGadget
2022-08-17 14:33     ` Junio C Hamano
2022-08-16 23:58   ` [PATCH v2 2/5] scalar-[un]register: clearly indicate source of error Victoria Dye via GitGitGadget
2022-08-16 23:58   ` [PATCH v2 3/5] scalar: enable built-in FSMonitor on `register` Matthew John Cheetham via GitGitGadget
2022-08-17 14:34     ` Derrick Stolee
2022-08-17 15:54       ` Junio C Hamano
2022-08-17 23:47       ` Victoria Dye
2022-08-18 13:19         ` Derrick Stolee [this message]
2022-08-17 14:43     ` Junio C Hamano
2022-08-16 23:58   ` [PATCH v2 4/5] scalar unregister: stop FSMonitor daemon Johannes Schindelin via GitGitGadget
2022-08-17 14:39     ` Derrick Stolee
2022-08-17 17:36       ` Victoria Dye
2022-08-17 17:45         ` Derrick Stolee
2022-08-16 23:58   ` [PATCH v2 5/5] scalar: update technical doc roadmap with FSMonitor support Victoria Dye via GitGitGadget
2022-08-17 14:51   ` [PATCH v2 0/5] scalar: enable built-in FSMonitor Derrick Stolee
2022-08-18 21:40   ` [PATCH v3 0/8] " Victoria Dye via GitGitGadget
2022-08-18 21:40     ` [PATCH v3 1/8] scalar: constrain enlistment search Victoria Dye via GitGitGadget
2022-08-19 18:32       ` Derrick Stolee
2022-08-18 21:40     ` [PATCH v3 2/8] scalar-unregister: handle error codes greater than 0 Victoria Dye via GitGitGadget
2022-08-18 21:40     ` [PATCH v3 3/8] scalar-[un]register: clearly indicate source of error Victoria Dye via GitGitGadget
2022-08-18 21:40     ` [PATCH v3 4/8] scalar-delete: do not 'die()' in 'delete_enlistment()' Victoria Dye via GitGitGadget
2022-08-18 21:40     ` [PATCH v3 5/8] scalar: move config setting logic into its own function Victoria Dye via GitGitGadget
2022-08-18 21:40     ` [PATCH v3 6/8] scalar: enable built-in FSMonitor on `register` Matthew John Cheetham via GitGitGadget
2022-08-19 18:44       ` Derrick Stolee
2022-08-18 21:40     ` [PATCH v3 7/8] scalar unregister: stop FSMonitor daemon Johannes Schindelin via GitGitGadget
2022-08-18 21:40     ` [PATCH v3 8/8] scalar: update technical doc roadmap with FSMonitor support Victoria Dye via GitGitGadget
2022-08-19 18:45     ` [PATCH v3 0/8] scalar: enable built-in FSMonitor Derrick Stolee
2022-08-19 21:06       ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=82716e5b-3522-68f5-7479-1b39811e0cb2@github.com \
    --to=derrickstolee@github.com \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=gitster@pobox.com \
    --cc=johannes.schindelin@gmx.de \
    --cc=mjcheetham@outlook.com \
    --cc=vdye@github.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).