Re: [PATCH] exec_cmd.c, sideband.c, Makefile: avoid multiple PREFIX definitions

Philip Oakley [off-list ref] writes:

The short and sweet PREFIX can be confused when used in many places.

Rename both usages to better describe their purpose.

Noticed when compiling Git for Windows using MSVC/Visual Studio which
reports the conflict beteeen the command line definition and the
definition in sideband.c

Good eyes.

I am not entirely happy with this name as the name can be easily
confused with GIT_EXEC_PATH.

This is a fallback used under RUNTIME_PREFIX option, if I am reading
the code correctly, so the name should hint the linkage between the
"runtime prefix" mechanism and this variable.

Perhaps RUNTIME_PREFIX_FALLBACK?  Used at only two places, that
should not be an overlong name.

DISPLAY_PREFIX is OK, as it is an entirely a local thing to
sideband.c, but with the externally visible PREFIX fixed to be a
more appropriate name that hints its relation with the "runtime
prefix" mechanism, it may be better not to even touch it.

From: "Junio C Hamano" <redacted>

Philip Oakley [off-list ref] writes:

quoted

The short and sweet PREFIX can be confused when used in many places.

Rename both usages to better describe their purpose.

Noticed when compiling Git for Windows using MSVC/Visual Studio which
reports the conflict beteeen the command line definition and the
definition in sideband.c

Good eyes.

quoted

Signed-off-by: Philip Oakley <redacted>
---
 Makefile   | 2 +-
 exec_cmd.c | 4 ++--
 sideband.c | 6 +++---
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/Makefile b/Makefile
index 33b0f76..bcdd3ec 100644
--- a/Makefile
+++ b/Makefile

@@ -1973,7 +1973,7 @@ exec_cmd.sp exec_cmd.s exec_cmd.o: GIT-PREFIX
 exec_cmd.sp exec_cmd.s exec_cmd.o: EXTRA_CPPFLAGS = \
 '-DGIT_EXEC_PATH="$(gitexecdir_SQ)"' \
 '-DBINDIR="$(bindir_relative_SQ)"' \
- '-DPREFIX="$(prefix_SQ)"'
+ '-DEXEC_PREFIX="$(prefix_SQ)"'

I am not entirely happy with this name as the name can be easily
confused with GIT_EXEC_PATH.

This is a fallback used under RUNTIME_PREFIX option, if I am reading
the code correctly, so the name should hint the linkage between the
"runtime prefix" mechanism and this variable.

Perhaps RUNTIME_PREFIX_FALLBACK?  Used at only two places, that
should not be an overlong name.

Perhaps EXEC_CMD_PREFIX, for that is what it is? And it's got a 2-way 
difference from the GIT_EXEC_PATH. I wasn't seeing it as a fallback, rather 
just a different way the OS uses to get to the path. If anything 
RUNTIME_PREFIX feels a bit long for a flag.

DISPLAY_PREFIX is OK, as it is an entirely a local thing to
sideband.c, but with the externally visible PREFIX fixed to be a
more appropriate name that hints its relation with the "runtime
prefix" mechanism, it may be better not to even touch it.

quoted

 builtin/init-db.sp builtin/init-db.s builtin/init-db.o: GIT-PREFIX
 builtin/init-db.sp builtin/init-db.s builtin/init-db.o: EXTRA_CPPFLAGS = 
\

diff --git a/exec_cmd.c b/exec_cmd.c
index 9d5703a..2a79781 100644
--- a/exec_cmd.c
+++ b/exec_cmd.c

@@ -12,7 +12,7 @@ char *system_path(const char *path)
 #ifdef RUNTIME_PREFIX
 static const char *prefix;
 #else
- static const char *prefix = PREFIX;
+ static const char *prefix = EXEC_PREFIX;
 #endif
 struct strbuf d = STRBUF_INIT;

@@ -27,7 +27,7 @@ char *system_path(const char *path)
     !(prefix = strip_path_suffix(argv0_path, GIT_EXEC_PATH)) &&
     !(prefix = strip_path_suffix(argv0_path, BINDIR)) &&
     !(prefix = strip_path_suffix(argv0_path, "git"))) {
- prefix = PREFIX;
+ prefix = EXEC_PREFIX;
 trace_printf("RUNTIME_PREFIX requested, "
 "but prefix computation failed.  "
 "Using static fallback '%s'.\n", prefix);