[PATCH] Wrap RUNTIME_PREFIX warning in a compile-time condition

[PATCH] Wrap RUNTIME_PREFIX warning in a compile-time condition

[Johannes Sixt]

From: Johannes Sixt <j6t@kdbg.org>

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.

Signed-off-by: Johannes Sixt <j6t@kdbg.org>
---
 exec_cmd.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/exec_cmd.c b/exec_cmd.c
index 408e4e5..5a22635 100644
--- a/exec_cmd.c
+++ b/exec_cmd.c
@@ -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

Re: [PATCH] Wrap RUNTIME_PREFIX warning in a compile-time condition

[Johannes Schindelin]

Hi,

On Tue, 23 Feb 2010, Johannes Sixt wrote:

> From: Johannes Sixt <j6t@kdbg.org>
> 
> 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.

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

Ciao,
Dscho

Re: [PATCH] Wrap RUNTIME_PREFIX warning in a compile-time condition

[Johannes Sixt]

Johannes Schindelin schrieb:
> On Tue, 23 Feb 2010, Johannes Sixt wrote:
>> With this patch, individual developers can disable the warning by setting
>>
>>    BASIC_CFLAGS += -DNO_WARN_RUNTIME_PREFIX
>>
>> in config.mak.
> 
> Would this option not prefer to be a runtime option?

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

Re: [PATCH] Wrap RUNTIME_PREFIX warning in a compile-time condition

[Johannes Schindelin]

Hi,

On Tue, 23 Feb 2010, Johannes Sixt wrote:

> Johannes Schindelin schrieb:
> > On Tue, 23 Feb 2010, Johannes Sixt wrote:
> >> With this patch, individual developers can disable the warning by setting
> >>
> >>    BASIC_CFLAGS += -DNO_WARN_RUNTIME_PREFIX
> >>
> >> in config.mak.
> > 
> > Would this option not prefer to be a runtime option?
> 
> No. The warning is utterly useless IMO and extremely annoying, and the
> only reason that it still survives is because you disagree. ;-)

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

Ciao,
Dscho

Re: [PATCH] Wrap RUNTIME_PREFIX warning in a compile-time condition

[Johannes Sixt]

Johannes Schindelin schrieb:
> On Tue, 23 Feb 2010, Johannes Sixt wrote:
>> Johannes Schindelin schrieb:
>>> On Tue, 23 Feb 2010, Johannes Sixt wrote:
>>>> With this patch, individual developers can disable the warning by setting
>>>>
>>>>    BASIC_CFLAGS += -DNO_WARN_RUNTIME_PREFIX
>>>>
>>>> in config.mak.
>>> Would this option not prefer to be a runtime option?
>> No. The warning is utterly useless IMO and extremely annoying, and the
>> only reason that it still survives is because you disagree. ;-)
> 
> I have been convinced of things before. It just takes a good argument.

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

Re: [PATCH] Wrap RUNTIME_PREFIX warning in a compile-time condition

[Johannes Schindelin]

Hi,

On Tue, 23 Feb 2010, Johannes Sixt wrote:

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

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.

[PATCH] Print RUNTIME_PREFIX warning only when GIT_TRACE is set

[Johannes Sixt]

From: Johannes Sixt <j6t@kdbg.org>

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.

Signed-off-by: Johannes Sixt <j6t@kdbg.org>
---
Johannes Schindelin schrieb:
> 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.

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(-)

diff --git a/exec_cmd.c b/exec_cmd.c
index 408e4e5..b2c07c7 100644
--- a/exec_cmd.c
+++ b/exec_cmd.c
@@ -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

Re: [PATCH] Print RUNTIME_PREFIX warning only when GIT_TRACE is set

[Johannes Schindelin]

Hi,

On Tue, 23 Feb 2010, Johannes Sixt wrote:

> From: Johannes Sixt <j6t@kdbg.org>
> 
> 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.
> 
> Signed-off-by: Johannes Sixt <j6t@kdbg.org>

ACK

And thanks,
Dscho