Hi Junio,
On Wed, 28 Mar 2018, Junio C Hamano wrote:
> Daniel Jacques <d...@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
That will only work when the directory into which git (or git.exe) was
compiled is called "bin" or "git" (or "git-core" in a "libexec"
directory), because this is the sanity check we have to determine that Git
is installed into a sensible location where we *can* assume that
libexec/git-core/ is the corresponding location of the support
executables/scripts.
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
