Re: [PATCH v3 3/5] fsmonitor: update GIT_TEST_FSMONITOR support

2018-09-28 Thread Junio C Hamano
Ben Peart  writes:

> Junio, can you squash in the following patch or would you prefer I
> reroll the entire series?

Squash it to f8cd77d5 ("fsmonitor: update GIT_TEST_FSMONITOR
support", 2018-09-18) and use the two new lines in the log message?

I can do that.

Thanks.


Re: [PATCH v3 3/5] fsmonitor: update GIT_TEST_FSMONITOR support

2018-09-28 Thread Ben Peart




On 9/28/2018 10:21 AM, Ben Peart wrote:



On 9/28/2018 6:01 AM, SZEDER Gábor wrote:

On Tue, Sep 18, 2018 at 11:29:35PM +, Ben Peart wrote:

diff --git a/t/README b/t/README
index 56a417439c..47165f7eab 100644
--- a/t/README
+++ b/t/README
@@ -319,6 +319,10 @@ GIT_TEST_OE_DELTA_SIZE= exercises the 
uncommon pack-objects code

  path where deltas larger than this limit require extra memory
  allocation for bookkeeping.
+GIT_TEST_FSMONITOR=$PWD/t7519/fsmonitor-all exercises the fsmonitor
+code path for utilizing a file system monitor to speed up detecting
+new or changed files.


Here you tell us to set GIT_TEST_FSMONITOR to an absolute path, and we
are good to go.


diff --git a/t/t7519-status-fsmonitor.sh b/t/t7519-status-fsmonitor.sh
index 756beb0d8e..d77012ea6d 100755
--- a/t/t7519-status-fsmonitor.sh
+++ b/t/t7519-status-fsmonitor.sh
@@ -8,7 +8,7 @@ test_description='git status with file system watcher'
  # To run the entire git test suite using fsmonitor:
  #
  # copy t/t7519/fsmonitor-all to a location in your path and then set
-# GIT_FSMONITOR_TEST=fsmonitor-all and run your tests.
+# GIT_TEST_FSMONITOR=fsmonitor-all and run your tests.


But this old comment is different, suggesting copying that script to
our $PATH.

I prefer your instructions above, because it's only a single step,
and, more importantly, it won't pollute my $PATH.  I think this
comment should be updated to make the advices in both places
consistent.  Or perhaps even removed, now that all GIT_TEST variables
are documented in the same place?



I prefer the suggestion to simply remove this text from the test script 
now that there is documentation for it in the t/README file.


Junio, can you squash in the following patch or would you prefer I 
reroll the entire series?


Thanks,

Ben

From 393007340dc1baf3539ab727e0a8128e7c408a27 Mon Sep 17 00:00:00 2001
From: Ben Peart 
Date: Fri, 28 Sep 2018 10:23:18 -0400
Subject: fixup! fsmonitor: remove outdated instructions from test

Remove the outdated instructions on how to run the test suite utilizing
fsmonitor now that it is properly documented in t/README.

Signed-off-by: Ben Peart 
---

Notes:
Base Ref: git-test-cleanup-v3
Web-Diff: https://github.com/benpeart/git/commit/393007340d
Checkout: git fetch https://github.com/benpeart/git 
git-test-cleanup-v1 && git checkout 393007340d


 t/t7519-status-fsmonitor.sh | 7 ---
 1 file changed, 7 deletions(-)

diff --git a/t/t7519-status-fsmonitor.sh b/t/t7519-status-fsmonitor.sh
index 8308d6d5b1..3f0dd98010 100755
--- a/t/t7519-status-fsmonitor.sh
+++ b/t/t7519-status-fsmonitor.sh
@@ -4,13 +4,6 @@ test_description='git status with file system watcher'

 . ./test-lib.sh

-#
-# To run the entire git test suite using fsmonitor:
-#
-# copy t/t7519/fsmonitor-all to a location in your path and then set
-# GIT_TEST_FSMONITOR=fsmonitor-all and run your tests.
-#
-
 # Note, after "git reset --hard HEAD" no extensions exist other than 
'TREE'

 # "git update-index --fsmonitor" can be used to get the extension written
 # before testing the results.

base-commit: 043246d9369fb851c5c2b922466f77fc7ef0327b
--
2.18.0.windows.1



Re: [PATCH v3 3/5] fsmonitor: update GIT_TEST_FSMONITOR support

2018-09-28 Thread Ben Peart




On 9/28/2018 6:01 AM, SZEDER Gábor wrote:

On Tue, Sep 18, 2018 at 11:29:35PM +, Ben Peart wrote:

diff --git a/t/README b/t/README
index 56a417439c..47165f7eab 100644
--- a/t/README
+++ b/t/README
@@ -319,6 +319,10 @@ GIT_TEST_OE_DELTA_SIZE= exercises the uncommon 
pack-objects code
  path where deltas larger than this limit require extra memory
  allocation for bookkeeping.
  
+GIT_TEST_FSMONITOR=$PWD/t7519/fsmonitor-all exercises the fsmonitor

+code path for utilizing a file system monitor to speed up detecting
+new or changed files.


Here you tell us to set GIT_TEST_FSMONITOR to an absolute path, and we
are good to go.


diff --git a/t/t7519-status-fsmonitor.sh b/t/t7519-status-fsmonitor.sh
index 756beb0d8e..d77012ea6d 100755
--- a/t/t7519-status-fsmonitor.sh
+++ b/t/t7519-status-fsmonitor.sh
@@ -8,7 +8,7 @@ test_description='git status with file system watcher'
  # To run the entire git test suite using fsmonitor:
  #
  # copy t/t7519/fsmonitor-all to a location in your path and then set
-# GIT_FSMONITOR_TEST=fsmonitor-all and run your tests.
+# GIT_TEST_FSMONITOR=fsmonitor-all and run your tests.


But this old comment is different, suggesting copying that script to
our $PATH.

I prefer your instructions above, because it's only a single step,
and, more importantly, it won't pollute my $PATH.  I think this
comment should be updated to make the advices in both places
consistent.  Or perhaps even removed, now that all GIT_TEST variables
are documented in the same place?



I prefer the suggestion to simply remove this text from the test script 
now that there is documentation for it in the t/README file.


Re: [PATCH v3 3/5] fsmonitor: update GIT_TEST_FSMONITOR support

2018-09-28 Thread SZEDER Gábor
On Tue, Sep 18, 2018 at 11:29:35PM +, Ben Peart wrote:
> diff --git a/t/README b/t/README
> index 56a417439c..47165f7eab 100644
> --- a/t/README
> +++ b/t/README
> @@ -319,6 +319,10 @@ GIT_TEST_OE_DELTA_SIZE= exercises the uncommon 
> pack-objects code
>  path where deltas larger than this limit require extra memory
>  allocation for bookkeeping.
>  
> +GIT_TEST_FSMONITOR=$PWD/t7519/fsmonitor-all exercises the fsmonitor
> +code path for utilizing a file system monitor to speed up detecting
> +new or changed files.

Here you tell us to set GIT_TEST_FSMONITOR to an absolute path, and we
are good to go.

> diff --git a/t/t7519-status-fsmonitor.sh b/t/t7519-status-fsmonitor.sh
> index 756beb0d8e..d77012ea6d 100755
> --- a/t/t7519-status-fsmonitor.sh
> +++ b/t/t7519-status-fsmonitor.sh
> @@ -8,7 +8,7 @@ test_description='git status with file system watcher'
>  # To run the entire git test suite using fsmonitor:
>  #
>  # copy t/t7519/fsmonitor-all to a location in your path and then set
> -# GIT_FSMONITOR_TEST=fsmonitor-all and run your tests.
> +# GIT_TEST_FSMONITOR=fsmonitor-all and run your tests.

But this old comment is different, suggesting copying that script to
our $PATH.

I prefer your instructions above, because it's only a single step,
and, more importantly, it won't pollute my $PATH.  I think this
comment should be updated to make the advices in both places
consistent.  Or perhaps even removed, now that all GIT_TEST variables
are documented in the same place?



[PATCH v3 3/5] fsmonitor: update GIT_TEST_FSMONITOR support

2018-09-18 Thread Ben Peart
Rename GIT_FSMONITOR_TEST to GIT_TEST_FSMONITOR for consistency with the
other GIT_TEST_ special setups and properly document its use.

Add logic in t/test-lib.sh to give a warning when the old variable is set to
let people know they need to update their environment to use the new
variable.

Signed-off-by: Ben Peart 
---
 config.c|  2 +-
 t/README|  4 
 t/t1700-split-index.sh  |  2 +-
 t/t7519-status-fsmonitor.sh |  2 +-
 t/test-lib.sh   | 20 
 5 files changed, 27 insertions(+), 3 deletions(-)

diff --git a/config.c b/config.c
index 3461993f0a..3555c63f28 100644
--- a/config.c
+++ b/config.c
@@ -2278,7 +2278,7 @@ int git_config_get_max_percent_split_change(void)
 int git_config_get_fsmonitor(void)
 {
if (git_config_get_pathname("core.fsmonitor", _fsmonitor))
-   core_fsmonitor = getenv("GIT_FSMONITOR_TEST");
+   core_fsmonitor = getenv("GIT_TEST_FSMONITOR");
 
if (core_fsmonitor && !*core_fsmonitor)
core_fsmonitor = NULL;
diff --git a/t/README b/t/README
index 56a417439c..47165f7eab 100644
--- a/t/README
+++ b/t/README
@@ -319,6 +319,10 @@ GIT_TEST_OE_DELTA_SIZE= exercises the uncommon 
pack-objects code
 path where deltas larger than this limit require extra memory
 allocation for bookkeeping.
 
+GIT_TEST_FSMONITOR=$PWD/t7519/fsmonitor-all exercises the fsmonitor
+code path for utilizing a file system monitor to speed up detecting
+new or changed files.
+
 Naming Tests
 
 
diff --git a/t/t1700-split-index.sh b/t/t1700-split-index.sh
index b3b4d83eaf..f6a856f24c 100755
--- a/t/t1700-split-index.sh
+++ b/t/t1700-split-index.sh
@@ -6,7 +6,7 @@ test_description='split index mode tests'
 
 # We need total control of index splitting here
 sane_unset GIT_TEST_SPLIT_INDEX
-sane_unset GIT_FSMONITOR_TEST
+sane_unset GIT_TEST_FSMONITOR
 
 test_expect_success 'enable split index' '
git config splitIndex.maxPercentChange 100 &&
diff --git a/t/t7519-status-fsmonitor.sh b/t/t7519-status-fsmonitor.sh
index 756beb0d8e..d77012ea6d 100755
--- a/t/t7519-status-fsmonitor.sh
+++ b/t/t7519-status-fsmonitor.sh
@@ -8,7 +8,7 @@ test_description='git status with file system watcher'
 # To run the entire git test suite using fsmonitor:
 #
 # copy t/t7519/fsmonitor-all to a location in your path and then set
-# GIT_FSMONITOR_TEST=fsmonitor-all and run your tests.
+# GIT_TEST_FSMONITOR=fsmonitor-all and run your tests.
 #
 
 # Note, after "git reset --hard HEAD" no extensions exist other than 'TREE'
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 44288cbb59..653688c067 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -140,6 +140,26 @@ then
export GIT_INDEX_VERSION
 fi
 
+check_var_migration () {
+   old_name=$1 new_name=$2
+   eval "old_isset=\${${old_name}:+isset}"
+   eval "new_isset=\${${new_name}:+isset}"
+   case "$old_isset,$new_isset" in
+   isset,)
+   echo >&2 "warning: $old_name is now $new_name"
+   echo >&2 "hint: set $new_name too during the transition period"
+   eval "$new_name=\$$old_name"
+   ;;
+   isset,isset)
+   # do this later
+   # echo >&2 "warning: $old_name is now $new_name"
+   # echo >&2 "hint: remove $old_name"
+   ;;
+   esac
+}
+
+check_var_migration GIT_FSMONITOR_TEST GIT_TEST_FSMONITOR
+
 # Add libc MALLOC and MALLOC_PERTURB test
 # only if we are not executing the test with valgrind
 if expr " $GIT_TEST_OPTS " : ".* --valgrind " >/dev/null ||
-- 
2.18.0.windows.1