On 1/29/2019 10:58 AM, SZEDER Gábor wrote: > On Tue, Jan 29, 2019 at 06:56:08AM -0800, Derrick Stolee via GitGitGadget wrote: >> +prove: all >> + $(MAKE) -C t/ prove >> + > > You don't need this 'prove' target in the "main" Makefile, because > 'make test' will run the test suite using DEFAULT_TEST_TARGET anyway. Thanks! >> +coverage-prove: coverage-clean-results coverage-compile >> + $(MAKE) CFLAGS="$(COVERAGE_CFLAGS)" LDFLAGS="$(COVERAGE_LDFLAGS)" \ >> + DEFAULT_TEST_TARGET=prove -j1 prove > > First I was wondering why do you need a dedicated 'coverage-prove' > target, instead of letting DEFAULT_TEST_TARGET from the environment or > from 'config.mak' do its thing. But then I noticed in the hunk > context, that, for some reason, the 'coverage-test' target hardcoded > 'DEFAULT_TEST_TARGET=test -j1'. Then I was wondering why would it > want to do that, and stumbled upon commit c14cc77c11: > > coverage: set DEFAULT_TEST_TARGET to avoid using prove > > If the user sets DEFAULT_TEST_TARGET=prove in his config.mak, that > carries over into the coverage tests. Which is really bad if he also > sets GIT_PROVE_OPTS=-j<..> as that completely breaks the coverage > runs. > > Instead of attempting to mess with the GIT_PROVE_OPTS, just force the > test target to 'test' so that we run under make, like we intended all > along. Thanks for finding this! > I'm afraid that this issue would badly affect 'coverage-prove' as well > (I didn't try). Or if doesn't (anymore?), then that should be > mentioned in the commit message, and then perhaps it's time to remove > that '-j1' from the 'coverage-test' target as well. I'll fix this by forcing an update to GIT_PROVE_OPTS. It does limit our ability to use GIT_PROVE_OPTS as a pass-through, but at least this new target will have that assumption built in. Thanks, -Stolee
