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

From

Johannes Schindelin <johannes.schindelin@gmx.de>

Date

Mar 29, 2018, 14:54 UTC

Message-ID

<nycvar.QRO.7.76.6.1803291637050.5026@qfpub.tvgsbejvaqbjf.bet>

In-Reply-To

<xmqqa7us86pi.fsf@gitster-ct.c.googlers.com>

Hi Junio,

On Wed, 28 Mar 2018, Junio C Hamano wrote:

> Daniel Jacques <dnj@google.com> writes:
> 
> > 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.
> >
> > 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?
> 
> It would be a nice-to-have follow-up, I would say, but as you two
> seem to be working well together and it shouldn't be too involved to
> have the minimum test that makes sure the version of "git" being
> tested thinks things should be where we think they should be, with
> something like...
> 
> 	test_expect_success RUNTIME_PREFIX 'runtime-prefix basics' '
> 		(
> 			# maybe others
> 			safe_unset GIT_EXEC_PATH &&
> 			git --exec-path >actual

I initially thought that we could somehow do this:

-- snip --

diff --git a/t/helper/test-path-utils.c b/t/helper/test-path-utils.c
index 2b3c5092a19..3040f0dae49 100644
--- a/t/helper/test-path-utils.c
+++ b/t/helper/test-path-utils.c
@@ -1,5 +1,6 @@
 #include "cache.h"
 #include "string-list.h"
+#include "exec_cmd.h"
 
 /*
  * A "string_list_each_func_t" function that normalizes an entry from
@@ -270,6 +271,25 @@ int cmd_main(int argc, const char **argv)
 	if (argc == 2 && !strcmp(argv[1], "dirname"))
 		return test_function(dirname_data, posix_dirname,
argv[1]);
 
+	if (argc == 3 && !strcmp(argv[1], "runtime-prefix")) {
+#ifndef RUNTIME_PREFIX
+		warning("RUNTIME_PREFIX support not compiled in;
skipping");
+		return 0;
+#else
+		char *path;
+
+		git_resolve_executable_dir(argv[2]);
+		path = system_path("");
+
+		if (!starts_with(argv[2], path))
+			return error("unexpected prefix: '%s'", path);
+
+		puts(path);
+
+		return 0;
+#endif
+	}
+
 	fprintf(stderr, "%s: unknown function name: %s\n", argv[0],
 		argv[1] ? argv[1] : "(there was none)");
 	return 1;
```

but this simply won't work, as the main idea of
`git_resolve_executable_dir()` is to use the executable path whenever
possible, instead of the passed-in parameter.

And since we usually work via the bin-wrappers, we cannot even add a
sanity check that Git was cloned into a directory called "git"...

So... I think we have to leave this out of the patch series, unless
somebody comes up with an idea neither Dan nor I has thought about to test
this reliably *without* copying the Git executable (which would, as I
mentioned, break testing when .dll files need to be present in the same
directory as git.exe).

Ciao,
Dscho