From: "Victoria Dye via GitGitGadget" <gitgitgadget@gmail.com>
To: git@vger.kernel.org
Cc: johannes.schindelin@gmx.de, mjcheetham@outlook.com,
gitster@pobox.com, Victoria Dye <vdye@github.com>
Subject: [PATCH v2 0/5] scalar: enable built-in FSMonitor
Date: Tue, 16 Aug 2022 23:58:04 +0000 [thread overview]
Message-ID: <pull.1324.v2.git.1660694290.gitgitgadget@gmail.com> (raw)
In-Reply-To: <pull.1324.git.1660673269.gitgitgadget@gmail.com>
This series enables the built-in FSMonitor [1] on 'scalar'-registered
repository enlistments. To avoid errors when unregistering an enlistment,
the FSMonitor daemon is explicitly stopped during 'scalar unregister'.
Maintainer's note: this series has a minor conflict with
'vd/scalar-generalize-diagnose'. Please let me know if there's anything else
I can provide (in addition to [2]) that would make resolution easier.
Changes since V1
================
* Added a patch to fix 'unregister_dir()'s handling of return values > 0
from 'toggle_maintenance()' and 'add_or_remove_enlistment()'.
* Added a patch to print error messages in 'register_dir()' and
'unregister_dir()' indicating which of their internal steps fail.
* Moved check of 'fsmonitor_ipc__is_supported()' to '[un]register_dir()' to
avoid calling '(start|stop)_fsmonitor_daemon()' when the feature is not
supported. Added assertion of 'fsmonitor_ipc__is_supported()' to
'(start|stop)_fsmonitor_daemon()' to enforce that they are not called
when the feature is unavailable.
* Simplified '(start|stop)_fsmonitor_daemon()' implementation. Now, if
FSMonitor is already running/stopped (respectively), the function simply
returns 0; otherwise, it runs 'git fsmonitor--daemon (start|stop)' and
returns the exit code.
* Note that the "could not (start|stop) the FSMonitor daemon: <err_msg>"
error messages are no longer printed by
'(start|stop)_fsmonitor_daemon()'. Instead, "<err_msg>" is printed to
stderr by swapping 'pipe_command()' out for 'run_git()', and
'[un]register_dir()' prints the "could not (start|stop) the FSMonitor
daemon" message.
Thanks
* Victoria
[1]
https://lore.kernel.org/git/pull.1143.git.1644940773.gitgitgadget@gmail.com/
[2] The conflict is a result of both series updating the Scalar roadmap doc.
For reference, my merge resolution (from git diff <merge commit> <merge
commit>^1 <merge commit>^2, where <merge commit>^1 is
'vd/scalar-generalize-diagnose' and <merge commit>^2 is this series) looks
like:
------------->8------------->8------------->8------------->8------------->8-------------
diff --cc Documentation/technical/scalar.txt
index f6353375f0,047390e46e..0600150b3a
--- a/Documentation/technical/scalar.txt
+++ b/Documentation/technical/scalar.txt
@@@ -84,20 -84,26 +84,23 @@@ series have been accepted
- `scalar-diagnose`: The `scalar` command is taught the `diagnose` subcommand.
+- `scalar-generalize-diagnose`: Move the functionality of `scalar diagnose`
+ into `git diagnose` and `git bugreport --diagnose`.
+
+ - 'scalar-add-fsmonitor: Enable the built-in FSMonitor in Scalar
+ enlistments. At the end of this series, Scalar should be feature-complete
+ from the perspective of a user.
+
Roughly speaking (and subject to change), the following series are needed to
"finish" this initial version of Scalar:
- - Finish Scalar features: Enable the built-in FSMonitor in Scalar enlistments
- and implement `scalar help`. At the end of this series, Scalar should be
- feature-complete from the perspective of a user.
-- Generalize features not specific to Scalar: In the spirit of making Scalar
- configure only what is needed for large repo performance, move common
- utilities into other parts of Git. Some of this will be internal-only, but one
- major change will be generalizing `scalar diagnose` for use with any Git
- repository.
--
- Move Scalar to toplevel: Move Scalar out of `contrib/` and into the root of
- `git`, including updates to build and install it with the rest of Git. This
- change will incorporate Scalar into the Git CI and test framework, as well as
- expand regression and performance testing to ensure the tool is stable.
+ `git`. This includes a variety of related updates, including:
+ - building & installing Scalar in the Git root-level 'make [install]'.
+ - builing & testing Scalar as part of CI.
+ - moving and expanding test coverage of Scalar (including perf tests).
+ - implementing 'scalar help'/'git help scalar' to display scalar
+ documentation.
Finally, there are two additional patch series that exist in Microsoft's fork of
Git, but there is no current plan to upstream them. There are some interesting
-------------8<-------------8<-------------8<-------------8<-------------8<---------
Johannes Schindelin (1):
scalar unregister: stop FSMonitor daemon
Matthew John Cheetham (1):
scalar: enable built-in FSMonitor on `register`
Victoria Dye (3):
scalar-unregister: handle error codes greater than 0
scalar-[un]register: clearly indicate source of error
scalar: update technical doc roadmap with FSMonitor support
Documentation/technical/scalar.txt | 17 +++++----
contrib/scalar/scalar.c | 55 ++++++++++++++++++++++++------
contrib/scalar/t/t9099-scalar.sh | 11 ++++++
3 files changed, 66 insertions(+), 17 deletions(-)
base-commit: 4af7188bc97f70277d0f10d56d5373022b1fa385
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1324%2Fvdye%2Fscalar%2Fadd-fsmonitor-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1324/vdye/scalar/add-fsmonitor-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/1324
Range-diff vs v1:
-: ----------- > 1: 36fc3cb604d scalar-unregister: handle error codes greater than 0
-: ----------- > 2: 4bacf8bce8a scalar-[un]register: clearly indicate source of error
1: 62682ccf696 ! 3: 5fdf8337972 scalar: enable built-in FSMonitor on `register`
@@ Commit message
For simplicity, we only support the built-in FSMonitor (and no external
file system monitor such as e.g. Watchman).
+ Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Matthew John Cheetham <mjcheetham@outlook.com>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Victoria Dye <vdye@github.com>
@@ contrib/scalar/scalar.c: static int add_or_remove_enlistment(int add)
+static int start_fsmonitor_daemon(void)
+{
-+ int res = 0;
-+ if (fsmonitor_ipc__is_supported() &&
-+ fsmonitor_ipc__get_state() != IPC_STATE__LISTENING) {
-+ struct strbuf err = STRBUF_INIT;
-+ struct child_process cp = CHILD_PROCESS_INIT;
++ assert(fsmonitor_ipc__is_supported());
+
-+ /* Try to start the FSMonitor daemon */
-+ cp.git_cmd = 1;
-+ strvec_pushl(&cp.args, "fsmonitor--daemon", "start", NULL);
-+ if (!pipe_command(&cp, NULL, 0, NULL, 0, &err, 0)) {
-+ /* Successfully started FSMonitor */
-+ strbuf_release(&err);
-+ return 0;
-+ }
++ if (fsmonitor_ipc__get_state() != IPC_STATE__LISTENING)
++ return run_git("fsmonitor--daemon", "start", NULL);
+
-+ /* If FSMonitor really hasn't started, emit error */
-+ if (fsmonitor_ipc__get_state() != IPC_STATE__LISTENING)
-+ res = error(_("could not start the FSMonitor daemon: %s"),
-+ err.buf);
-+
-+ strbuf_release(&err);
-+ }
-+
-+ return res;
++ return 0;
+}
+
static int register_dir(void)
{
- int res = add_or_remove_enlistment(1);
+ if (add_or_remove_enlistment(1))
@@ contrib/scalar/scalar.c: static int register_dir(void)
- if (!res)
- res = toggle_maintenance(1);
+ if (toggle_maintenance(1))
+ return error(_("could not turn on maintenance"));
-+ if (!res)
-+ res = start_fsmonitor_daemon();
++ if (fsmonitor_ipc__is_supported() && start_fsmonitor_daemon())
++ return error(_("could not start the FSMonitor daemon"));
+
- return res;
+ return 0;
}
2: 78a7f0b1be0 ! 4: fc4aa1fde31 scalar unregister: stop FSMonitor daemon
@@ Commit message
that the directory needs to be removed (the daemon would otherwise hold
a handle to that directory, preventing it from being deleted).
+ Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Victoria Dye <vdye@github.com>
## contrib/scalar/scalar.c ##
@@ contrib/scalar/scalar.c: static int start_fsmonitor_daemon(void)
- return res;
+ return 0;
}
+static int stop_fsmonitor_daemon(void)
+{
-+ int res = 0;
-+ if (fsmonitor_ipc__is_supported()) {
-+ struct strbuf err = STRBUF_INIT;
-+ struct child_process cp = CHILD_PROCESS_INIT;
-+
-+ /* Try to stop the FSMonitor daemon */
-+ cp.git_cmd = 1;
-+ strvec_pushl(&cp.args, "fsmonitor--daemon", "stop", NULL);
-+ if (!pipe_command(&cp, NULL, 0, NULL, 0, &err, 0)) {
-+ /* Successfully stopped FSMonitor */
-+ strbuf_release(&err);
-+ return 0;
-+ }
-+
-+ /* If FSMonitor really hasn't stopped, emit error */
-+ if (fsmonitor_ipc__get_state() == IPC_STATE__LISTENING)
-+ res = error(_("could not stop the FSMonitor daemon: %s"),
-+ err.buf);
++ assert(fsmonitor_ipc__is_supported());
+
-+ strbuf_release(&err);
-+ }
++ if (fsmonitor_ipc__get_state() == IPC_STATE__LISTENING)
++ return run_git("fsmonitor--daemon", "stop", NULL);
+
-+ return res;
++ return 0;
+}
+
static int register_dir(void)
{
- int res = add_or_remove_enlistment(1);
+ if (add_or_remove_enlistment(1))
@@ contrib/scalar/scalar.c: static int unregister_dir(void)
- if (add_or_remove_enlistment(0) < 0)
- res = -1;
+ if (add_or_remove_enlistment(0))
+ res = error(_("could not remove enlistment"));
-+ if (stop_fsmonitor_daemon() < 0)
-+ res = -1;
++ if (fsmonitor_ipc__is_supported() && stop_fsmonitor_daemon() < 0)
++ res = error(_("could not stop the FSMonitor daemon"));
+
return res;
}
3: 5457a8ff1fa = 5: dd59caa2e5a scalar: update technical doc roadmap with FSMonitor support
--
gitgitgadget
next prev parent reply other threads:[~2022-08-17 0:00 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 ` Victoria Dye via GitGitGadget [this message]
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
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=pull.1324.v2.git.1660694290.gitgitgadget@gmail.com \
--to=gitgitgadget@gmail.com \
--cc=git@vger.kernel.org \
--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).