Re: [PATCH v2 8/8] test-lib: clear watchman watches at test completion

From: Derrick Stolee <stolee@gmail.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: Derrick Stolee via GitGitGadget <gitgitgadget@gmail.com>,
	git@vger.kernel.org, szeder.dev@gmail.com, ukshah2@illinois.edu,
	Kevin.Willford@microsoft.com,
	Derrick Stolee <dstolee@microsoft.com>
Subject: Re: [PATCH v2 8/8] test-lib: clear watchman watches at test completion
Date: Tue, 10 Dec 2019 08:51:16 -0500	[thread overview]
Message-ID: <dd8c14d3-21b6-731c-bd50-650d063d686f@gmail.com> (raw)
In-Reply-To: <xmqqo8wgsqnn.fsf@gitster-ct.c.googlers.com>

On 12/10/2019 12:20 AM, Junio C Hamano wrote:
> Derrick Stolee <stolee@gmail.com> writes:
> 
>> Hm. That is a good point. Can we assume that our version of grep has
>> a "-F" or "--fixed-strings" option? ([1] seems to say that "-F" would
>> work.)
> 
> $ git grep "grep -F" -- \*.sh
> 
> is your friend ;-) 

Yes, of course I should have just looked for examples.

> And never use https://www.gnu.org/ manual as a yardstick---you will
> end up using GNUism that is not unavailable elsewhere pretty easily.

I tried to focus on the part that said "this is part of POSIX", but
you are right that may not be the best place to look.

>> [1] https://www.gnu.org/savannah-checkouts/gnu/grep/manual/grep.html#index-grep-programs
>>
>>> What are these stripping of ", and " about?  Could you tell readers
>>> how a typical output from the program we are reading from looks like
>>> perhaps in the log message or in-code comment around here?
>>
>> Watchman outputs its list of paths in JSON format. Luckily, it formats
>> the output so the path lines are on separate lines, each quoted.
>>
>> For example:
>>
>> {
>> 	"version": "4.9.0",
>> 	"roots": [
>> 		"<path1>",
>> 		"<path2>",
>> 		"<path3>"
>> 	]
>> }
> 
> Yeek; how is a dq in path represented?  by doubling?  by
> backslash-quoting (if so how is a backslash in path represented)?
> By something else?
> 
> It's OK at least for now to declare that our test repository does
> not contain any funny paths, but in the longer run does the above
> mean that we somehow need to be able to grok JSON reliably in our
> tests?  It may not be such a bad thing especially for longer term,
> as there are other parts of the system that may benefit from having
> JSON capable output readers in our tests (e.g. trace2 code can do
> JSON, right?)..

trace2 can _write_ JSON, not parse it. However, we have some parsing
code (using a package) in the performance tests. I could try
adapting that for this purpose. That package is not currently required
by the test suite, so it causes some dependency issues when first
running the perf suite. At least we wouldn't need the package
unless running with GIT_TEST_FSMONITOR.

My guess is that this patch is going to be trouble, so I'll eject
it in the next version and save the JSON parsing and everything
for its own series. We only really need it when we are getting
close to running watchman in CI on Windows.

Thanks,
-Stolee