git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: Ben Peart <benpeart@microsoft.com>
Cc: "git\@vger.kernel.org" <git@vger.kernel.org>,
	"gitster\@pobox.com" <gitster@pobox.com>,
	Ben Peart <Ben.Peart@microsoft.com>
Subject: Re: [PATCH v1] fsmonitor: update GIT_TEST_FSMONITOR support
Date: Thu, 13 Sep 2018 20:54:46 +0200	[thread overview]
Message-ID: <87h8itkz2h.fsf@evledraar.gmail.com> (raw)
In-Reply-To: <20180913174522.53872-1-benpeart@microsoft.com>


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

  parent reply	other threads:[~2018-09-13 18:54 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 [this message]
2018-09-14 13:09   ` Ben Peart

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=87h8itkz2h.fsf@evledraar.gmail.com \
    --to=avarab@gmail.com \
    --cc=Ben.Peart@microsoft.com \
    --cc=benpeart@microsoft.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.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).