threads / patch / 22763

patchWrap RUNTIME_PREFIX warning in a compile-time condition

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

## tl;dr

8 messages between Feb 23, 2010 and Feb 23, 2010. Diffs are folded; open one to read it.

replies: 7people: 2as markdown or json

Johannes Sixt· Feb 23, 2010, 08:32 UTC · lore
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(-)
Show changes to exec_cmd.c +2 −0
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
Johannes Schindelin· Feb 23, 2010, 09:42 UTC · re: Johannes Sixt · lore

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

Hi,
On Tue, 23 Feb 2010, Johannes Sixt wrote:
Show 16 quoted lines
> 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

Johannes Sixt· Feb 23, 2010, 09:43 UTC · re: Johannes Schindelin · lore

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

Johannes Schindelin schrieb:
Show 8 quoted lines
> 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
Johannes Schindelin· Feb 23, 2010, 10:03 UTC · re: Johannes Sixt · lore

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

Hi,
On Tue, 23 Feb 2010, Johannes Sixt wrote:
Show 12 quoted lines
> 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

Johannes Sixt· Feb 23, 2010, 10:10 UTC · re: Johannes Schindelin · lore

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

Johannes Schindelin schrieb:
Show 13 quoted lines
> 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
Johannes Schindelin· Feb 23, 2010, 11:07 UTC · re: Johannes Sixt · lore

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

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.

Johannes Sixt· Feb 23, 2010, 11:42 UTC · re: Johannes Schindelin · lore

[PATCH] Print RUNTIME_PREFIX warning only when GIT_TRACE is set

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(-)
Show changes to exec_cmd.c +1 −1
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
Johannes Schindelin· Feb 23, 2010, 12:57 UTC · re: Johannes Sixt · lore

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

Hi,
On Tue, 23 Feb 2010, Johannes Sixt wrote:
Show 14 quoted lines
> 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

← back to recent threads