Wrap RUNTIME_PREFIX warning in a compile-time condition

From: Johannes Sixt <[email protected]>

When RUNTIME_PREFIX is enabled, the installation prefix is derived by trying a limited set of known locations where the git executable can reside. If none of these is found, a warning is emitted.

When git is built in a directory that matches neither of these known names, the warning would always be emitted when the uninstalled executable is run. This is a problem on Windows, where gitk picks the uninstalled git when invoked on the build directory and gets confused by the warning.

With this patch, individual developers can disable the warning by setting

BASIC_CFLAGS += -DNO_WARN_RUNTIME_PREFIX

in config.mak.

---
 exec_cmd.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

index 408e4e5..5a22635 100644
@@ -28,9 +28,11 @@ const char *system_path(const char *path)
 	    !(prefix = strip_path_suffix(argv0_path, BINDIR)) &&
 	    !(prefix = strip_path_suffix(argv0_path, "git"))) {
 		prefix = PREFIX;
+#ifndef NO_WARN_RUNTIME_PREFIX
 		fprintf(stderr, "RUNTIME_PREFIX requested, "
 				"but prefix computation failed.  "
 				"Using static fallback '%s'.\n", prefix);
+#endif
 	}
 #endif
 

1.7.0.84.g6ef09

Hi,

Would this option not prefer to be a runtime option? An environment variable should suffice.

Ciao,
Dscho

No. The warning is utterly useless IMO and extremely annoying, and the only reason that it still survives is because you disagree. ;-) Since nobody else than I has ever complained about this warning, this patch looks like tailor-made for me and my build-environment (and nobody else will be affected by default), and I don't want to set an environment variable, thank you.

-- Hannes

Hi,

I have been convinced of things before. It just takes a good argument.

Ciao,
Dscho

The good argument is:

With this patch in upstream git, I have more time to spend on testing topics from pu and to write new topics on top of vanilla master because I don't need to apply my private patch all over the place (and back it out before I submit patches).

Whether the option is compile-time or runtime is secondary. The option is *for me*,[*] and I prefer it compile-time. *For you* nothing changes regardless of compile-time or runtime (or do you think you would set the option?).

[*] As I said, nobody else seems to complain.

-- Hannes

Hi,

If it is so annoying (and I agree that it hit me, too), maybe just remove it altogether. It is not really a helpful warning, either.

Maybe the best would be to replace it with a trace_printf().

Ciao,
Dscho

P.S.: I cannot recall defending the warning, but I might have thought it a good idea at some stage. This is no longer so.

From: Johannes Sixt <[email protected]>

When RUNTIME_PREFIX is enabled, the installation prefix is derived by trying a limited set of known locations where the git executable can reside. If none of these is found, a warning is emitted.

When git is built in a directory that matches neither of these known names, the warning would always be emitted when the uninstalled executable is run. This is a problem on Windows, where gitk picks the uninstalled git when invoked from the build directory and gets confused by the warning.

Print the warning only when GIT_TRACE is set.

---

I do recall you defended it; good to know that you changed your mind.

Thanks,
-- Hannes

 exec_cmd.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

index 408e4e5..b2c07c7 100644
@@ -28,7 +28,7 @@ const char *system_path(const char *path)
 	    !(prefix = strip_path_suffix(argv0_path, BINDIR)) &&
 	    !(prefix = strip_path_suffix(argv0_path, "git"))) {
 		prefix = PREFIX;
-		fprintf(stderr, "RUNTIME_PREFIX requested, "
+		trace_printf("RUNTIME_PREFIX requested, "
 				"but prefix computation failed.  "
 				"Using static fallback '%s'.\n", prefix);
 	}

1.7.0.83.g241b9.dirty

Hi,

ACK

And thanks,
Dscho