Re: [PATCH 0/2] Add Windows support to the new RUNTIME_PREFIX design

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Mar 27, 2018, 16:28 UTC
Message-ID: <nycvar.QRO.7.76.6.1803271817060.77@ZVAVAG-6OXH6DA.rhebcr.pbec.zvpebfbsg.pbz>
In-Reply-To: <CAD1RUU9XK837mdRwicMwM5qVApzz8o2e4Eg=B0LH3SRtLqG9WQ@mail.gmail.com>

Hi Dan,

On Tue, 27 Mar 2018, Daniel Jacques wrote:

Show 8 quoted lines

> On Tue, Mar 27, 2018 at 11:54 AM Johannes Schindelin <
> Johannes.Schindelin@gmx.de> wrote:
> 
> > I guess we should add a test where we copy the `git` executable into a
> > subdirectory with the name "git" and call `git/git --exec-path` and
> > verify that its output matches our expectation?
> 
> I'm actually a little fuzzy on the testing model here.

Alright, I'll bite.

You are correct that the test must be contingent on the RUNTIME_PREFIX prerequisite. This could be tested thusly:

	test_lazy_prereq RUNTIME_PREFIX '
		# test whether we built with RUNTIME_PREFIX support
		grep " -DRUNTIME_PREFIX" "$GIT_BUILD_DIR/GIT-CFLAGS"
	'

The subsequent test would run like this:

	test_expect_success RUNTIME_PREFIX '
		mkdir git &&
		cp "$GIT_BUILD_DIR/git$X" git/ &&
		path="$(git/git$X --exec-path)" &&
		case "$(echo "$path" | tr '\\' /)" in
		"$(pwd)/libexec/git-core") ;; # okay
		*)
			echo "Unexpected exec path: $path" >&2
			return 1
			;;
		esac
	'

I say "like this" because it is a little bit tricky to get right, in particular when supporting Windows ;-)

For example, when building with Visual C, the dependencies' .dll files need to be copied into the same directory as the .exe files because there is no good central place to put them (don't get me started on the problems incurred by some software copying some random OpenSSL version's ssleay32.dll into C:\Windows\system32, unless you buy me beer all night and want to be entertained). And that obviously would fail with this approach.

Show 13 quoted lines

> As things are, this test will only work if Git is relocatable; however,
> the test suite doesn't seem to be equipped to build multiple versions of
> Git for different tests.  From this I conclude that the right approach
> would be to make a test that runs conditional on RUNTIME_PREFIX being
> set, but I'm not familiar enough with the testing framework to be
> confident that this is correct, or really how to go about writing such a
> test.
> 
> A simple grep suggests that the current test suite doesn't seem to have any
> RUNTIME_PREFIX-specific tests. When I've been running the test suites, I've
> been doing it with a "config.mak" file that explicitly enables
> RUNTIME_PREFIX to get the runtime prefix code tested against the standard
> Git testing suites.

Indeed, this would be the first test.

>  From a Git maintainer's perspective, would such a test be a
>  prerequisite for landing this patch series, or is this a good candidate
>  for follow-up work to improve our testing coverage?

I cannot speak for Junio, but from my understanding he would probably be fine without such a test. Or a separate patch at a later stage that introduces that.

Or something completely different such as a helper in t/helper/ that always succeeds if RUNTIME_PREFIX is not defined, otherwise passes argv[1] as parameter to git_resolve_executable_dir() and outputs that. Would be a lot more robust than what I described above. But I would want for Duy's test-tool patch series to land first because I would hate to introduce *yet* another stand-alone .exe in t/helper/.

Ciao, Dscho