git@vger.kernel.org mailing list mirror (one of many)
 help / Atom feed
* [PATCH v1] fsmonitor: update GIT_TEST_FSMONITOR support
@ 2018-09-13 17:45 Ben Peart
  2018-09-13 18:03 ` Junio C Hamano
  2018-09-13 18:54 ` Ævar Arnfjörð Bjarmason
  0 siblings, 2 replies; 4+ messages in thread
From: Ben Peart @ 2018-09-13 17:45 UTC (permalink / raw)
  To: git; +Cc: gitster, Ben Peart, Ben Peart

Rename GIT_FSMONITOR_TEST to GIT_TEST_FSMONITOR for consistency with the
other GIT_TEST_ special setups and properly document its use.

Signed-off-by: Ben Peart <Ben.Peart@microsoft.com>
---

Notes:
    Base Ref: v2.19.0
    Web-Diff: https://github.com/benpeart/git/commit/311484a684
    Checkout: git fetch https://github.com/benpeart/git git-test-fsmonitor-v1 && git checkout 311484a684

 config.c                    | 2 +-
 t/README                    | 4 ++++
 t/t1700-split-index.sh      | 2 +-
 t/t7519-status-fsmonitor.sh | 2 +-
 4 files changed, 7 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", &core_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 9028b47d92..545438c820 100644
--- a/t/README
+++ b/t/README
@@ -319,6 +319,10 @@ GIT_TEST_OE_DELTA_SIZE=<n> exercises the uncomon 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'

base-commit: 1d4361b0f344188ab5eec6dcea01f61a3a3a1670
-- 
2.18.0.windows.1


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v1] fsmonitor: update GIT_TEST_FSMONITOR support
  2018-09-13 17:45 [PATCH v1] fsmonitor: update GIT_TEST_FSMONITOR support Ben Peart
@ 2018-09-13 18:03 ` Junio C Hamano
  2018-09-13 18:54 ` Ævar Arnfjörð Bjarmason
  1 sibling, 0 replies; 4+ messages in thread
From: Junio C Hamano @ 2018-09-13 18:03 UTC (permalink / raw)
  To: Ben Peart; +Cc: git\, Ben Peart

Ben Peart <benpeart@microsoft.com> writes:

> Rename GIT_FSMONITOR_TEST to GIT_TEST_FSMONITOR for consistency with the
> other GIT_TEST_ special setups and properly document its use.

Makes sense.

Thanks for such an attention to detail.

>
> Signed-off-by: Ben Peart <Ben.Peart@microsoft.com>
> ---
>
> Notes:
>     Base Ref: v2.19.0
>     Web-Diff: https://github.com/benpeart/git/commit/311484a684
>     Checkout: git fetch https://github.com/benpeart/git git-test-fsmonitor-v1 && git checkout 311484a684
>
>  config.c                    | 2 +-
>  t/README                    | 4 ++++
>  t/t1700-split-index.sh      | 2 +-
>  t/t7519-status-fsmonitor.sh | 2 +-
>  4 files changed, 7 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", &core_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 9028b47d92..545438c820 100644
> --- a/t/README
> +++ b/t/README
> @@ -319,6 +319,10 @@ GIT_TEST_OE_DELTA_SIZE=<n> exercises the uncomon 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'
>
> base-commit: 1d4361b0f344188ab5eec6dcea01f61a3a3a1670

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v1] fsmonitor: update GIT_TEST_FSMONITOR support
  2018-09-13 17:45 [PATCH v1] fsmonitor: update GIT_TEST_FSMONITOR support Ben Peart
  2018-09-13 18:03 ` Junio C Hamano
@ 2018-09-13 18:54 ` Ævar Arnfjörð Bjarmason
  2018-09-14 13:09   ` Ben Peart
  1 sibling, 1 reply; 4+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2018-09-13 18:54 UTC (permalink / raw)
  To: Ben Peart; +Cc: git\, gitster\, Ben Peart


On Thu, Sep 13 2018, Ben Peart wrote:

> 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", &core_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 9028b47d92..545438c820 100644
> --- a/t/README
> +++ b/t/README
> @@ -319,6 +319,10 @@ GIT_TEST_OE_DELTA_SIZE=<n> exercises the uncomon 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
>  ------------

I've seen this & will watch out for it, but still, when I'm updating to
"next" in a couple of months I may not be tracking the exact state of
the integration of this patch, and then running with
GIT_FSMONITOR_TEST=... will suddenly be a noop.

So maybe something like this to test-lib.sh as well (or directly in
config.c):

    if test -n "$GIT_FSMONITOR_TEST"
    then
        echo "The GIT_FSMONITOR_TEST variable has been renamed to GIT_TEST_FSMONITOR"
        exit 1
    fi

Maybe I'm being too nitpicky and there's only two of us who run the test
with this anyway, and we can deal with it.

It just rubs me the wrong way that we have a test mode that silently
stops being picked up because a command-line option or env variable got
renamed, especially since we've had it for 4 stable releases, especially
since it's so easy for us to avoid that confusion (just die),
v.s. potential time wasted downstream (wondering why fsmonitor stuff
broke on $SOME_OS even though we're testing for it during package
build...).

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v1] fsmonitor: update GIT_TEST_FSMONITOR support
  2018-09-13 18:54 ` Ævar Arnfjörð Bjarmason
@ 2018-09-14 13:09   ` Ben Peart
  0 siblings, 0 replies; 4+ messages in thread
From: Ben Peart @ 2018-09-14 13:09 UTC (permalink / raw)
  To: Ævar Arnfjörð Bjarmason, Ben Peart; +Cc: git, gitster, Ben Peart



On 9/13/2018 2:54 PM, Ævar Arnfjörð Bjarmason wrote:
> 
> On Thu, Sep 13 2018, Ben Peart wrote:
> 
>> 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", &core_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 9028b47d92..545438c820 100644
>> --- a/t/README
>> +++ b/t/README
>> @@ -319,6 +319,10 @@ GIT_TEST_OE_DELTA_SIZE=<n> exercises the uncomon 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
>>   ------------
> 
> I've seen this & will watch out for it, but still, when I'm updating to
> "next" in a couple of months I may not be tracking the exact state of
> the integration of this patch, and then running with
> GIT_FSMONITOR_TEST=... will suddenly be a noop.
> 
> So maybe something like this to test-lib.sh as well (or directly in
> config.c):
> 
>      if test -n "$GIT_FSMONITOR_TEST"
>      then
>          echo "The GIT_FSMONITOR_TEST variable has been renamed to GIT_TEST_FSMONITOR"
>          exit 1
>      fi
> 
> Maybe I'm being too nitpicky and there's only two of us who run the test
> with this anyway, and we can deal with it.
> 

I agree that there are probably only 2 people in the world who ever used 
this but I'm happy to add the additional test to make it obvious it has 
been renamed.

> It just rubs me the wrong way that we have a test mode that silently
> stops being picked up because a command-line option or env variable got
> renamed, especially since we've had it for 4 stable releases, especially
> since it's so easy for us to avoid that confusion (just die),
> v.s. potential time wasted downstream (wondering why fsmonitor stuff
> broke on $SOME_OS even though we're testing for it during package
> build...).
> 

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, back to index

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-09-13 17:45 [PATCH v1] fsmonitor: update GIT_TEST_FSMONITOR support Ben Peart
2018-09-13 18:03 ` Junio C Hamano
2018-09-13 18:54 ` Ævar Arnfjörð Bjarmason
2018-09-14 13:09   ` Ben Peart

git@vger.kernel.org mailing list mirror (one of many)

Archives are clonable:
	git clone --mirror https://public-inbox.org/git
	git clone --mirror http://ou63pmih66umazou.onion/git
	git clone --mirror http://czquwvybam4bgbro.onion/git
	git clone --mirror http://hjrcffqmbrq6wope.onion/git

Newsgroups are available over NNTP:
	nntp://news.public-inbox.org/inbox.comp.version-control.git
	nntp://ou63pmih66umazou.onion/inbox.comp.version-control.git
	nntp://czquwvybam4bgbro.onion/inbox.comp.version-control.git
	nntp://hjrcffqmbrq6wope.onion/inbox.comp.version-control.git
	nntp://news.gmane.org/gmane.comp.version-control.git

 note: .onion URLs require Tor: https://www.torproject.org/
       or Tor2web: https://www.tor2web.org/

AGPL code for this site: git clone https://public-inbox.org/ public-inbox