Re: [PATCH] exec_cmd.c, sideband.c, Makefile: avoid multiple PREFIX definitions
- From
Junio C Hamano <gitster@pobox.com>
- Date
- May 5, 2016, 22:02 UTC
- Message-ID
- <xmqqk2j89n9y.fsf@gitster.mtv.corp.google.com>
- In-Reply-To
- <1462483733-3496-2-git-send-email-philipoakley@iee.org>
Philip Oakley <philipoakley@iee.org> writes:
Show 7 quoted lines
> 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.
Show 18 quoted lines
> > Signed-off-by: Philip Oakley <philipoakley@iee.org> > --- > 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.
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.
Show 24 quoted lines
> 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);