Re: [PATCH 3/3] var: add config file locations

[Eric Sunshine <sunshine@sunshineco.com>]

On Thu, Jun 22, 2023 at 4:06 PM brian m. carlson
<sandals@crustytoothpaste.net> wrote:
> Much like with attributes files, sometimes programs would like to know
> the location of configuration files at the global or system levels.
> However, it isn't always clear where these may live, especially for the
> system file, which may have been hard-coded at compile time or computed
> dynamically based on the runtime prefix.
>
> Since other parties cannot intuitively know how Git was compiled and
> where it looks for these files, help them by providing variables that
> can be queried.  Because we have multiple paths for global config
> values, print them in order from highest to lowest priority, and be sure
> to split on newlines so that "git var -l" produces two entries for the
> global value.
>
> However, be careful not to split all values on newlines, since our
> editor values could well contain such characters, and we don't want to
> split them in such a case.
>
> Note in the documentation that some values may contain multiple paths
> and that callers should be prepared for that fact.  This helps people
> write code that will continue to work in the event we allow multiple
> items elsewhere in the future.
>
> Signed-off-by: brian m. carlson <bk2204@github.com>
> ---
> diff --git a/builtin/var.c b/builtin/var.c
> @@ -62,21 +62,59 @@ static const char *git_attr_val_global(int flag)
>  struct git_var {
>         const char *name;
>         const char *(*read)(int);
> +       int multivalued;
>         int free;
>  };
>  static struct git_var git_vars[] = {
> +       { "GIT_COMMITTER_IDENT", git_committer_info, 0, 0 },
> +       { "GIT_AUTHOR_IDENT",   git_author_info, 0, 0 },
> +       { "GIT_EDITOR", editor, 0, 0 },
> +       { "GIT_SEQUENCE_EDITOR", sequence_editor, 0, 0 },
> +       { "GIT_PAGER", pager, 0, 0 },
> +       { "GIT_DEFAULT_BRANCH", default_branch, 0, 9 },

Why "9"?

> +       { "GIT_SHELL_PATH", shell_path, 0, 0 },
> +       { "GIT_ATTR_SYSTEM", git_attr_val_system, 0, 1 },
> +       { "GIT_ATTR_GLOBAL", git_attr_val_global, 0, 1 },
> +       { "GIT_CONFIG_SYSTEM", git_config_val_system, 0, 1 },
> +       { "GIT_CONFIG_GLOBAL", git_config_val_global, 1, 1 },
>         { "", NULL },
>  };
> diff --git a/t/t0007-git-var.sh b/t/t0007-git-var.sh
> @@ -179,6 +179,49 @@ test_expect_success 'GIT_ATTR_GLOBAL points to the correct location' '
> +test_expect_success 'GIT_CONFIG_SYSTEM points to the correct location' '
> +       TRASHDIR="$(test-tool path-utils normalize_path_copy "$(pwd)")" &&

Same comment as in [2/3]: $(pwd) is unnecessary. Simpler:

    TRASHDIR="$(test-tool path-utils normalize_path_copy .)" &&

> +       test_must_fail env GIT_CONFIG_NOSYSTEM=1 git var GIT_CONFIG_SYSTEM &&
> +       (
> +               sane_unset GIT_CONFIG_NOSYSTEM &&
> +               git var GIT_CONFIG_SYSTEM >path &&
> +               test "$(cat path)" != "" &&
> +               GIT_CONFIG_SYSTEM=/dev/null git var GIT_CONFIG_SYSTEM >path &&
> +               if test_have_prereq MINGW
> +               then
> +                       test "$(cat path)" = "nul"
> +               else
> +                       test "$(cat path)" = "/dev/null"
> +               fi &&
> +               GIT_CONFIG_SYSTEM="$TRASHDIR/gitconfig" git var GIT_CONFIG_SYSTEM >path &&
> +               test "$(cat path)" = "$TRASHDIR/gitconfig"
> +       )
> +'

Ditto regarding unnecessary temporary file.

> +test_expect_success 'GIT_CONFIG_GLOBAL points to the correct location' '
> +       TRASHDIR="$(test-tool path-utils normalize_path_copy "$(pwd)")" &&

Ditto regarding $(pwd).

> @@ -196,6 +239,29 @@ test_expect_success 'git var -l lists config' '
> +test_expect_success 'git var -l does not split multiline editors' '
> +       (
> +               GIT_EDITOR="!f() {
> +                       echo Hello!
> +               }; f" &&
> +               export GIT_EDITOR &&
> +               echo "GIT_EDITOR=$GIT_EDITOR" >expected &&
> +               git var -l >var &&
> +               cat var &&

Is this `cat` leftover debugging code?

> +               sed -n -e "/^GIT_EDITOR/,\$p" var | head -n 3 >actual &&
> +               test_cmp expected actual
> +       )
> +'