Re: [PATCH v2 4/4] t/Makefile: introduce TEST_SHELL_PATH

2017-12-21 Thread Jeff King
On Fri, Dec 15, 2017 at 08:58:22AM -0800, Junio C Hamano wrote: > Jeff King writes: > > > I think (4) and (5) are the only things that actually change the > > behavior in a meaningful way. But they're a bit more hacky and > > repetitive than I'd like. Especially given that I'm

Re: [PATCH v2 4/4] t/Makefile: introduce TEST_SHELL_PATH

2017-12-15 Thread Junio C Hamano
Jeff King writes: > I think (4) and (5) are the only things that actually change the > behavior in a meaningful way. But they're a bit more hacky and > repetitive than I'd like. Especially given that I'm not really sure > we're solving an interesting problem. I'm happy enough with

Re: [PATCH v2 4/4] t/Makefile: introduce TEST_SHELL_PATH

2017-12-15 Thread Jeff King
On Mon, Dec 11, 2017 at 12:37:42PM -0800, Junio C Hamano wrote: > > Interestingly, many of those do something like this in the Makefile: > > > > ifdef GIT_TEST_CMP > > @echo GIT_TEST_OPTS=... >>$@+ > > endif > > > > which seems utterly confusing to me. Because it means that if you build >

Re: [PATCH v2 4/4] t/Makefile: introduce TEST_SHELL_PATH

2017-12-11 Thread Junio C Hamano
Jeff King writes: > I'm not sure that's true. Look at what already goes into > GIT-BUILD-OPTIONS: TEST_OUTPUT_DIRECTORY, GIT_TEST_CMP, GIT_PERF_*, etc. > > Interestingly, many of those do something like this in the Makefile: > > ifdef GIT_TEST_CMP > @echo GIT_TEST_OPTS=...

Re: [PATCH v2 4/4] t/Makefile: introduce TEST_SHELL_PATH

2017-12-10 Thread Jeff King
On Sat, Dec 09, 2017 at 02:44:44PM +0100, Johannes Schindelin wrote: > > > ... and we could simply see whether the environment variable > > > TEST_SHELL_PATH (which we would set in t/Makefile from the passed-in > > > SHELL_PATH) is set, and override it again. > > > > > > I still think we can do

Re: [PATCH v2 4/4] t/Makefile: introduce TEST_SHELL_PATH

2017-12-09 Thread Johannes Schindelin
Hi Peff, On Fri, 8 Dec 2017, Jeff King wrote: > On Fri, Dec 08, 2017 at 04:08:19PM +0100, Johannes Schindelin wrote: > > > > Yes, but test-lib.sh sources GIT-BUILD-OPTIONS, which we > > > built during the first "make". And that overrides the > > > environment, giving us the original SHELL_PATH

Re: [PATCH v2 4/4] t/Makefile: introduce TEST_SHELL_PATH

2017-12-08 Thread Jeff King
On Fri, Dec 08, 2017 at 04:08:19PM +0100, Johannes Schindelin wrote: > > Yes, but test-lib.sh sources GIT-BUILD-OPTIONS, which we > > built during the first "make". And that overrides the > > environment, giving us the original SHELL_PATH again. > > ... and we could simply see whether the

Re: [PATCH v2 4/4] t/Makefile: introduce TEST_SHELL_PATH

2017-12-08 Thread Johannes Schindelin
Hi Peff, the other three patches look good to me. On Fri, 8 Dec 2017, Jeff King wrote: > You may want to run the test suite with a different shell > than you use to build Git. For instance, you may build with > SHELL_PATH=/bin/sh (because it's faster, or it's what you > expect to exist on

[PATCH v2 4/4] t/Makefile: introduce TEST_SHELL_PATH

2017-12-08 Thread Jeff King
You may want to run the test suite with a different shell than you use to build Git. For instance, you may build with SHELL_PATH=/bin/sh (because it's faster, or it's what you expect to exist on systems where the build will be used) but want to run the test suite with bash (e.g., since that allows