[PATCH v2] status: long status advice adapted to recent capabilities

From: Rudy Rigot <rudy.rigot@gmail.com>

Currently, if git-status takes more than 2 seconds for enumerating untracked files, a piece of advice is given to the user to consider ignoring untracked files. But Git now offers more possibilities to resolve that situation (untracked cache, fsmonitor) with different downsides.

This change is about refreshing that advice message. A new section in the documentation is introduced to present the possibilities, and the advice message links to it. I'm also introducing tests for this advice message, which was untested so far.

One of the downsides of untracked cache / fsmonitor, is that the first call may be long in order to generate the cache, but the user may not know what their current configuration is. When collecting feedback from users of our very large repo, that's the most common point of confusion that keeps coming back: people complain about git status being slow, but are satisfied when we inform them that it's being cached and they should run it again to check. As a result, the advice message tries to keep them informed of their current configuration.

Signed-off-by: Rudy Rigot <rudy.rigot@gmail.com>
---
    status: long status advice adapted to recent capabilities
    
    Here is version 2 for this patch to change the long status advice to
    adapt to recent capabilities (untracked cache, FSMonitor).
    
    Clerical note: I /preview'd this with GitGitGadget, and this seems to be
    expressed as a patch on top of my first patch ("Range-diff vs v1")
    first, before showing the actual patch as refreshed. I'm assuming that
    is what is expected, please let me know if I'm doing this wrong.
    
    Changes since v1:
    
     * Introduced a new section in git status's docs to explain the various
       options, that the new advice can link to. I realized there's already
       solid details about untracked cache and fsmonitor in the
       update-index's docs, so I kept the new section very high-level and
       strategic, linking to update-index's docs for more details. I don't
       think anything's inaccurate, but I really could use some eyes on this
       anyway to be sure.
     * Changed the advice message depending on context (see test files to
       see every possible phrasing the end user can meet). I am very open to
       feedback, of course. One of my priorities was to keep the
       already-optimized user in the loop about what optimizations they may
       already have, in case they're only seeing this message because it's
       the git status run that did the first caching, since that's what's
       been confusing our users when using fsmonitor.
     * Introduced tests for all this. I chose to simulate the slowness
       thanks to a an environment variable rather than a symbol, just
       because it was also helpful to set it from the outside at dev time,
       but I'm not strongly opinionated about whether it should be one or
       the other.
     * Removed the advice.statusFsmonitor config I had introduced in v1,
       since it's all one short consolidated advice message now.

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1384%2Frudyrigot%2Fadvice_statusFsmonitor-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1384/rudyrigot/advice_statusFsmonitor-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/1384

Range-diff vs v1:

 1:  cc372e7c9d0 ! 1:  9ef7f1834b7 fsmonitor: long status advice adapted to the fsmonitor use case
     @@ Metadata
      Author: Rudy Rigot <rudy.rigot@gmail.com>
      
       ## Commit message ##
     -    fsmonitor: long status advice adapted to the fsmonitor use case
     +    status: long status advice adapted to recent capabilities
      
     -    Currently, if git-status takes more than 2 seconds for enumerating
     -    untracked files, a piece of advice is given to the user to consider
     -    ignoring untracked files. This is somewhat at odds with the UX
     -    upsides from having fsmonitor enabled, since fsmonitor will be
     -    here to take care of mitigating the performance downsides from
     -    those untracked files.
     +    Currently, if git-status takes more than 2 seconds for enumerating untracked
     +    files, a piece of advice is given to the user to consider ignoring untracked
     +    files. But Git now offers more possibilities to resolve that situation
     +    (untracked cache, fsmonitor) with different downsides.
      
     -    I considered just suppressing that piece of advice entirely for
     -    repositories with fsmonitor disabled, but I decided to replace
     -    it with another piece of advice instead, letting the user know
     -    that this run may have been slow, but the next ones should be faster.
     -    Of course, please let me know if the phrasing can be improved. To
     -    keep consistent with other pieces of advice, this new one can be
     -    hidden with a new advice.statusFsmonitor config.
     +    This change is about refreshing that advice message. A new section in the
     +    documentation is introduced to present the possibilities, and the advice
     +    message links to it. I'm also introducing tests for this advice message,
     +    which was untested so far.
      
     -    If the repository does not have fsmonitor enabled, or if the new
     -    piece of advice is hidden by config, the behavior falls back to
     -    today's behavior: show the message advising to ignore untracked
     -    files, as long as it wasn't disabled with the existing advice.statusUoption
     -    config.
     -
     -    Test-wise, I tried to figure out ways to mock the behavior of a
     -    slow git-status, but I couldn't figure it out, so I could use some
     -    advice. I tracked down Commit 6a38ef2ced (status: advise to consider
     -    use of -u when read_directory takes too long, 2013-03-13), and it
     -    also didn't have tests, so I'm questioning whether it can in fact
     -    be reasonably done. Thanks in advance for any guidance.
     +    One of the downsides of untracked cache / fsmonitor, is that the first call
     +    may be long in order to generate the cache, but the user may not know what
     +    their current configuration is. When collecting feedback from users of our
     +    very large repo, that's the most common point of confusion that keeps coming
     +    back: people complain about git status being slow, but are satisfied when
     +    we inform them that it's being cached and they should run it again to check.
     +    As a result, the advice message tries to keep them informed of their current
     +    configuration.
      
          Signed-off-by: Rudy Rigot <rudy.rigot@gmail.com>
      
     - ## Documentation/config/advice.txt ##
     -@@ Documentation/config/advice.txt: advice.*::
     - 		and that calculation takes longer than expected. Will not
     - 		appear if `status.aheadBehind` is false or the option
     - 		`--no-ahead-behind` is given.
     -+	statusFsmonitor::
     -+		Shown when the linkgit:git-status[1] command takes more than
     -+		2 seconds to enumerate untracked files, and fsmonitor is enabled.
     - 	statusHints::
     - 		Show directions on how to proceed from the current
     - 		state in the output of linkgit:git-status[1], in
     + ## Documentation/git-status.txt ##
     +@@ Documentation/git-status.txt: during the write may conflict with other simultaneous processes, causing
     + them to fail. Scripts running `status` in the background should consider
     + using `git --no-optional-locks status` (see linkgit:git[1] for details).
     + 
     ++UNTRACKED FILES AND STATUS SPEED
     ++--------------------------------
     ++
     ++If your untracked files take an unusual amount of time to enumerate, your
     ++repository certainly has a lot of them, and an advice message will display
     ++about it. Here are some configurations to consider in order to improve the
     ++situation:
     ++
     ++* Setting the `core.untrackedCache` configuration as `true` will allow for
     ++`git status` to keep track of the mtime of folders, in order to cache past
     ++`status` results and be sure to only browse folders that changed on subsequent
     ++runs, for filesystems that can support it (see linkgit:git-update-index[1]
     ++for details).
     ++* Used in conjonction with `core.untrackedCache`, setting the `core.fsmonitor`
     ++configuration as `true` will allow for `git status` to keep track of what
     ++files recently changed, in order to cache past `status` results and be sure
     ++to only focus on those files on subsequent runs (see linkgit:git-update-index[1]
     ++for details).
     ++* If none of the above options are satisfactory, setting the
     ++`status.showUntrackedFiles` configuration as `no` will cause `git status`
     ++to not attempt to list untracked files anymore, in which case you have to be
     ++careful not to forget to add new files yourself.
     ++
     ++If none of the above solutions are satisfactory, and you are bothered with
     ++the advice message, you can disable it by setting the `advice.statusUoption`
     ++configuration to `false`.
     ++
     + SEE ALSO
     + --------
     + linkgit:gitignore[5]
     +
     + ## t/t7065-wtstatus-slow.sh (new) ##
     +@@
     ++#!/bin/sh
     ++
     ++test_description='test status when slow untracked files'
     ++
     ++. ./test-lib.sh
     ++
     ++DATA="$TEST_DIRECTORY/t7065"
     ++
     ++GIT_TEST_UF_DELAY_WARNING=1
     ++export GIT_TEST_UF_DELAY_WARNING
     ++
     ++test_expect_success setup '
     ++	git checkout -b test
     ++'
     ++
     ++test_expect_success 'when core.untrackedCache and fsmonitor are unset' '
     ++	test_must_fail git config --get core.untrackedCache &&
     ++	test_must_fail git config --get core.fsmonitor &&
     ++    git status | sed "s/[0-9]\.[0-9][0-9]/X/g" >../actual &&
     ++    test_cmp "$DATA/no_untrackedcache_no_fsmonitor" ../actual &&
     ++    rm -fr ../actual
     ++'
     ++
     ++test_expect_success 'when core.untrackedCache true, but not fsmonitor' '
     ++    git config core.untrackedCache true &&
     ++	test_must_fail git config --get core.fsmonitor &&
     ++    git status | sed "s/[0-9]\.[0-9][0-9]/X/g" >../actual &&
     ++    test_cmp "$DATA/with_untrackedcache_no_fsmonitor" ../actual &&
     ++    rm -fr ../actual
     ++'
     ++
     ++test_expect_success 'when core.untrackedCache true, and fsmonitor' '
     ++    git config core.untrackedCache true &&
     ++	git config core.fsmonitor true &&
     ++    git status | sed "s/[0-9]\.[0-9][0-9]/X/g" >../actual &&
     ++    test_cmp "$DATA/with_untrackedcache_with_fsmonitor" ../actual &&
     ++    rm -fr ../actual
     ++'
     ++
     ++test_done
     + \ No newline at end of file
      
     - ## advice.c ##
     -@@ advice.c: static struct {
     - 	[ADVICE_SET_UPSTREAM_FAILURE]			= { "setUpstreamFailure", 1 },
     - 	[ADVICE_SKIPPED_CHERRY_PICKS]			= { "skippedCherryPicks", 1 },
     - 	[ADVICE_STATUS_AHEAD_BEHIND_WARNING]		= { "statusAheadBehindWarning", 1 },
     -+	[ADVICE_STATUS_FSMONITOR]			= { "statusFsmonitor", 1 },
     - 	[ADVICE_STATUS_HINTS]				= { "statusHints", 1 },
     - 	[ADVICE_STATUS_U_OPTION]			= { "statusUoption", 1 },
     - 	[ADVICE_SUBMODULE_ALTERNATE_ERROR_STRATEGY_DIE] = { "submoduleAlternateErrorStrategyDie", 1 },
     + ## t/t7065/no_untrackedcache_no_fsmonitor (new) ##
     +@@
     ++On branch test
     ++
     ++No commits yet
     ++
     ++
     ++It took X seconds to enumerate untracked files.
     ++See https://git-scm.com/docs/git-status#_untracked_files_and_status_speed
     ++for configuration options that may improve that time.
     ++
     ++nothing to commit (create/copy files and use "git add" to track)
      
     - ## advice.h ##
     -@@ advice.h: struct string_list;
     - 	ADVICE_SEQUENCER_IN_USE,
     - 	ADVICE_SET_UPSTREAM_FAILURE,
     - 	ADVICE_STATUS_AHEAD_BEHIND_WARNING,
     -+	ADVICE_STATUS_FSMONITOR,
     - 	ADVICE_STATUS_HINTS,
     - 	ADVICE_STATUS_U_OPTION,
     - 	ADVICE_SUBMODULE_ALTERNATE_ERROR_STRATEGY_DIE,
     + ## t/t7065/with_untrackedcache_no_fsmonitor (new) ##
     +@@
     ++On branch test
     ++
     ++No commits yet
     ++
     ++
     ++It took X seconds to enumerate untracked files,
     ++but this is currently being cached, with fsmonitor OFF.
     ++See https://git-scm.com/docs/git-status#_untracked_files_and_status_speed
     ++for configuration options that may improve that time.
     ++
     ++nothing to commit (create/copy files and use "git add" to track)
     +
     + ## t/t7065/with_untrackedcache_with_fsmonitor (new) ##
     +@@
     ++On branch test
     ++
     ++No commits yet
     ++
     ++
     ++It took X seconds to enumerate untracked files,
     ++but this is currently being cached, with fsmonitor ON.
     ++See https://git-scm.com/docs/git-status#_untracked_files_and_status_speed
     ++for configuration options that may improve that time.
     ++
     ++nothing to commit (create/copy files and use "git add" to track)
      
       ## wt-status.c ##
      @@
     @@ wt-status.c
      +#include "fsmonitor-settings.h"
       
       #define AB_DELAY_WARNING_IN_MS (2 * 1000)
     ++#define UF_DELAY_WARNING_IN_MS (2 * 1000)
     + 
     + static const char cut_line[] =
     + "------------------------ >8 ------------------------\n";
     +@@ wt-status.c: static void wt_longstatus_print_tracking(struct wt_status *s)
     + 	strbuf_release(&sb);
     + }
       
     ++static inline int uf_was_slow(uint32_t untracked_in_ms)
     ++{
     ++	const char *x;
     ++	x = getenv("GIT_TEST_UF_DELAY_WARNING");
     ++	if (x) {
     ++		untracked_in_ms += UF_DELAY_WARNING_IN_MS + 1;
     ++	}
     ++
     ++	return UF_DELAY_WARNING_IN_MS < untracked_in_ms;
     ++}
     ++
     + static void show_merge_in_progress(struct wt_status *s,
     + 				   const char *color)
     + {
      @@ wt-status.c: static void wt_longstatus_print(struct wt_status *s)
       {
       	const char *branch_color = color(WT_STATUS_ONBRANCH, s);
     @@ wt-status.c: static void wt_longstatus_print(struct wt_status *s)
      -					   "may speed it up, but you have to be careful not to forget to add\n"
      -					   "new files yourself (see 'git help status')."),
      -					 s->untracked_in_ms / 1000.0);
     -+		if (2000 < s->untracked_in_ms) {
     -+			if (advice_enabled(ADVICE_STATUS_FSMONITOR) && fsm_mode > FSMONITOR_MODE_DISABLED) {
     ++		if (uf_was_slow(s->untracked_in_ms)) {
     ++			if (advice_enabled(ADVICE_STATUS_U_OPTION)) {
      +				status_printf_ln(s, GIT_COLOR_NORMAL, "%s", "");
     ++				if (s->repo->settings.core_untracked_cache == UNTRACKED_CACHE_WRITE) {
     ++					status_printf_ln(s, GIT_COLOR_NORMAL,
     ++							_("It took %.2f seconds to enumerate untracked files,\n"
     ++							"but this is currently being cached, with fsmonitor %s."),
     ++							s->untracked_in_ms / 1000.0,
     ++							(fsm_mode > FSMONITOR_MODE_DISABLED) ? "ON" : "OFF");
     ++				} else {
     ++					status_printf_ln(s, GIT_COLOR_NORMAL,
     ++							_("It took %.2f seconds to enumerate untracked files."),
     ++							s->untracked_in_ms / 1000.0);
     ++				}
      +				status_printf_ln(s, GIT_COLOR_NORMAL,
     -+						_("It took a while to check your git status this time, but the results\n"
     -+						"were cached, and your next runs should be faster."));
     -+			} else if (advice_enabled(ADVICE_STATUS_U_OPTION)) {
     ++						_("See https://git-scm.com/docs/git-status#_untracked_files_and_status_speed\n"
     ++						"for configuration options that may improve that time."));
      +				status_printf_ln(s, GIT_COLOR_NORMAL, "%s", "");
     -+				status_printf_ln(s, GIT_COLOR_NORMAL,
     -+						_("It took %.2f seconds to enumerate untracked files. 'status -uno'\n"
     -+						"may speed it up, but you have to be careful not to forget to add\n"
     -+						"new files yourself (see 'git help status')."),
     -+						s->untracked_in_ms / 1000.0);
      +			}
       		}
       	} else if (s->committable)

 Documentation/git-status.txt               | 27 +++++++++++++++
 t/t7065-wtstatus-slow.sh                   | 40 ++++++++++++++++++++++
 t/t7065/no_untrackedcache_no_fsmonitor     | 10 ++++++
 t/t7065/with_untrackedcache_no_fsmonitor   | 11 ++++++
 t/t7065/with_untrackedcache_with_fsmonitor | 11 ++++++
 wt-status.c                                | 40 ++++++++++++++++++----
 6 files changed, 132 insertions(+), 7 deletions(-)
 create mode 100755 t/t7065-wtstatus-slow.sh
 create mode 100644 t/t7065/no_untrackedcache_no_fsmonitor
 create mode 100644 t/t7065/with_untrackedcache_no_fsmonitor
 create mode 100644 t/t7065/with_untrackedcache_with_fsmonitor