Re: [PATCH v6 3/3] exec_cmd: RUNTIME_PREFIX on some POSIX systems

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Mar 19, 2018, 19:27 UTC
Message-ID: <87woy7c1cf.fsf@evledraar.gmail.com>
In-Reply-To: <20180319025046.58052-4-dnj@google.com>

On Mon, Mar 19 2018, Dan Jacques jotted:

Show 16 quoted lines

>  #
>  # Define HAVE_BSD_SYSCTL if your platform has a BSD-compatible sysctl function.
>  #
> +# Define HAVE_BSD_KERN_PROC_SYSCTL if your platform supports the KERN_PROC BSD
> +# sysctl function.
> +#
> +# Define PROCFS_EXECUTABLE_PATH if your platform mounts a "procfs" filesystem
> +# capable of resolving the path of the current executable. If defined, this
> +# must be the canonical path for the "procfs" current executable path.
> +#
> +# Define HAVE_NS_GET_EXECUTABLE_PATH if your platform supports calling
> +# _NSGetExecutablePath to retrieve the path of the running executable.
> +#
>  # Define HAVE_GETDELIM if your system has the getdelim() function.
>  #
>  # Define PAGER_ENV to a SP separated VAR=VAL pairs to define

This is fine in isolation, but the sum total of the series ends up being:

    diff --git a/Makefile b/Makefile
    index 96f6138f63..c23d4d10f0 100644
    --- a/Makefile
    +++ b/Makefile
    @@ -425,6 +425,16 @@ all::
     #
     # Define HAVE_BSD_SYSCTL if your platform has a BSD-compatible sysctl function.
     #
    +# Define HAVE_BSD_KERN_PROC_SYSCTL if your platform supports the KERN_PROC BSD
    +# sysctl function.
    +#
    +# Define PROCFS_EXECUTABLE_PATH if your platform mounts a "procfs" filesystem
    +# capable of resolving the path of the current executable. If defined, this
    +# must be the canonical path for the "procfs" current executable path.
    +#
    +# Define HAVE_NS_GET_EXECUTABLE_PATH if your platform supports calling
    +# _NSGetExecutablePath to retrieve the path of the running executable.
    +#
     # Define HAVE_GETDELIM if your system has the getdelim() function.
     #
     # Define PAGER_ENV to a SP separated VAR=VAL pairs to define
    @@ -441,6 +451,13 @@ all::
     #
     # When cross-compiling, define HOST_CPU as the canonical name of the CPU on
     # which the built Git will run (for instance "x86_64").
    +#
    +# Define RUNTIME_PREFIX to configure Git to resolve its ancillary tooling and
    +# support files relative to the location of the runtime binary, rather than
    +# hard-coding them into the binary. Git installations built with RUNTIME_PREFIX
    +# can be moved to arbitrary filesystem locations. RUNTIME_PREFIX also causes
    +# Perl scripts to use a modified entry point header allowing them to resolve
    +# support files at runtime.

I wonder if it wouldn't be a lot more understandable if these were noted together, i.e. let's first document RUNTIME_PREFIX, then for all the other ones say below that:

   # When using RUNTIME_PREFIX, define HAVE_BSD[...]

Or something like that. We can always drop the "When using RUNTIME_PREFIX, " bit later if it ends up benig used for other stuff, but for now it's helpful to note that you don't need to care about these if you're not using RUNTIME_PREFIX.

Show 5 quoted lines

> -				"but prefix computation failed.  "
> -				"Using static fallback '%s'.\n", prefix);
> +			     "but prefix computation failed.  "
> +			     "Using static fallback '%s'.\n",
> +			     prefix);

Whitespace changed mixed in with the actual change.