{"thread":{"id":"59899","subject":"[PATCH 0/3] Additional variables for git var","startedAt":"2023-06-22T19:51:39Z","lastAt":"2023-06-27T20:18:10Z","messageCount":42,"participants":["brian m. carlson","Derrick Stolee","Eric Sunshine","Junio C Hamano","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"478696","messageId":"20230622195059.320593-1-sandals@crustytoothpaste.net","threadId":"59899","inReplyTo":null,"subject":"[PATCH 0/3] Additional variables for git var","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2023-06-22T19:50:56Z","receivedAt":"2023-06-22T19:51:39Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On many Unix systems, we have a good idea where Git's configuration\nfiles and the shell it uses are located. However, there are some\nsystems where that's not the case, such as Windows and with Homebrew,\nwhere the expected files might be found in another location.\n\nRight now, programs who would like to access things like the system\ngitattributes or config file have to guess where the distributor of Git\nplaced these files, and with runtime relocation, it's not even\nguaranteed that these will be in a fixed place from invocation to\ninvocation. As a result, we need a way to query Git about the location\nof these files.\n\nThis series introduces five new configuration variables that refer to\nthe shell path, the system and global gitattributes files, and the\nsystem and global config files. The global values are not technically\nneeded, since they should be computable from the environment alone, but\nthey are added to make life easier for users.\n\nThe shell path is especially useful on Windows, where Git usually\nprovides the POSIX shell, and is designed to make using programs that\ninteract with Git or Unix-like environment variables (e.g., `EDITOR` and\n`VISUAL`) easier.\n\nThe gitattributes files are primarily for the benefit of Git LFS, but of\ncourse the goal is to be generally useful.\n\nThe curious reviewer may ask, \"Why are these in `git var` and not `git\nrev-parse`?\" The answer is that these refer to configuration (albeit\npartially at compile time), and thus are like the existing GIT_EDITOR,\nand unlike what `git rev-parse` produces, which is almost completely\nlimited to paths and metadata related specifically to a given\nrepository.\n\nThis should be a relatively straightforward series, but of course any\nfeedback is welcome.\n\nbrian m. carlson (3):\n var: add support for listing the shell\n var: add attributes files locations\n var: add config file locations\n\n Documentation/git-var.txt | 23 +++++++++\n attr.c | 6 +--\n attr.h | 4 ++\n builtin/var.c | 99 +++++++++++++++++++++++++++++++++++----\n t/t0007-git-var.sh | 98 ++++++++++++++++++++++++++++++++++++++\n 5 files changed, 219 insertions(+), 11 deletions(-)\n\n"},{"id":"478697","messageId":"20230622195059.320593-2-sandals@crustytoothpaste.net","threadId":"59899","inReplyTo":"20230622195059.320593-1-sandals@crustytoothpaste.net","subject":"[PATCH 1/3] var: add support for listing the shell","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2023-06-22T19:50:57Z","receivedAt":"2023-06-22T19:51:39Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"From: \"brian m. carlson\" \n\nOn most Unix systems, finding a suitable shell is easy: one simply uses\n\"sh\" with an appropriate PATH value. However, in many Windows\nenvironments, the shell is shipped alongside Git, and it may or may not\nbe in PATH, even if Git is.\n\nIn such an environment, it can be very helpful to query Git for the\nshell it's using, since other tools may want to use the same shell as\nwell. To help them out, let's add a variable, GIT_SHELL_PATH, that\npoints to the location of the shell.\n\nOn Unix, we know our shell must be executable to be functional, so\nassume that the distributor has correctly configured their environment,\nand use that as a basic test. On Git for Windows, we know that our\nshell will be one of a few fixed values, all of which end in \"sh\" (such\nas \"bash\"). This seems like it might be a nice test on Unix as well,\nsince it is customary for all shells to end in \"sh\", but there probably\nexist such systems that don't have such a configuration, so be careful\nhere not to break them.\n\nSigned-off-by: brian m. carlson \n---\n Documentation/git-var.txt | 3 +++\n builtin/var.c | 6 ++++++\n t/t0007-git-var.sh | 12 ++++++++++++\n 3 files changed, 21 insertions(+)\n\ndiff --git a/Documentation/git-var.txt b/Documentation/git-var.txt\nindex f40202b8e3..f0f647e14a 100644\n--- a/Documentation/git-var.txt\n+++ b/Documentation/git-var.txt\n@@ -71,6 +71,9 @@ endif::git-default-pager[]\n GIT_DEFAULT_BRANCH::\n The name of the first branch created in newly initialized repositories.\n \n+GIT_SHELL_PATH::\n+ The path of the binary providing the POSIX shell for commands which use the shell.\n+\n SEE ALSO\n --------\n linkgit:git-commit-tree[1]\ndiff --git a/builtin/var.c b/builtin/var.c\nindex 2149998980..f97178eed1 100644\n--- a/builtin/var.c\n+++ b/builtin/var.c\n@@ -36,6 +36,11 @@ static const char *default_branch(int flag)\n \treturn git_default_branch_name(1);\n }\n \n+static const char *shell_path(int flag)\n+{\n+\treturn SHELL_PATH;\n+}\n+\n struct git_var {\n \tconst char *name;\n \tconst char *(*read)(int);\n@@ -47,6 +52,7 @@ static struct git_var git_vars[] = {\n \t{ \"GIT_SEQUENCE_EDITOR\", sequence_editor },\n \t{ \"GIT_PAGER\", pager },\n \t{ \"GIT_DEFAULT_BRANCH\", default_branch },\n+\t{ \"GIT_SHELL_PATH\", shell_path },\n \t{ \"\", NULL },\n };\n \ndiff --git a/t/t0007-git-var.sh b/t/t0007-git-var.sh\nindex eeb8539c1b..270bd4e512 100755\n--- a/t/t0007-git-var.sh\n+++ b/t/t0007-git-var.sh\n@@ -147,6 +147,18 @@ test_expect_success 'get GIT_SEQUENCE_EDITOR with configuration and environment\n \t)\n '\n \n+test_expect_success POSIXPERM 'GIT_SHELL_PATH points to a valid executable' '\n+\tgit var GIT_SHELL_PATH >shell &&\n+\ttest -x \"$(cat shell)\"\n+'\n+\n+# We know in this environment that our shell will be one of a few fixed values\n+# that all end in \"sh\".\n+test_expect_success MINGW 'GIT_SHELL_PATH points to a suitable shell' '\n+\tgit var GIT_SHELL_PATH >shell &&\n+\tgrep \"sh\\$\" shell\n+'\n+\n # For git var -l, we check only a representative variable;\n # testing the whole output would make our test too brittle with\n # respect to unrelated changes in the test suite's environment.\n"},{"id":"478698","messageId":"20230622195059.320593-4-sandals@crustytoothpaste.net","threadId":"59899","inReplyTo":"20230622195059.320593-1-sandals@crustytoothpaste.net","subject":"[PATCH 3/3] var: add config file locations","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2023-06-22T19:50:59Z","receivedAt":"2023-06-22T19:51:39Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"From: \"brian m. carlson\" \n\nMuch like with attributes files, sometimes programs would like to know\nthe location of configuration files at the global or system levels.\nHowever, it isn't always clear where these may live, especially for the\nsystem file, which may have been hard-coded at compile time or computed\ndynamically based on the runtime prefix.\n\nSince other parties cannot intuitively know how Git was compiled and\nwhere it looks for these files, help them by providing variables that\ncan be queried. Because we have multiple paths for global config\nvalues, print them in order from highest to lowest priority, and be sure\nto split on newlines so that \"git var -l\" produces two entries for the\nglobal value.\n\nHowever, be careful not to split all values on newlines, since our\neditor values could well contain such characters, and we don't want to\nsplit them in such a case.\n\nNote in the documentation that some values may contain multiple paths\nand that callers should be prepared for that fact. This helps people\nwrite code that will continue to work in the event we allow multiple\nitems elsewhere in the future.\n\nSigned-off-by: brian m. carlson \n---\n Documentation/git-var.txt | 14 ++++++++\n builtin/var.c | 68 +++++++++++++++++++++++++++++++++------\n t/t0007-git-var.sh | 66 +++++++++++++++++++++++++++++++++++++\n 3 files changed, 138 insertions(+), 10 deletions(-)\n\ndiff --git a/Documentation/git-var.txt b/Documentation/git-var.txt\nindex dfbe5cb52b..c38fb3968b 100644\n--- a/Documentation/git-var.txt\n+++ b/Documentation/git-var.txt\n@@ -80,6 +80,20 @@ GIT_ATTR_SYSTEM::\n GIT_ATTR_GLOBAL::\n The path to the global (per-user) linkgit:gitattributes[5] file.\n \n+GIT_CONFIG_SYSTEM::\n+ The path to the system configuration file, if one is enabled.\n+\n+GIT_CONFIG_GLOBAL::\n+ The path to the global (per-user) configuration files, if any.\n+\n+Most path values contain only one value. However, some can contain multiple\n+values, which are separated by newlines, and are listed in order from highest to\n+lowest priority. Callers should be prepared for any such path value to contain\n+multiple items.\n+\n+Note that paths are printed even if they do not exist, but not if they are\n+disabled by other environment variables.\n+\n SEE ALSO\n --------\n linkgit:git-commit-tree[1]\ndiff --git a/builtin/var.c b/builtin/var.c\nindex b9e2f23697..8a47b74777 100644\n--- a/builtin/var.c\n+++ b/builtin/var.c\n@@ -62,21 +62,59 @@ static const char *git_attr_val_global(int flag)\n \treturn NULL;\n }\n \n+static const char *git_config_val_system(int flag)\n+{\n+\tif (git_config_system()) {\n+\t\tchar *file = git_system_config();\n+\t\tnormalize_path_copy(file, file);\n+\t\treturn file;\n+\t}\n+\treturn NULL;\n+}\n+\n+static const char *git_config_val_global(int flag)\n+{\n+\tstruct strbuf buf = STRBUF_INIT;\n+\tchar *user, *xdg;\n+\tsize_t unused;\n+\n+\tgit_global_config(&user, &xdg);\n+\tif (xdg && *xdg) {\n+\t\tnormalize_path_copy(xdg, xdg);\n+\t\tstrbuf_addf(&buf, \"%s\\n\", xdg);\n+\t}\n+\tif (user && *user) {\n+\t\tnormalize_path_copy(user, user);\n+\t\tstrbuf_addf(&buf, \"%s\\n\", user);\n+\t}\n+\tfree(xdg);\n+\tfree(user);\n+\tstrbuf_trim_trailing_newline(&buf);\n+\tif (buf.len == 0) {\n+\t\tstrbuf_release(&buf);\n+\t\treturn NULL;\n+\t}\n+\treturn strbuf_detach(&buf, &unused);\n+}\n+\n struct git_var {\n \tconst char *name;\n \tconst char *(*read)(int);\n+\tint multivalued;\n \tint free;\n };\n static struct git_var git_vars[] = {\n-\t{ \"GIT_COMMITTER_IDENT\", git_committer_info, 0 },\n-\t{ \"GIT_AUTHOR_IDENT\", git_author_info, 0 },\n-\t{ \"GIT_EDITOR\", editor, 0 },\n-\t{ \"GIT_SEQUENCE_EDITOR\", sequence_editor, 0 },\n-\t{ \"GIT_PAGER\", pager, 0 },\n-\t{ \"GIT_DEFAULT_BRANCH\", default_branch, 0 },\n-\t{ \"GIT_SHELL_PATH\", shell_path, 0 },\n-\t{ \"GIT_ATTR_SYSTEM\", git_attr_val_system, 1 },\n-\t{ \"GIT_ATTR_GLOBAL\", git_attr_val_global, 1 },\n+\t{ \"GIT_COMMITTER_IDENT\", git_committer_info, 0, 0 },\n+\t{ \"GIT_AUTHOR_IDENT\", git_author_info, 0, 0 },\n+\t{ \"GIT_EDITOR\", editor, 0, 0 },\n+\t{ \"GIT_SEQUENCE_EDITOR\", sequence_editor, 0, 0 },\n+\t{ \"GIT_PAGER\", pager, 0, 0 },\n+\t{ \"GIT_DEFAULT_BRANCH\", default_branch, 0, 9 },\n+\t{ \"GIT_SHELL_PATH\", shell_path, 0, 0 },\n+\t{ \"GIT_ATTR_SYSTEM\", git_attr_val_system, 0, 1 },\n+\t{ \"GIT_ATTR_GLOBAL\", git_attr_val_global, 0, 1 },\n+\t{ \"GIT_CONFIG_SYSTEM\", git_config_val_system, 0, 1 },\n+\t{ \"GIT_CONFIG_GLOBAL\", git_config_val_global, 1, 1 },\n \t{ \"\", NULL },\n };\n \n@@ -87,7 +125,17 @@ static void list_vars(void)\n \n \tfor (ptr = git_vars; ptr->read; ptr++)\n \t\tif ((val = ptr->read(0))) {\n-\t\t\tprintf(\"%s=%s\\n\", ptr->name, val);\n+\t\t\tif (ptr->multivalued && *val) {\n+\t\t\t\tstruct string_list list = STRING_LIST_INIT_DUP;\n+\t\t\t\tint i;\n+\n+\t\t\t\tstring_list_split(&list, val, '\\n', -1);\n+\t\t\t\tfor (i = 0; i < list.nr; i++)\n+\t\t\t\t\tprintf(\"%s=%s\\n\", ptr->name, list.items[i].string);\n+\t\t\t\tstring_list_clear(&list, 0);\n+\t\t\t} else {\n+\t\t\t\tprintf(\"%s=%s\\n\", ptr->name, val);\n+\t\t\t}\n \t\t\tif (ptr->free)\n \t\t\t\tfree((void *)val);\n \t\t}\ndiff --git a/t/t0007-git-var.sh b/t/t0007-git-var.sh\nindex 6a2cc94abb..d519c2f441 100755\n--- a/t/t0007-git-var.sh\n+++ b/t/t0007-git-var.sh\n@@ -179,6 +179,49 @@ test_expect_success 'GIT_ATTR_GLOBAL points to the correct location' '\n \t)\n '\n \n+test_expect_success 'GIT_CONFIG_SYSTEM points to the correct location' '\n+\tTRASHDIR=\"$(test-tool path-utils normalize_path_copy \"$(pwd)\")\" &&\n+\ttest_must_fail env GIT_CONFIG_NOSYSTEM=1 git var GIT_CONFIG_SYSTEM &&\n+\t(\n+\t\tsane_unset GIT_CONFIG_NOSYSTEM &&\n+\t\tgit var GIT_CONFIG_SYSTEM >path &&\n+\t\ttest \"$(cat path)\" != \"\" &&\n+\t\tGIT_CONFIG_SYSTEM=/dev/null git var GIT_CONFIG_SYSTEM >path &&\n+\t\tif test_have_prereq MINGW\n+\t\tthen\n+\t\t\ttest \"$(cat path)\" = \"nul\"\n+\t\telse\n+\t\t\ttest \"$(cat path)\" = \"/dev/null\"\n+\t\tfi &&\n+\t\tGIT_CONFIG_SYSTEM=\"$TRASHDIR/gitconfig\" git var GIT_CONFIG_SYSTEM >path &&\n+\t\ttest \"$(cat path)\" = \"$TRASHDIR/gitconfig\"\n+\t)\n+'\n+\n+test_expect_success 'GIT_CONFIG_GLOBAL points to the correct location' '\n+\tTRASHDIR=\"$(test-tool path-utils normalize_path_copy \"$(pwd)\")\" &&\n+\tHOME=\"$TRASHDIR\" XDG_CONFIG_HOME=\"$TRASHDIR/foo\" git var GIT_CONFIG_GLOBAL >actual &&\n+\techo \"$TRASHDIR/foo/git/config\" >expected &&\n+\techo \"$TRASHDIR/.gitconfig\" >>expected &&\n+\ttest_cmp expected actual &&\n+\t(\n+\t\tsane_unset XDG_CONFIG_HOME &&\n+\t\tHOME=\"$TRASHDIR\" git var GIT_CONFIG_GLOBAL >actual &&\n+\t\techo \"$TRASHDIR/.config/git/config\" >expected &&\n+\t\techo \"$TRASHDIR/.gitconfig\" >>expected &&\n+\t\ttest_cmp expected actual &&\n+\t\tGIT_CONFIG_GLOBAL=/dev/null git var GIT_CONFIG_GLOBAL >path &&\n+\t\tif test_have_prereq MINGW\n+\t\tthen\n+\t\t\ttest \"$(cat path)\" = \"nul\"\n+\t\telse\n+\t\t\ttest \"$(cat path)\" = \"/dev/null\"\n+\t\tfi &&\n+\t\tGIT_CONFIG_GLOBAL=\"$TRASHDIR/gitconfig\" git var GIT_CONFIG_GLOBAL >path &&\n+\t\ttest \"$(cat path)\" = \"$TRASHDIR/gitconfig\"\n+\t)\n+'\n+\n # For git var -l, we check only a representative variable;\n # testing the whole output would make our test too brittle with\n # respect to unrelated changes in the test suite's environment.\n@@ -196,6 +239,29 @@ test_expect_success 'git var -l lists config' '\n \ttest_cmp expect actual.bare\n '\n \n+test_expect_success 'git var -l lists multiple global configs' '\n+\tTRASHDIR=\"$(test-tool path-utils normalize_path_copy \"$(pwd)\")\" &&\n+\tHOME=\"$TRASHDIR\" XDG_CONFIG_HOME=\"$TRASHDIR/foo\" git var -l >actual &&\n+\tgrep \"^GIT_CONFIG_GLOBAL=\" actual >filtered &&\n+\techo \"GIT_CONFIG_GLOBAL=$TRASHDIR/foo/git/config\" >expected &&\n+\techo \"GIT_CONFIG_GLOBAL=$TRASHDIR/.gitconfig\" >>expected &&\n+\ttest_cmp expected filtered\n+'\n+\n+test_expect_success 'git var -l does not split multiline editors' '\n+\t(\n+\t\tGIT_EDITOR=\"!f() {\n+\t\t\techo Hello!\n+\t\t}; f\" &&\n+\t\texport GIT_EDITOR &&\n+\t\techo \"GIT_EDITOR=$GIT_EDITOR\" >expected &&\n+\t\tgit var -l >var &&\n+\t\tcat var &&\n+\t\tsed -n -e \"/^GIT_EDITOR/,\\$p\" var | head -n 3 >actual &&\n+\t\ttest_cmp expected actual\n+\t)\n+'\n+\n test_expect_success 'listing and asking for variables are exclusive' '\n \ttest_must_fail git var -l GIT_COMMITTER_IDENT\n '\n"},{"id":"478699","messageId":"20230622195059.320593-3-sandals@crustytoothpaste.net","threadId":"59899","inReplyTo":"20230622195059.320593-1-sandals@crustytoothpaste.net","subject":"[PATCH 2/3] var: add attributes files locations","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2023-06-22T19:50:58Z","receivedAt":"2023-06-22T19:51:41Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"From: \"brian m. carlson\" \n\nCurrently, there are some programs which would like to read and parse\nthe gitattributes files at the global or system levels. However, it's\nnot always obvious where these files live, especially for the system\nfile, which may have been hard-coded at compile time or computed\ndynamically based on the runtime prefix.\n\nIt's not reasonable to expect all callers of Git to intuitively know\nwhere the Git distributor or user has configured these locations to\nbe, so add some entries to allow us to determine their location. Honor\nthe GIT_ATTR_NOSYSTEM environment variable if one is specified. Expose\nthe accessor functions in a way that we can reuse them from within the\nvar code.\n\nIn order to make our paths consistent on Windows and also use the same\nform as paths use in \"git rev-parse\", let's normalize the path before we\nreturn it. This results in Windows-style paths that use slashes, which\nis convenient for making our tests function in a consistent way across\nplatforms. Note that this requires that some of our values be freed, so\nlet's add a flag about whether the value needs to be freed and use it\naccordingly.\n\nSigned-off-by: brian m. carlson \n---\n Documentation/git-var.txt | 6 ++++++\n attr.c | 6 +++---\n attr.h | 4 ++++\n builtin/var.c | 45 ++++++++++++++++++++++++++++++++-------\n t/t0007-git-var.sh | 20 +++++++++++++++++\n 5 files changed, 70 insertions(+), 11 deletions(-)\n\ndiff --git a/Documentation/git-var.txt b/Documentation/git-var.txt\nindex f0f647e14a..dfbe5cb52b 100644\n--- a/Documentation/git-var.txt\n+++ b/Documentation/git-var.txt\n@@ -74,6 +74,12 @@ GIT_DEFAULT_BRANCH::\n GIT_SHELL_PATH::\n The path of the binary providing the POSIX shell for commands which use the shell.\n \n+GIT_ATTR_SYSTEM::\n+ The path to the system linkgit:gitattributes[5] file, if one is enabled.\n+\n+GIT_ATTR_GLOBAL::\n+ The path to the global (per-user) linkgit:gitattributes[5] file.\n+\n SEE ALSO\n --------\n linkgit:git-commit-tree[1]\ndiff --git a/attr.c b/attr.c\nindex d45d34058d..822d214053 100644\n--- a/attr.c\n+++ b/attr.c\n@@ -870,7 +870,7 @@ static struct attr_stack *read_attr(struct index_state *istate,\n \treturn res;\n }\n \n-static const char *git_etc_gitattributes(void)\n+const char *git_etc_gitattributes(void)\n {\n \tstatic const char *system_wide;\n \tif (!system_wide)\n@@ -878,7 +878,7 @@ static const char *git_etc_gitattributes(void)\n \treturn system_wide;\n }\n \n-static const char *get_home_gitattributes(void)\n+const char *get_home_gitattributes(void)\n {\n \tif (!git_attributes_file)\n \t\tgit_attributes_file = xdg_config_home(\"attributes\");\n@@ -886,7 +886,7 @@ static const char *get_home_gitattributes(void)\n \treturn git_attributes_file;\n }\n \n-static int git_attr_system(void)\n+int git_attr_system(void)\n {\n \treturn !git_env_bool(\"GIT_ATTR_NOSYSTEM\", 0);\n }\ndiff --git a/attr.h b/attr.h\nindex 676bd17ce2..05a2e4c622 100644\n--- a/attr.h\n+++ b/attr.h\n@@ -227,4 +227,8 @@ void git_attr_set_direction(enum git_attr_direction new_direction);\n \n void attr_start(void);\n \n+const char *git_etc_gitattributes(void);\n+const char *get_home_gitattributes(void);\n+int git_attr_system(void);\n+\n #endif /* ATTR_H */\ndiff --git a/builtin/var.c b/builtin/var.c\nindex f97178eed1..b9e2f23697 100644\n--- a/builtin/var.c\n+++ b/builtin/var.c\n@@ -4,6 +4,7 @@\n * Copyright (C) Eric Biederman, 2005\n */\n #include \"builtin.h\"\n+#include \"attr.h\"\n #include \"config.h\"\n #include \"editor.h\"\n #include \"ident.h\"\n@@ -41,18 +42,41 @@ static const char *shell_path(int flag)\n \treturn SHELL_PATH;\n }\n \n+static const char *git_attr_val_system(int flag)\n+{\n+\tif (git_attr_system()) {\n+\t\tchar *file = xstrdup(git_etc_gitattributes());\n+\t\tnormalize_path_copy(file, file);\n+\t\treturn file;\n+\t}\n+\treturn NULL;\n+}\n+\n+static const char *git_attr_val_global(int flag)\n+{\n+\tchar *file = xstrdup(get_home_gitattributes());\n+\tif (file) {\n+\t\tnormalize_path_copy(file, file);\n+\t\treturn file;\n+\t}\n+\treturn NULL;\n+}\n+\n struct git_var {\n \tconst char *name;\n \tconst char *(*read)(int);\n+\tint free;\n };\n static struct git_var git_vars[] = {\n-\t{ \"GIT_COMMITTER_IDENT\", git_committer_info },\n-\t{ \"GIT_AUTHOR_IDENT\", git_author_info },\n-\t{ \"GIT_EDITOR\", editor },\n-\t{ \"GIT_SEQUENCE_EDITOR\", sequence_editor },\n-\t{ \"GIT_PAGER\", pager },\n-\t{ \"GIT_DEFAULT_BRANCH\", default_branch },\n-\t{ \"GIT_SHELL_PATH\", shell_path },\n+\t{ \"GIT_COMMITTER_IDENT\", git_committer_info, 0 },\n+\t{ \"GIT_AUTHOR_IDENT\", git_author_info, 0 },\n+\t{ \"GIT_EDITOR\", editor, 0 },\n+\t{ \"GIT_SEQUENCE_EDITOR\", sequence_editor, 0 },\n+\t{ \"GIT_PAGER\", pager, 0 },\n+\t{ \"GIT_DEFAULT_BRANCH\", default_branch, 0 },\n+\t{ \"GIT_SHELL_PATH\", shell_path, 0 },\n+\t{ \"GIT_ATTR_SYSTEM\", git_attr_val_system, 1 },\n+\t{ \"GIT_ATTR_GLOBAL\", git_attr_val_global, 1 },\n \t{ \"\", NULL },\n };\n \n@@ -62,8 +86,11 @@ static void list_vars(void)\n \tconst char *val;\n \n \tfor (ptr = git_vars; ptr->read; ptr++)\n-\t\tif ((val = ptr->read(0)))\n+\t\tif ((val = ptr->read(0))) {\n \t\t\tprintf(\"%s=%s\\n\", ptr->name, val);\n+\t\t\tif (ptr->free)\n+\t\t\t\tfree((void *)val);\n+\t\t}\n }\n \n static const struct git_var *get_git_var(const char *var)\n@@ -110,6 +137,8 @@ int cmd_var(int argc, const char **argv, const char *prefix UNUSED)\n \t\treturn 1;\n \n \tprintf(\"%s\\n\", val);\n+\tif (git_var->free)\n+\t\tfree((void *)val);\n \n \treturn 0;\n }\ndiff --git a/t/t0007-git-var.sh b/t/t0007-git-var.sh\nindex 270bd4e512..6a2cc94abb 100755\n--- a/t/t0007-git-var.sh\n+++ b/t/t0007-git-var.sh\n@@ -159,6 +159,26 @@ test_expect_success MINGW 'GIT_SHELL_PATH points to a suitable shell' '\n \tgrep \"sh\\$\" shell\n '\n \n+test_expect_success 'GIT_ATTR_SYSTEM points to the correct location' '\n+\ttest_must_fail env GIT_ATTR_NOSYSTEM=1 git var GIT_ATTR_SYSTEM &&\n+\t(\n+\t\tsane_unset GIT_ATTR_NOSYSTEM &&\n+\t\tgit var GIT_ATTR_SYSTEM >path &&\n+\t\ttest \"$(cat path)\" != \"\"\n+\t)\n+'\n+\n+test_expect_success 'GIT_ATTR_GLOBAL points to the correct location' '\n+\tTRASHDIR=\"$(test-tool path-utils normalize_path_copy \"$(pwd)\")\" &&\n+\tXDG_CONFIG_HOME=\"$TRASHDIR/.config\" git var GIT_ATTR_GLOBAL >path &&\n+\ttest \"$(cat path)\" = \"$TRASHDIR/.config/git/attributes\" &&\n+\t(\n+\t\tsane_unset XDG_CONFIG_HOME &&\n+\t\tHOME=\"$TRASHDIR\" git var GIT_ATTR_GLOBAL >path &&\n+\t\ttest \"$(cat path)\" = \"$TRASHDIR/.config/git/attributes\"\n+\t)\n+'\n+\n # For git var -l, we check only a representative variable;\n # testing the whole output would make our test too brittle with\n # respect to unrelated changes in the test suite's environment.\n"},{"id":"478700","messageId":"ebdf8a20-9ac7-dcf7-8cc6-b63894b7eb45@github.com","threadId":"59899","inReplyTo":"20230622195059.320593-3-sandals@crustytoothpaste.net","subject":"Re: [PATCH 2/3] var: add attributes files locations","fromName":"Derrick Stolee","fromEmail":"derrickstolee@github.com","sentAt":"2023-06-22T20:19:46Z","receivedAt":"2023-06-22T20:19:58Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 6/22/2023 3:50 PM, brian m. carlson wrote:\n\n> struct git_var {\n> \tconst char *name;\n> \tconst char *(*read)(int);\n> +\tint free;\n> };\n\nI see you're expanding the git_var struct, and you do it\nmore in patch 3. At that point, there will be two\nconsecutive 'int' parameters, which can make things\nunclear as to how to proceed.\n\n> static struct git_var git_vars[] = {\n> -\t{ \"GIT_COMMITTER_IDENT\", git_committer_info },\n> -\t{ \"GIT_AUTHOR_IDENT\", git_author_info },\n> -\t{ \"GIT_EDITOR\", editor },\n> -\t{ \"GIT_SEQUENCE_EDITOR\", sequence_editor },\n> -\t{ \"GIT_PAGER\", pager },\n> -\t{ \"GIT_DEFAULT_BRANCH\", default_branch },\n> -\t{ \"GIT_SHELL_PATH\", shell_path },\n> +\t{ \"GIT_COMMITTER_IDENT\", git_committer_info, 0 },\n> +\t{ \"GIT_AUTHOR_IDENT\", git_author_info, 0 },\n\nThis also makes for a big diff like this.\n\nOne way to solve for this is to use the more modern style\nwhen initializing the structs:\n\nstatic struct git_var git_vars[] = {\n\t{\n\t\t.name = \"GIT_COMMITTER_IDENT\",\n\t\t.read = git_author_info,\n\t\t.free = 0,\n\t},\n\t...\n}\n\nBonus points if you do this conversion before modifying\nthe struct, as later changes will only add lines to the\nexisting initialization...\n\n static struct git_var git_vars[] = {\n \t{\n \t\t.name = \"GIT_COMMITTER_IDENT\",\n \t\t.read = git_author_info,\n+ \t\t.multivalued = 0,\n \t\t.free = 0,\n \t},\n \t...\n }\n\nThanks,\n-Stolee\n"},{"id":"478701","messageId":"CAPig+cSTR6oHeYjcHZ7m2CKYcFo2eistxz_X-7J2rhd7h+uf3g@mail.gmail.com","threadId":"59899","inReplyTo":"20230622195059.320593-2-sandals@crustytoothpaste.net","subject":"Re: [PATCH 1/3] var: add support for listing the shell","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2023-06-22T20:42:31Z","receivedAt":"2023-06-22T20:42:46Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Thu, Jun 22, 2023 at 4:03 PM brian m. carlson\n wrote:\n> On most Unix systems, finding a suitable shell is easy: one simply uses\n> \"sh\" with an appropriate PATH value. However, in many Windows\n> environments, the shell is shipped alongside Git, and it may or may not\n> be in PATH, even if Git is.\n>\n> In such an environment, it can be very helpful to query Git for the\n> shell it's using, since other tools may want to use the same shell as\n> well. To help them out, let's add a variable, GIT_SHELL_PATH, that\n> points to the location of the shell.\n>\n> On Unix, we know our shell must be executable to be functional, so\n> assume that the distributor has correctly configured their environment,\n> and use that as a basic test. On Git for Windows, we know that our\n> shell will be one of a few fixed values, all of which end in \"sh\" (such\n> as \"bash\"). This seems like it might be a nice test on Unix as well,\n> since it is customary for all shells to end in \"sh\", but there probably\n> exist such systems that don't have such a configuration, so be careful\n> here not to break them.\n>\n> Signed-off-by: brian m. carlson \n> ---\n> diff --git a/t/t0007-git-var.sh b/t/t0007-git-var.sh\n> @@ -147,6 +147,18 @@ test_expect_success 'get GIT_SEQUENCE_EDITOR with configuration and environment\n> +test_expect_success POSIXPERM 'GIT_SHELL_PATH points to a valid executable' '\n> + git var GIT_SHELL_PATH >shell &&\n> + test -x \"$(cat shell)\"\n> +'\n\nThis can be implemented more simply without a temporary file:\n\n shpath=$(git var GIT_SHELL_PATH) &&\n test -x \"$shpath\"\n\nThis is safe since the exit code of the Git command is preserved\nacross the `shpath=...` assignment.\n\n> +# We know in this environment that our shell will be one of a few fixed values\n> +# that all end in \"sh\".\n> +test_expect_success MINGW 'GIT_SHELL_PATH points to a suitable shell' '\n> + git var GIT_SHELL_PATH >shell &&\n> + grep \"sh\\$\" shell\n> +'\n\nSimilarly, there is no need for a temporary file or an extra process.\nThis can all be done entirely in the shell itself:\n\n shpath=$(git var GIT_SHELL_PATH) &&\n case \"$shpath\" in\n *sh) ;;\n *) return 1\n esac\n"},{"id":"478702","messageId":"xmqqjzvvdx7g.fsf@gitster.g","threadId":"59899","inReplyTo":"CAPig+cSTR6oHeYjcHZ7m2CKYcFo2eistxz_X-7J2rhd7h+uf3g@mail.gmail.com","subject":"Re: [PATCH 1/3] var: add support for listing the shell","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-06-22T21:05:39Z","receivedAt":"2023-06-22T21:09:16Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Sunshine writes:\n\n> This can be implemented more simply without a temporary file:\n>\n> shpath=$(git var GIT_SHELL_PATH) &&\n> test -x \"$shpath\"\n>\n> This is safe since the exit code of the Git command is preserved\n> across the `shpath=...` assignment.\n\nCorrect. I also suspect that we want to add test_path_is_executable\nhelper next to test_path_is_{file,dir,missing} helpers and list it\nin t/README. One downside of your approach is that the output from\nthe command is only in $shpath and cannot be observed easily in the\n$TRASH_DIRECTORY after the test fails, but with such a helper we can\nreport the problematic path when the expectation fails.\n\nThanks.\n"},{"id":"478703","messageId":"CAPig+cTnCth-qBcOXLAEQefpfmnOnBX0wdutwDGsoqJH648FmQ@mail.gmail.com","threadId":"59899","inReplyTo":"xmqqjzvvdx7g.fsf@gitster.g","subject":"Re: [PATCH 1/3] var: add support for listing the shell","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2023-06-22T21:13:08Z","receivedAt":"2023-06-22T21:13:22Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Thu, Jun 22, 2023 at 5:05 PM Junio C Hamano wrote:\n> Eric Sunshine writes:\n> > This can be implemented more simply without a temporary file:\n> >\n> > shpath=$(git var GIT_SHELL_PATH) &&\n> > test -x \"$shpath\"\n> >\n> > This is safe since the exit code of the Git command is preserved\n> > across the `shpath=...` assignment.\n>\n> Correct. I also suspect that we want to add test_path_is_executable\n> helper next to test_path_is_{file,dir,missing} helpers and list it\n> in t/README. One downside of your approach is that the output from\n> the command is only in $shpath and cannot be observed easily in the\n> $TRASH_DIRECTORY after the test fails, but with such a helper we can\n> report the problematic path when the expectation fails.\n\nGood idea. A test_path_is_executable() function would also be a good\nplace to encapsulate the Windows-specific hackiness.\n"},{"id":"478704","messageId":"xmqqcz1ndwnz.fsf@gitster.g","threadId":"59899","inReplyTo":"20230622195059.320593-3-sandals@crustytoothpaste.net","subject":"Re: [PATCH 2/3] var: add attributes files locations","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-06-22T21:17:20Z","receivedAt":"2023-06-22T21:17:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"brian m. carlson\" writes:\n\n> -static const char *git_etc_gitattributes(void)\n> +const char *git_etc_gitattributes(void)\n> {\n> \tstatic const char *system_wide;\n> \tif (!system_wide)\n> @@ -878,7 +878,7 @@ static const char *git_etc_gitattributes(void)\n> \treturn system_wide;\n> }\n> \n> -static const char *get_home_gitattributes(void)\n> +const char *get_home_gitattributes(void)\n> {\n> \tif (!git_attributes_file)\n> \t\tgit_attributes_file = xdg_config_home(\"attributes\");\n> @@ -886,7 +886,7 @@ static const char *get_home_gitattributes(void)\n> \treturn git_attributes_file;\n> }\n\nThese are sensible, but ...\n\n> -static int git_attr_system(void)\n> +int git_attr_system(void)\n> {\n> \treturn !git_env_bool(\"GIT_ATTR_NOSYSTEM\", 0);\n> }\n\n... even though this is not exactly a new problem, but this function\nis misnamed and exposing it outside the file is not exactly nice.\nWe would want to rename it to perhaps git_attr_use_system() or\nsomething? \"allow\" would also work as a verb there, but the point\nis without any verb, it is easily confused with what the function\ngit_etc_git_attributes() wants to do.\n\nside note: arguably the \"etc-gitattributes\" and \"home-gitattributes\"\nare also suboptimal public names to give these functions, even\nthough they are fine as long as they are confined in a subsystem.\nFor global functions, it would be better to give name that match\nwhat the end-users think of them, when possible. The former would\nbe \"git_attr_system()\" and the latter would be \"git_attr_global()\".\n\n> struct git_var {\n> \tconst char *name;\n> \tconst char *(*read)(int);\n> +\tint free;\n> };\n> static struct git_var git_vars[] = {\n> -\t{ \"GIT_COMMITTER_IDENT\", git_committer_info },\n> -\t{ \"GIT_AUTHOR_IDENT\", git_author_info },\n> -\t{ \"GIT_EDITOR\", editor },\n> -\t{ \"GIT_SEQUENCE_EDITOR\", sequence_editor },\n> -\t{ \"GIT_PAGER\", pager },\n> -\t{ \"GIT_DEFAULT_BRANCH\", default_branch },\n> -\t{ \"GIT_SHELL_PATH\", shell_path },\n> +\t{ \"GIT_COMMITTER_IDENT\", git_committer_info, 0 },\n> +\t{ \"GIT_AUTHOR_IDENT\", git_author_info, 0 },\n> +\t{ \"GIT_EDITOR\", editor, 0 },\n> +\t{ \"GIT_SEQUENCE_EDITOR\", sequence_editor, 0 },\n> +\t{ \"GIT_PAGER\", pager, 0 },\n> +\t{ \"GIT_DEFAULT_BRANCH\", default_branch, 0 },\n> +\t{ \"GIT_SHELL_PATH\", shell_path, 0 },\n> +\t{ \"GIT_ATTR_SYSTEM\", git_attr_val_system, 1 },\n> +\t{ \"GIT_ATTR_GLOBAL\", git_attr_val_global, 1 },\n> \t{ \"\", NULL },\n> };\n\nI am not sure if the \"free\" (which stands for \"allocated\") is worth\nadding here for such a single-use-and-then-exit command. It is a\nmaintenance burden for anybody who adds new variables.\n\nEither mark these values with UNLEAK(), or make the ones that do\nnot allocate xstrdup() so that they can be free'd blindly, would be\nless unwieldy option, I guess.\n\n> diff --git a/t/t0007-git-var.sh b/t/t0007-git-var.sh\n> index 270bd4e512..6a2cc94abb 100755\n> --- a/t/t0007-git-var.sh\n> +++ b/t/t0007-git-var.sh\n> @@ -159,6 +159,26 @@ test_expect_success MINGW 'GIT_SHELL_PATH points to a suitable shell' '\n> \tgrep \"sh\\$\" shell\n> '\n> \n> +test_expect_success 'GIT_ATTR_SYSTEM points to the correct location' '\n\nI found it somewhat funny to claim we are checking that the variable\npoints to the \"correct\" location, while the test is happy as long as\nit points at any path.\n\n> +\ttest_must_fail env GIT_ATTR_NOSYSTEM=1 git var GIT_ATTR_SYSTEM &&\n> +\t(\n> +\t\tsane_unset GIT_ATTR_NOSYSTEM &&\n> +\t\tgit var GIT_ATTR_SYSTEM >path &&\n> +\t\ttest \"$(cat path)\" != \"\"\n> +\t)\n> +'\n> +\n> +test_expect_success 'GIT_ATTR_GLOBAL points to the correct location' '\n> +\tTRASHDIR=\"$(test-tool path-utils normalize_path_copy \"$(pwd)\")\" &&\n> +\tXDG_CONFIG_HOME=\"$TRASHDIR/.config\" git var GIT_ATTR_GLOBAL >path &&\n> +\ttest \"$(cat path)\" = \"$TRASHDIR/.config/git/attributes\" &&\n> +\t(\n> +\t\tsane_unset XDG_CONFIG_HOME &&\n> +\t\tHOME=\"$TRASHDIR\" git var GIT_ATTR_GLOBAL >path &&\n> +\t\ttest \"$(cat path)\" = \"$TRASHDIR/.config/git/attributes\"\n> +\t)\n> +'\n\nThis one is much less funny ;-) Very nice.\n"},{"id":"478705","messageId":"ZJS6eARkNOuT+Zd/@tapette.crustytoothpaste.net","threadId":"59899","inReplyTo":"ebdf8a20-9ac7-dcf7-8cc6-b63894b7eb45@github.com","subject":"Re: [PATCH 2/3] var: add attributes files locations","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2023-06-22T21:17:44Z","receivedAt":"2023-06-22T21:17:51Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2023-06-22 at 20:19:46, Derrick Stolee wrote:\n> One way to solve for this is to use the more modern style\n> when initializing the structs:\n> \n> static struct git_var git_vars[] = {\n> \t{\n> \t\t.name = \"GIT_COMMITTER_IDENT\",\n> \t\t.read = git_author_info,\n> \t\t.free = 0,\n> \t},\n> \t...\n> }\n\nGood idea, I've got this in for a v2.\n-- \nbrian m. carlson (he/him or they/them)\nToronto, Ontario, CA\n"},{"id":"478706","messageId":"CAPig+cQ=B7M6nPYx-+gqtsJeSuiqKeQhCfiWmyfjiGXsVnFbTw@mail.gmail.com","threadId":"59899","inReplyTo":"20230622195059.320593-3-sandals@crustytoothpaste.net","subject":"Re: [PATCH 2/3] var: add attributes files locations","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2023-06-22T21:18:46Z","receivedAt":"2023-06-22T21:19:10Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Thu, Jun 22, 2023 at 4:06 PM brian m. carlson\n wrote:\n> Currently, there are some programs which would like to read and parse\n> the gitattributes files at the global or system levels. However, it's\n> not always obvious where these files live, especially for the system\n> file, which may have been hard-coded at compile time or computed\n> dynamically based on the runtime prefix.\n>\n> It's not reasonable to expect all callers of Git to intuitively know\n> where the Git distributor or user has configured these locations to\n> be, so add some entries to allow us to determine their location. Honor\n> the GIT_ATTR_NOSYSTEM environment variable if one is specified. Expose\n> the accessor functions in a way that we can reuse them from within the\n> var code.\n>\n> In order to make our paths consistent on Windows and also use the same\n> form as paths use in \"git rev-parse\", let's normalize the path before we\n> return it. This results in Windows-style paths that use slashes, which\n> is convenient for making our tests function in a consistent way across\n> platforms. Note that this requires that some of our values be freed, so\n> let's add a flag about whether the value needs to be freed and use it\n> accordingly.\n>\n> Signed-off-by: brian m. carlson \n> ---\n> diff --git a/t/t0007-git-var.sh b/t/t0007-git-var.sh\n> @@ -159,6 +159,26 @@ test_expect_success MINGW 'GIT_SHELL_PATH points to a suitable shell' '\n> +test_expect_success 'GIT_ATTR_SYSTEM points to the correct location' '\n> + test_must_fail env GIT_ATTR_NOSYSTEM=1 git var GIT_ATTR_SYSTEM &&\n> + (\n> + sane_unset GIT_ATTR_NOSYSTEM &&\n> + git var GIT_ATTR_SYSTEM >path &&\n> + test \"$(cat path)\" != \"\"\n> + )\n> +'\n\nSame observation as in patch [1/3]: no need for a temporary file:\n\n p=$(git var GIT_ATTR_SYSTEM) &&\n test -n \"$p\"\n\n> +test_expect_success 'GIT_ATTR_GLOBAL points to the correct location' '\n> + TRASHDIR=\"$(test-tool path-utils normalize_path_copy \"$(pwd)\")\" &&\n\nThe reference to $(pwd) is unnecessary, thus potentially confusing. Simpler:\n\n TRASHDIR=\"$(test-tool path-utils normalize_path_copy .)\" &&\n\n> + XDG_CONFIG_HOME=\"$TRASHDIR/.config\" git var GIT_ATTR_GLOBAL >path &&\n> + test \"$(cat path)\" = \"$TRASHDIR/.config/git/attributes\" &&\n\nSame observation about unnecessary temporary file:\n\n p=$(XDG_CONFIG_HOME=\"$TRASHDIR/.config\" git var GIT_ATTR_GLOBAL) &&\n test \"$p\" = \"$TRASHDIR/.config/git/attributes\" &&\n\n> + (\n> + sane_unset XDG_CONFIG_HOME &&\n> + HOME=\"$TRASHDIR\" git var GIT_ATTR_GLOBAL >path &&\n> + test \"$(cat path)\" = \"$TRASHDIR/.config/git/attributes\"\n> + )\n\nAnd here too.\n\n> +'\n"},{"id":"478707","messageId":"ZJS7J/IoHqgZISy6@tapette.crustytoothpaste.net","threadId":"59899","inReplyTo":"CAPig+cSTR6oHeYjcHZ7m2CKYcFo2eistxz_X-7J2rhd7h+uf3g@mail.gmail.com","subject":"Re: [PATCH 1/3] var: add support for listing the shell","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2023-06-22T21:20:39Z","receivedAt":"2023-06-22T21:20:45Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2023-06-22 at 20:42:31, Eric Sunshine wrote:\n> This can be implemented more simply without a temporary file:\n> \n> shpath=$(git var GIT_SHELL_PATH) &&\n> test -x \"$shpath\"\n> \n> This is safe since the exit code of the Git command is preserved\n> across the `shpath=...` assignment.\n\nI can do this in v2.\n\n> Similarly, there is no need for a temporary file or an extra process.\n> This can all be done entirely in the shell itself:\n> \n> shpath=$(git var GIT_SHELL_PATH) &&\n> case \"$shpath\" in\n> *sh) ;;\n> *) return 1\n> esac\n\nThat's true, but this is much uglier and harder to read.\n-- \nbrian m. carlson (he/him or they/them)\nToronto, Ontario, CA\n"},{"id":"478708","messageId":"CAPig+cTq2QfOX71MZbxQo5uLMuekFXtf=eiAk0f_o_Du=v1P_Q@mail.gmail.com","threadId":"59899","inReplyTo":"20230622195059.320593-3-sandals@crustytoothpaste.net","subject":"Re: [PATCH 2/3] var: add attributes files locations","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2023-06-22T21:21:25Z","receivedAt":"2023-06-22T21:21:39Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Thu, Jun 22, 2023 at 4:06 PM brian m. carlson\n wrote:\n> Currently, there are some programs which would like to read and parse\n> the gitattributes files at the global or system levels. However, it's\n> not always obvious where these files live, especially for the system\n> file, which may have been hard-coded at compile time or computed\n> dynamically based on the runtime prefix.\n>\n> It's not reasonable to expect all callers of Git to intuitively know\n> where the Git distributor or user has configured these locations to\n> be, so add some entries to allow us to determine their location. Honor\n> the GIT_ATTR_NOSYSTEM environment variable if one is specified. Expose\n> the accessor functions in a way that we can reuse them from within the\n> var code.\n>\n> In order to make our paths consistent on Windows and also use the same\n> form as paths use in \"git rev-parse\", let's normalize the path before we\n> return it. This results in Windows-style paths that use slashes, which\n> is convenient for making our tests function in a consistent way across\n> platforms. Note that this requires that some of our values be freed, so\n> let's add a flag about whether the value needs to be freed and use it\n> accordingly.\n>\n> Signed-off-by: brian m. carlson \n> ---\n> diff --git a/attr.h b/attr.h\n> @@ -227,4 +227,8 @@ void git_attr_set_direction(enum git_attr_direction new_direction);\n> +const char *git_etc_gitattributes(void);\n> +const char *get_home_gitattributes(void);\n> +int git_attr_system(void);\n\nI forgot to mention that it would be appreciated if you document these\nnew public declarations. Even a one-sentence comment would be helpful.\nFor instance:\n\n /* Return the location of the personal .gitattributes file. */\n const char *get_home_gitattributes(void);\n"},{"id":"478709","messageId":"ZJS8Ot+dHDRXZD/t@tapette.crustytoothpaste.net","threadId":"59899","inReplyTo":"xmqqjzvvdx7g.fsf@gitster.g","subject":"Re: [PATCH 1/3] var: add support for listing the shell","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2023-06-22T21:25:14Z","receivedAt":"2023-06-22T21:25:20Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2023-06-22 at 21:05:39, Junio C Hamano wrote:\n> Correct. I also suspect that we want to add test_path_is_executable\n> helper next to test_path_is_{file,dir,missing} helpers and list it\n> in t/README. One downside of your approach is that the output from\n> the command is only in $shpath and cannot be observed easily in the\n> $TRASH_DIRECTORY after the test fails, but with such a helper we can\n> report the problematic path when the expectation fails.\n\nAt first glance, I thought that was a good idea, too, but unfortunately\nthere is no way to make that work on Windows. That's why all of our\ntests skip those assertions with POSIXPERM, and why my tests\nspecifically look for something different on Windows.\n\nWe could in theory just make it always succeed there, but my concern\nwith writing such a function is that people will think it works\ngenerally, when in fact it does not. That's why, typically throughout\nthe codebase, we specifically use \"test -x\".\n-- \nbrian m. carlson (he/him or they/them)\nToronto, Ontario, CA\n"},{"id":"478710","messageId":"ZJS9XhlIs4jGBjHE@tapette.crustytoothpaste.net","threadId":"59899","inReplyTo":"CAPig+cQ=B7M6nPYx-+gqtsJeSuiqKeQhCfiWmyfjiGXsVnFbTw@mail.gmail.com","subject":"Re: [PATCH 2/3] var: add attributes files locations","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2023-06-22T21:30:06Z","receivedAt":"2023-06-22T21:30:10Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2023-06-22 at 21:18:46, Eric Sunshine wrote:\n> The reference to $(pwd) is unnecessary, thus potentially confusing. Simpler:\n> \n> TRASHDIR=\"$(test-tool path-utils normalize_path_copy .)\" &&\n\nMaybe. I spent a lot of time fighting with Windows and its path\nhandling here (as much as writing the initial series). If that's\npossible and produces an acceptable result there, I can consider it for\nv2.\n-- \nbrian m. carlson (he/him or they/them)\nToronto, Ontario, CA\n"},{"id":"478711","messageId":"CAPig+cTmqUfzrA14xBo1jQSR9kLufze_DFZSf_PC2aCNnOHFcw@mail.gmail.com","threadId":"59899","inReplyTo":"20230622195059.320593-4-sandals@crustytoothpaste.net","subject":"Re: [PATCH 3/3] var: add config file locations","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2023-06-22T21:35:15Z","receivedAt":"2023-06-22T21:35:30Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Thu, Jun 22, 2023 at 4:06 PM brian m. carlson\n wrote:\n> Much like with attributes files, sometimes programs would like to know\n> the location of configuration files at the global or system levels.\n> However, it isn't always clear where these may live, especially for the\n> system file, which may have been hard-coded at compile time or computed\n> dynamically based on the runtime prefix.\n>\n> Since other parties cannot intuitively know how Git was compiled and\n> where it looks for these files, help them by providing variables that\n> can be queried. Because we have multiple paths for global config\n> values, print them in order from highest to lowest priority, and be sure\n> to split on newlines so that \"git var -l\" produces two entries for the\n> global value.\n>\n> However, be careful not to split all values on newlines, since our\n> editor values could well contain such characters, and we don't want to\n> split them in such a case.\n>\n> Note in the documentation that some values may contain multiple paths\n> and that callers should be prepared for that fact. This helps people\n> write code that will continue to work in the event we allow multiple\n> items elsewhere in the future.\n>\n> Signed-off-by: brian m. carlson \n> ---\n> diff --git a/builtin/var.c b/builtin/var.c\n> @@ -62,21 +62,59 @@ static const char *git_attr_val_global(int flag)\n> struct git_var {\n> const char *name;\n> const char *(*read)(int);\n> + int multivalued;\n> int free;\n> };\n> static struct git_var git_vars[] = {\n> + { \"GIT_COMMITTER_IDENT\", git_committer_info, 0, 0 },\n> + { \"GIT_AUTHOR_IDENT\", git_author_info, 0, 0 },\n> + { \"GIT_EDITOR\", editor, 0, 0 },\n> + { \"GIT_SEQUENCE_EDITOR\", sequence_editor, 0, 0 },\n> + { \"GIT_PAGER\", pager, 0, 0 },\n> + { \"GIT_DEFAULT_BRANCH\", default_branch, 0, 9 },\n\nWhy \"9\"?\n\n> + { \"GIT_SHELL_PATH\", shell_path, 0, 0 },\n> + { \"GIT_ATTR_SYSTEM\", git_attr_val_system, 0, 1 },\n> + { \"GIT_ATTR_GLOBAL\", git_attr_val_global, 0, 1 },\n> + { \"GIT_CONFIG_SYSTEM\", git_config_val_system, 0, 1 },\n> + { \"GIT_CONFIG_GLOBAL\", git_config_val_global, 1, 1 },\n> { \"\", NULL },\n> };\n> diff --git a/t/t0007-git-var.sh b/t/t0007-git-var.sh\n> @@ -179,6 +179,49 @@ test_expect_success 'GIT_ATTR_GLOBAL points to the correct location' '\n> +test_expect_success 'GIT_CONFIG_SYSTEM points to the correct location' '\n> + TRASHDIR=\"$(test-tool path-utils normalize_path_copy \"$(pwd)\")\" &&\n\nSame comment as in [2/3]: $(pwd) is unnecessary. Simpler:\n\n TRASHDIR=\"$(test-tool path-utils normalize_path_copy .)\" &&\n\n> + test_must_fail env GIT_CONFIG_NOSYSTEM=1 git var GIT_CONFIG_SYSTEM &&\n> + (\n> + sane_unset GIT_CONFIG_NOSYSTEM &&\n> + git var GIT_CONFIG_SYSTEM >path &&\n> + test \"$(cat path)\" != \"\" &&\n> + GIT_CONFIG_SYSTEM=/dev/null git var GIT_CONFIG_SYSTEM >path &&\n> + if test_have_prereq MINGW\n> + then\n> + test \"$(cat path)\" = \"nul\"\n> + else\n> + test \"$(cat path)\" = \"/dev/null\"\n> + fi &&\n> + GIT_CONFIG_SYSTEM=\"$TRASHDIR/gitconfig\" git var GIT_CONFIG_SYSTEM >path &&\n> + test \"$(cat path)\" = \"$TRASHDIR/gitconfig\"\n> + )\n> +'\n\nDitto regarding unnecessary temporary file.\n\n> +test_expect_success 'GIT_CONFIG_GLOBAL points to the correct location' '\n> + TRASHDIR=\"$(test-tool path-utils normalize_path_copy \"$(pwd)\")\" &&\n\nDitto regarding $(pwd).\n\n> @@ -196,6 +239,29 @@ test_expect_success 'git var -l lists config' '\n> +test_expect_success 'git var -l does not split multiline editors' '\n> + (\n> + GIT_EDITOR=\"!f() {\n> + echo Hello!\n> + }; f\" &&\n> + export GIT_EDITOR &&\n> + echo \"GIT_EDITOR=$GIT_EDITOR\" >expected &&\n> + git var -l >var &&\n> + cat var &&\n\nIs this `cat` leftover debugging code?\n\n> + sed -n -e \"/^GIT_EDITOR/,\\$p\" var | head -n 3 >actual &&\n> + test_cmp expected actual\n> + )\n> +'\n"},{"id":"478712","messageId":"xmqq8rcbdvqn.fsf@gitster.g","threadId":"59899","inReplyTo":"ZJS6eARkNOuT+Zd/@tapette.crustytoothpaste.net","subject":"Re: [PATCH 2/3] var: add attributes files locations","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-06-22T21:37:20Z","receivedAt":"2023-06-22T21:37:29Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"brian m. carlson\" writes:\n\n> On 2023-06-22 at 20:19:46, Derrick Stolee wrote:\n>> One way to solve for this is to use the more modern style\n>> when initializing the structs:\n>> \n>> static struct git_var git_vars[] = {\n>> \t{\n>> \t\t.name = \"GIT_COMMITTER_IDENT\",\n>> \t\t.read = git_author_info,\n>> \t\t.free = 0,\n>> \t},\n>> \t...\n>> }\n>\n> Good idea, I've got this in for a v2.\n\nYeah, this would make it palatable ;-)\n"},{"id":"478713","messageId":"xmqq4jmzdvkh.fsf@gitster.g","threadId":"59899","inReplyTo":"ZJS8Ot+dHDRXZD/t@tapette.crustytoothpaste.net","subject":"Re: [PATCH 1/3] var: add support for listing the shell","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-06-22T21:41:02Z","receivedAt":"2023-06-22T21:41:09Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"brian m. carlson\" writes:\n\n> On 2023-06-22 at 21:05:39, Junio C Hamano wrote:\n>> Correct. I also suspect that we want to add test_path_is_executable\n>> helper next to test_path_is_{file,dir,missing} helpers and list it\n>> in t/README. One downside of your approach is that the output from\n>> the command is only in $shpath and cannot be observed easily in the\n>> $TRASH_DIRECTORY after the test fails, but with such a helper we can\n>> report the problematic path when the expectation fails.\n>\n> At first glance, I thought that was a good idea, too, but unfortunately\n> there is no way to make that work on Windows. That's why all of our\n> tests skip those assertions with POSIXPERM, and why my tests\n> specifically look for something different on Windows.\n>\n> We could in theory just make it always succeed there, but my concern\n> with writing such a function is that people will think it works\n> generally, when in fact it does not. That's why, typically throughout\n> the codebase, we specifically use \"test -x\".\n\nHmph. I would have thought that test_path_is_executable that is\nbased on \"test -x\" and gives a diagnosis when \"test -x\" fails would\nbe better than using bare \"test -x\" and be silent, even on Windows.\n"},{"id":"478847","messageId":"20230626190008.644769-2-sandals@crustytoothpaste.net","threadId":"59899","inReplyTo":"20230626190008.644769-1-sandals@crustytoothpaste.net","subject":"[PATCH v2 1/7] t: add a function to check executable bit","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2023-06-26T19:00:02Z","receivedAt":"2023-06-26T19:00:58Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"From: \"brian m. carlson\" \n\nIn line with our other helper functions for paths, let's add a function\nto check whether a path is executable, and if not, print a suitable\nerror message. Document this function, and note that it must only be\nused under the POSIXPERM prerequisite, since it doesn't otherwise work\non Windows.\n\nSigned-off-by: brian m. carlson \n---\n t/README | 6 ++++++\n t/test-lib-functions.sh | 9 +++++++++\n 2 files changed, 15 insertions(+)\n\ndiff --git a/t/README b/t/README\nindex bdfac4cceb..3e155011de 100644\n--- a/t/README\n+++ b/t/README\n@@ -1098,6 +1098,12 @@ see test-lib-functions.sh for the full list and their options.\n the symbolic link in the file system and a part that does; then only\n the latter part need be protected by a SYMLINKS prerequisite (see below).\n \n+ - test_path_is_executable\n+\n+ This tests whether a file is executable and prints an error message\n+ if not. This must be used only under the POSIXPERM prerequisite\n+ (see below).\n+\n - test_oid_init\n \n This function loads facts and useful object IDs related to the hash\ndiff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh\nindex b3864e22e9..2fa716c567 100644\n--- a/t/test-lib-functions.sh\n+++ b/t/test-lib-functions.sh\n@@ -910,6 +910,15 @@ test_path_is_symlink () {\n \tfi\n }\n \n+test_path_is_executable () {\n+\ttest \"$#\" -ne 1 && BUG \"1 param\"\n+\tif ! test -x \"$1\"\n+\tthen\n+\t\techo \"$1 is not executable\"\n+\t\tfalse\n+\tfi\n+}\n+\n # Check if the directory exists and is empty as expected, barf otherwise.\n test_dir_is_empty () {\n \ttest \"$#\" -ne 1 && BUG \"1 param\"\n"},{"id":"478848","messageId":"20230626190008.644769-1-sandals@crustytoothpaste.net","threadId":"59899","inReplyTo":"20230622195059.320593-1-sandals@crustytoothpaste.net","subject":"[PATCH v2 0/7] Additional variables for git var","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2023-06-26T19:00:01Z","receivedAt":"2023-06-26T19:01:00Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"\nOn many Unix systems, we have a good idea where Git's configuration\nfiles and the shell it uses are located. However, there are some\nsystems where that's not the case, such as Windows and with Homebrew,\nwhere the expected files might be found in another location.\n\nRight now, programs who would like to access things like the system\ngitattributes or config file have to guess where the distributor of Git\nplaced these files, and with runtime relocation, it's not even\nguaranteed that these will be in a fixed place from invocation to\ninvocation. As a result, we need a way to query Git about the location\nof these files.\n\nThis series introduces five new configuration variables that refer to\nthe shell path, the system and global gitattributes files, and the\nsystem and global config files. The global values are not technically\nneeded, since they should be computable from the environment alone, but\nthey are added to make life easier for users.\n\nI have adopted most of the changes suggested, but I have specifically\nnot adopted the use of `.` in our tests instead of `$(pwd)`. That\nbreaks the tests on Windows, the functionality of which has already been\nthe major portion of the time spent on this series despite careful\nforethought about functionality there, and as a non-Windows user, I'm\nsimply not capable of or interested in futzing around with it more.\nParties who are unhappy with that are invited to send a follow-up patch\nafter this series lands.\n\nChanges since v1:\n* Format variables with C99 initializers.\n* Minimize use of temporary files in the tests.\n* Remove debugging statement.\n* Avoid grep where possible in tests.\n* Duplicate memory rather than optionally choosing whether to free data.\n* Add and document test_file_is_executable.\n* Possibly more things which I have forgotten.\n\nbrian m. carlson (7):\n t: add a function to check executable bit\n var: add support for listing the shell\n var: format variable structure with C99 initializers\n var: adjust memory allocation for strings\n attr: expose and rename accessor functions\n var: add attributes files locations\n var: add config file locations\n\n Documentation/git-var.txt | 23 +++++\n attr.c | 14 +--\n attr.h | 9 ++\n builtin/var.c | 178 +++++++++++++++++++++++++++++++++-----\n t/README | 6 ++\n t/t0007-git-var.sh | 100 +++++++++++++++++++++\n t/test-lib-functions.sh | 9 ++\n 7 files changed, 312 insertions(+), 27 deletions(-)\n\nRange-diff against v1:\n-: ---------- > 1: 20b7b85e98 t: add a function to check executable bit\n1: 441dc45a86 ! 2: 7d92b4155f var: add support for listing the shell\n @@ t/t0007-git-var.sh: test_expect_success 'get GIT_SEQUENCE_EDITOR with configurat\n '\n \n +test_expect_success POSIXPERM 'GIT_SHELL_PATH points to a valid executable' '\n -+\tgit var GIT_SHELL_PATH >shell &&\n -+\ttest -x \"$(cat shell)\"\n ++\tshellpath=$(git var GIT_SHELL_PATH) &&\n ++\ttest_path_is_executable \"$shellpath\"\n +'\n +\n +# We know in this environment that our shell will be one of a few fixed values\n +# that all end in \"sh\".\n +test_expect_success MINGW 'GIT_SHELL_PATH points to a suitable shell' '\n -+\tgit var GIT_SHELL_PATH >shell &&\n -+\tgrep \"sh\\$\" shell\n ++\tshellpath=$(git var GIT_SHELL_PATH) &&\n ++\tcase \"$shellpath\" in\n ++\t*sh) ;;\n ++\t*) return 1;;\n ++\tesac\n +'\n +\n # For git var -l, we check only a representative variable;\n2: 3b6935f226 < -: ---------- var: add attributes files locations\n-: ---------- > 3: 29c338d59c var: format variable structure with C99 initializers\n-: ---------- > 4: 2a2a762c44 var: adjust memory allocation for strings\n-: ---------- > 5: c0c5c59df9 attr: expose and rename accessor functions\n-: ---------- > 6: 49a04bd142 var: add attributes files locations\n3: 6a2b5494dd ! 7: a8b4d9396b var: add config file locations\n @@ Documentation/git-var.txt: GIT_ATTR_SYSTEM::\n linkgit:git-commit-tree[1]\n \n ## builtin/var.c ##\n -@@ builtin/var.c: static const char *git_attr_val_global(int flag)\n +@@ builtin/var.c: static char *git_attr_val_global(int flag)\n \treturn NULL;\n }\n \n -+static const char *git_config_val_system(int flag)\n ++static char *git_config_val_system(int flag)\n +{\n +\tif (git_config_system()) {\n +\t\tchar *file = git_system_config();\n @@ builtin/var.c: static const char *git_attr_val_global(int flag)\n +\treturn NULL;\n +}\n +\n -+static const char *git_config_val_global(int flag)\n ++static char *git_config_val_global(int flag)\n +{\n +\tstruct strbuf buf = STRBUF_INIT;\n +\tchar *user, *xdg;\n @@ builtin/var.c: static const char *git_attr_val_global(int flag)\n +\n struct git_var {\n \tconst char *name;\n - \tconst char *(*read)(int);\n + \tchar *(*read)(int);\n +\tint multivalued;\n - \tint free;\n };\n static struct git_var git_vars[] = {\n --\t{ \"GIT_COMMITTER_IDENT\", git_committer_info, 0 },\n --\t{ \"GIT_AUTHOR_IDENT\", git_author_info, 0 },\n --\t{ \"GIT_EDITOR\", editor, 0 },\n --\t{ \"GIT_SEQUENCE_EDITOR\", sequence_editor, 0 },\n --\t{ \"GIT_PAGER\", pager, 0 },\n --\t{ \"GIT_DEFAULT_BRANCH\", default_branch, 0 },\n --\t{ \"GIT_SHELL_PATH\", shell_path, 0 },\n --\t{ \"GIT_ATTR_SYSTEM\", git_attr_val_system, 1 },\n --\t{ \"GIT_ATTR_GLOBAL\", git_attr_val_global, 1 },\n -+\t{ \"GIT_COMMITTER_IDENT\", git_committer_info, 0, 0 },\n -+\t{ \"GIT_AUTHOR_IDENT\", git_author_info, 0, 0 },\n -+\t{ \"GIT_EDITOR\", editor, 0, 0 },\n -+\t{ \"GIT_SEQUENCE_EDITOR\", sequence_editor, 0, 0 },\n -+\t{ \"GIT_PAGER\", pager, 0, 0 },\n -+\t{ \"GIT_DEFAULT_BRANCH\", default_branch, 0, 9 },\n -+\t{ \"GIT_SHELL_PATH\", shell_path, 0, 0 },\n -+\t{ \"GIT_ATTR_SYSTEM\", git_attr_val_system, 0, 1 },\n -+\t{ \"GIT_ATTR_GLOBAL\", git_attr_val_global, 0, 1 },\n -+\t{ \"GIT_CONFIG_SYSTEM\", git_config_val_system, 0, 1 },\n -+\t{ \"GIT_CONFIG_GLOBAL\", git_config_val_global, 1, 1 },\n - \t{ \"\", NULL },\n + \t{\n + \t\t.name = \"GIT_COMMITTER_IDENT\",\n + \t\t.read = committer,\n ++\t\t.multivalued = 0,\n + \t},\n + \t{\n + \t\t.name = \"GIT_AUTHOR_IDENT\",\n + \t\t.read = author,\n ++\t\t.multivalued = 0,\n + \t},\n + \t{\n + \t\t.name = \"GIT_EDITOR\",\n + \t\t.read = editor,\n ++\t\t.multivalued = 0,\n + \t},\n + \t{\n + \t\t.name = \"GIT_SEQUENCE_EDITOR\",\n + \t\t.read = sequence_editor,\n ++\t\t.multivalued = 0,\n + \t},\n + \t{\n + \t\t.name = \"GIT_PAGER\",\n + \t\t.read = pager,\n ++\t\t.multivalued = 0,\n + \t},\n + \t{\n + \t\t.name = \"GIT_DEFAULT_BRANCH\",\n + \t\t.read = default_branch,\n ++\t\t.multivalued = 0,\n + \t},\n + \t{\n + \t\t.name = \"GIT_SHELL_PATH\",\n + \t\t.read = shell_path,\n ++\t\t.multivalued = 0,\n + \t},\n + \t{\n + \t\t.name = \"GIT_ATTR_SYSTEM\",\n + \t\t.read = git_attr_val_system,\n ++\t\t.multivalued = 0,\n + \t},\n + \t{\n + \t\t.name = \"GIT_ATTR_GLOBAL\",\n + \t\t.read = git_attr_val_global,\n ++\t\t.multivalued = 0,\n ++\t},\n ++\t{\n ++\t\t.name = \"GIT_CONFIG_SYSTEM\",\n ++\t\t.read = git_config_val_system,\n ++\t\t.multivalued = 0,\n ++\t},\n ++\t{\n ++\t\t.name = \"GIT_CONFIG_GLOBAL\",\n ++\t\t.read = git_config_val_global,\n ++\t\t.multivalued = 1,\n + \t},\n + \t{\n + \t\t.name = \"\",\n + \t\t.read = NULL,\n ++\t\t.multivalued = 0,\n + \t},\n };\n \n @@ builtin/var.c: static void list_vars(void)\n @@ builtin/var.c: static void list_vars(void)\n +\t\t\t} else {\n +\t\t\t\tprintf(\"%s=%s\\n\", ptr->name, val);\n +\t\t\t}\n - \t\t\tif (ptr->free)\n - \t\t\t\tfree((void *)val);\n + \t\t\tfree(val);\n \t\t}\n + }\n \n ## t/t0007-git-var.sh ##\n @@ t/t0007-git-var.sh: test_expect_success 'GIT_ATTR_GLOBAL points to the correct location' '\n @@ t/t0007-git-var.sh: test_expect_success 'GIT_ATTR_GLOBAL points to the correct l\n +\ttest_must_fail env GIT_CONFIG_NOSYSTEM=1 git var GIT_CONFIG_SYSTEM &&\n +\t(\n +\t\tsane_unset GIT_CONFIG_NOSYSTEM &&\n -+\t\tgit var GIT_CONFIG_SYSTEM >path &&\n -+\t\ttest \"$(cat path)\" != \"\" &&\n -+\t\tGIT_CONFIG_SYSTEM=/dev/null git var GIT_CONFIG_SYSTEM >path &&\n ++\t\tsystempath=$(git var GIT_CONFIG_SYSTEM) &&\n ++\t\ttest \"$systempath\" != \"\" &&\n ++\t\tsystempath=$(GIT_CONFIG_SYSTEM=/dev/null git var GIT_CONFIG_SYSTEM) &&\n +\t\tif test_have_prereq MINGW\n +\t\tthen\n -+\t\t\ttest \"$(cat path)\" = \"nul\"\n ++\t\t\ttest \"$systempath\" = \"nul\"\n +\t\telse\n -+\t\t\ttest \"$(cat path)\" = \"/dev/null\"\n ++\t\t\ttest \"$systempath\" = \"/dev/null\"\n +\t\tfi &&\n -+\t\tGIT_CONFIG_SYSTEM=\"$TRASHDIR/gitconfig\" git var GIT_CONFIG_SYSTEM >path &&\n -+\t\ttest \"$(cat path)\" = \"$TRASHDIR/gitconfig\"\n ++\t\tsystempath=$(GIT_CONFIG_SYSTEM=\"$TRASHDIR/gitconfig\" git var GIT_CONFIG_SYSTEM) &&\n ++\t\ttest \"$systempath\" = \"$TRASHDIR/gitconfig\"\n +\t)\n +'\n +\n @@ t/t0007-git-var.sh: test_expect_success 'GIT_ATTR_GLOBAL points to the correct l\n +\t\techo \"$TRASHDIR/.config/git/config\" >expected &&\n +\t\techo \"$TRASHDIR/.gitconfig\" >>expected &&\n +\t\ttest_cmp expected actual &&\n -+\t\tGIT_CONFIG_GLOBAL=/dev/null git var GIT_CONFIG_GLOBAL >path &&\n ++\t\tglobalpath=$(GIT_CONFIG_GLOBAL=/dev/null git var GIT_CONFIG_GLOBAL) &&\n +\t\tif test_have_prereq MINGW\n +\t\tthen\n -+\t\t\ttest \"$(cat path)\" = \"nul\"\n ++\t\t\ttest \"$globalpath\" = \"nul\"\n +\t\telse\n -+\t\t\ttest \"$(cat path)\" = \"/dev/null\"\n ++\t\t\ttest \"$globalpath\" = \"/dev/null\"\n +\t\tfi &&\n -+\t\tGIT_CONFIG_GLOBAL=\"$TRASHDIR/gitconfig\" git var GIT_CONFIG_GLOBAL >path &&\n -+\t\ttest \"$(cat path)\" = \"$TRASHDIR/gitconfig\"\n ++\t\tglobalpath=$(GIT_CONFIG_GLOBAL=\"$TRASHDIR/gitconfig\" git var GIT_CONFIG_GLOBAL) &&\n ++\t\ttest \"$globalpath\" = \"$TRASHDIR/gitconfig\"\n +\t)\n +'\n +\n @@ t/t0007-git-var.sh: test_expect_success 'git var -l lists config' '\n +\t\texport GIT_EDITOR &&\n +\t\techo \"GIT_EDITOR=$GIT_EDITOR\" >expected &&\n +\t\tgit var -l >var &&\n -+\t\tcat var &&\n +\t\tsed -n -e \"/^GIT_EDITOR/,\\$p\" var | head -n 3 >actual &&\n +\t\ttest_cmp expected actual\n +\t)\n"},{"id":"478849","messageId":"20230626190008.644769-4-sandals@crustytoothpaste.net","threadId":"59899","inReplyTo":"20230626190008.644769-1-sandals@crustytoothpaste.net","subject":"[PATCH v2 3/7] var: format variable structure with C99 initializers","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2023-06-26T19:00:04Z","receivedAt":"2023-06-26T19:01:02Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"From: \"brian m. carlson\" \n\nRight now, we have only two items in our variable struct. However, in\nthe future, we're going to add two more items. To help keep our diffs\nnice and tidy and make this structure easier to read, switch to use\nC99-style initializers for our data.\n\nSigned-off-by: brian m. carlson \n---\n builtin/var.c | 40 ++++++++++++++++++++++++++++++++--------\n 1 file changed, 32 insertions(+), 8 deletions(-)\n\ndiff --git a/builtin/var.c b/builtin/var.c\nindex f97178eed1..bd2750b1bf 100644\n--- a/builtin/var.c\n+++ b/builtin/var.c\n@@ -46,14 +46,38 @@ struct git_var {\n \tconst char *(*read)(int);\n };\n static struct git_var git_vars[] = {\n-\t{ \"GIT_COMMITTER_IDENT\", git_committer_info },\n-\t{ \"GIT_AUTHOR_IDENT\", git_author_info },\n-\t{ \"GIT_EDITOR\", editor },\n-\t{ \"GIT_SEQUENCE_EDITOR\", sequence_editor },\n-\t{ \"GIT_PAGER\", pager },\n-\t{ \"GIT_DEFAULT_BRANCH\", default_branch },\n-\t{ \"GIT_SHELL_PATH\", shell_path },\n-\t{ \"\", NULL },\n+\t{\n+\t\t.name = \"GIT_COMMITTER_IDENT\",\n+\t\t.read = git_committer_info,\n+\t},\n+\t{\n+\t\t.name = \"GIT_AUTHOR_IDENT\",\n+\t\t.read = git_author_info,\n+\t},\n+\t{\n+\t\t.name = \"GIT_EDITOR\",\n+\t\t.read = editor,\n+\t},\n+\t{\n+\t\t.name = \"GIT_SEQUENCE_EDITOR\",\n+\t\t.read = sequence_editor,\n+\t},\n+\t{\n+\t\t.name = \"GIT_PAGER\",\n+\t\t.read = pager,\n+\t},\n+\t{\n+\t\t.name = \"GIT_DEFAULT_BRANCH\",\n+\t\t.read = default_branch,\n+\t},\n+\t{\n+\t\t.name = \"GIT_SHELL_PATH\",\n+\t\t.read = shell_path,\n+\t},\n+\t{\n+\t\t.name = \"\",\n+\t\t.read = NULL,\n+\t},\n };\n \n static void list_vars(void)\n"},{"id":"478850","messageId":"20230626190008.644769-5-sandals@crustytoothpaste.net","threadId":"59899","inReplyTo":"20230626190008.644769-1-sandals@crustytoothpaste.net","subject":"[PATCH v2 4/7] var: adjust memory allocation for strings","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2023-06-26T19:00:05Z","receivedAt":"2023-06-26T19:01:04Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"From: \"brian m. carlson\" \n\nRight now, all of our values are constants whose allocation is managed\nelsewhere. However, in the future, we'll have some variables whose\nmemory we will need to free. To keep things consistent, let's make each\nof our functions allocate its own memory and make the caller responsible\nfor freeing it.\n\nSigned-off-by: brian m. carlson \n---\n builtin/var.c | 45 +++++++++++++++++++++++++++++----------------\n 1 file changed, 29 insertions(+), 16 deletions(-)\n\ndiff --git a/builtin/var.c b/builtin/var.c\nindex bd2750b1bf..ce6a2231ca 100644\n--- a/builtin/var.c\n+++ b/builtin/var.c\n@@ -12,47 +12,57 @@\n \n static const char var_usage[] = \"git var (-l | )\";\n \n-static const char *editor(int flag)\n+static char *committer(int flag)\n {\n-\treturn git_editor();\n+\treturn xstrdup_or_null(git_committer_info(flag));\n }\n \n-static const char *sequence_editor(int flag)\n+static char *author(int flag)\n {\n-\treturn git_sequence_editor();\n+\treturn xstrdup_or_null(git_author_info(flag));\n }\n \n-static const char *pager(int flag)\n+static char *editor(int flag)\n+{\n+\treturn xstrdup_or_null(git_editor());\n+}\n+\n+static char *sequence_editor(int flag)\n+{\n+\treturn xstrdup_or_null(git_sequence_editor());\n+}\n+\n+static char *pager(int flag)\n {\n \tconst char *pgm = git_pager(1);\n \n \tif (!pgm)\n \t\tpgm = \"cat\";\n-\treturn pgm;\n+\treturn xstrdup(pgm);\n }\n \n-static const char *default_branch(int flag)\n+static char *default_branch(int flag)\n {\n-\treturn git_default_branch_name(1);\n+\treturn xstrdup_or_null(git_default_branch_name(1));\n }\n \n-static const char *shell_path(int flag)\n+static char *shell_path(int flag)\n {\n-\treturn SHELL_PATH;\n+\treturn xstrdup(SHELL_PATH);\n }\n \n struct git_var {\n \tconst char *name;\n-\tconst char *(*read)(int);\n+\tchar *(*read)(int);\n };\n static struct git_var git_vars[] = {\n \t{\n \t\t.name = \"GIT_COMMITTER_IDENT\",\n-\t\t.read = git_committer_info,\n+\t\t.read = committer,\n \t},\n \t{\n \t\t.name = \"GIT_AUTHOR_IDENT\",\n-\t\t.read = git_author_info,\n+\t\t.read = author,\n \t},\n \t{\n \t\t.name = \"GIT_EDITOR\",\n@@ -83,11 +93,13 @@ static struct git_var git_vars[] = {\n static void list_vars(void)\n {\n \tstruct git_var *ptr;\n-\tconst char *val;\n+\tchar *val;\n \n \tfor (ptr = git_vars; ptr->read; ptr++)\n-\t\tif ((val = ptr->read(0)))\n+\t\tif ((val = ptr->read(0))) {\n \t\t\tprintf(\"%s=%s\\n\", ptr->name, val);\n+\t\t\tfree(val);\n+\t\t}\n }\n \n static const struct git_var *get_git_var(const char *var)\n@@ -113,7 +125,7 @@ static int show_config(const char *var, const char *value, void *cb)\n int cmd_var(int argc, const char **argv, const char *prefix UNUSED)\n {\n \tconst struct git_var *git_var;\n-\tconst char *val;\n+\tchar *val;\n \n \tif (argc != 2)\n \t\tusage(var_usage);\n@@ -134,6 +146,7 @@ int cmd_var(int argc, const char **argv, const char *prefix UNUSED)\n \t\treturn 1;\n \n \tprintf(\"%s\\n\", val);\n+\tfree(val);\n \n \treturn 0;\n }\n"},{"id":"478851","messageId":"20230626190008.644769-3-sandals@crustytoothpaste.net","threadId":"59899","inReplyTo":"20230626190008.644769-1-sandals@crustytoothpaste.net","subject":"[PATCH v2 2/7] var: add support for listing the shell","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2023-06-26T19:00:03Z","receivedAt":"2023-06-26T19:01:05Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"From: \"brian m. carlson\" \n\nOn most Unix systems, finding a suitable shell is easy: one simply uses\n\"sh\" with an appropriate PATH value. However, in many Windows\nenvironments, the shell is shipped alongside Git, and it may or may not\nbe in PATH, even if Git is.\n\nIn such an environment, it can be very helpful to query Git for the\nshell it's using, since other tools may want to use the same shell as\nwell. To help them out, let's add a variable, GIT_SHELL_PATH, that\npoints to the location of the shell.\n\nOn Unix, we know our shell must be executable to be functional, so\nassume that the distributor has correctly configured their environment,\nand use that as a basic test. On Git for Windows, we know that our\nshell will be one of a few fixed values, all of which end in \"sh\" (such\nas \"bash\"). This seems like it might be a nice test on Unix as well,\nsince it is customary for all shells to end in \"sh\", but there probably\nexist such systems that don't have such a configuration, so be careful\nhere not to break them.\n\nSigned-off-by: brian m. carlson \n---\n Documentation/git-var.txt | 3 +++\n builtin/var.c | 6 ++++++\n t/t0007-git-var.sh | 15 +++++++++++++++\n 3 files changed, 24 insertions(+)\n\ndiff --git a/Documentation/git-var.txt b/Documentation/git-var.txt\nindex f40202b8e3..f0f647e14a 100644\n--- a/Documentation/git-var.txt\n+++ b/Documentation/git-var.txt\n@@ -71,6 +71,9 @@ endif::git-default-pager[]\n GIT_DEFAULT_BRANCH::\n The name of the first branch created in newly initialized repositories.\n \n+GIT_SHELL_PATH::\n+ The path of the binary providing the POSIX shell for commands which use the shell.\n+\n SEE ALSO\n --------\n linkgit:git-commit-tree[1]\ndiff --git a/builtin/var.c b/builtin/var.c\nindex 2149998980..f97178eed1 100644\n--- a/builtin/var.c\n+++ b/builtin/var.c\n@@ -36,6 +36,11 @@ static const char *default_branch(int flag)\n \treturn git_default_branch_name(1);\n }\n \n+static const char *shell_path(int flag)\n+{\n+\treturn SHELL_PATH;\n+}\n+\n struct git_var {\n \tconst char *name;\n \tconst char *(*read)(int);\n@@ -47,6 +52,7 @@ static struct git_var git_vars[] = {\n \t{ \"GIT_SEQUENCE_EDITOR\", sequence_editor },\n \t{ \"GIT_PAGER\", pager },\n \t{ \"GIT_DEFAULT_BRANCH\", default_branch },\n+\t{ \"GIT_SHELL_PATH\", shell_path },\n \t{ \"\", NULL },\n };\n \ndiff --git a/t/t0007-git-var.sh b/t/t0007-git-var.sh\nindex eeb8539c1b..e35f07afcb 100755\n--- a/t/t0007-git-var.sh\n+++ b/t/t0007-git-var.sh\n@@ -147,6 +147,21 @@ test_expect_success 'get GIT_SEQUENCE_EDITOR with configuration and environment\n \t)\n '\n \n+test_expect_success POSIXPERM 'GIT_SHELL_PATH points to a valid executable' '\n+\tshellpath=$(git var GIT_SHELL_PATH) &&\n+\ttest_path_is_executable \"$shellpath\"\n+'\n+\n+# We know in this environment that our shell will be one of a few fixed values\n+# that all end in \"sh\".\n+test_expect_success MINGW 'GIT_SHELL_PATH points to a suitable shell' '\n+\tshellpath=$(git var GIT_SHELL_PATH) &&\n+\tcase \"$shellpath\" in\n+\t*sh) ;;\n+\t*) return 1;;\n+\tesac\n+'\n+\n # For git var -l, we check only a representative variable;\n # testing the whole output would make our test too brittle with\n # respect to unrelated changes in the test suite's environment.\n"},{"id":"478852","messageId":"20230626190008.644769-6-sandals@crustytoothpaste.net","threadId":"59899","inReplyTo":"20230626190008.644769-1-sandals@crustytoothpaste.net","subject":"[PATCH v2 5/7] attr: expose and rename accessor functions","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2023-06-26T19:00:06Z","receivedAt":"2023-06-26T19:01:18Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"From: \"brian m. carlson\" \n\nRight now, the functions which determine the current system and global\ngitattributes files are not exposed. We'd like to use them in a future\ncommit, but they're not ideally named. Rename them to something more\nsuitable as a public interface, expose them, and document them.\n\nSigned-off-by: brian m. carlson \n---\n attr.c | 14 +++++++-------\n attr.h | 9 +++++++++\n 2 files changed, 16 insertions(+), 7 deletions(-)\n\ndiff --git a/attr.c b/attr.c\nindex d45d34058d..66203ce322 100644\n--- a/attr.c\n+++ b/attr.c\n@@ -870,7 +870,7 @@ static struct attr_stack *read_attr(struct index_state *istate,\n \treturn res;\n }\n \n-static const char *git_etc_gitattributes(void)\n+const char *git_attr_system_file(void)\n {\n \tstatic const char *system_wide;\n \tif (!system_wide)\n@@ -878,7 +878,7 @@ static const char *git_etc_gitattributes(void)\n \treturn system_wide;\n }\n \n-static const char *get_home_gitattributes(void)\n+const char *git_attr_global_file(void)\n {\n \tif (!git_attributes_file)\n \t\tgit_attributes_file = xdg_config_home(\"attributes\");\n@@ -886,7 +886,7 @@ static const char *get_home_gitattributes(void)\n \treturn git_attributes_file;\n }\n \n-static int git_attr_system(void)\n+int git_attr_system_is_enabled(void)\n {\n \treturn !git_env_bool(\"GIT_ATTR_NOSYSTEM\", 0);\n }\n@@ -920,14 +920,14 @@ static void bootstrap_attr_stack(struct index_state *istate,\n \tpush_stack(stack, e, NULL, 0);\n \n \t/* system-wide frame */\n-\tif (git_attr_system()) {\n-\t\te = read_attr_from_file(git_etc_gitattributes(), flags);\n+\tif (git_attr_system_is_enabled()) {\n+\t\te = read_attr_from_file(git_attr_system_file(), flags);\n \t\tpush_stack(stack, e, NULL, 0);\n \t}\n \n \t/* home directory */\n-\tif (get_home_gitattributes()) {\n-\t\te = read_attr_from_file(get_home_gitattributes(), flags);\n+\tif (git_attr_global_file()) {\n+\t\te = read_attr_from_file(git_attr_global_file(), flags);\n \t\tpush_stack(stack, e, NULL, 0);\n \t}\n \ndiff --git a/attr.h b/attr.h\nindex 676bd17ce2..2b745df405 100644\n--- a/attr.h\n+++ b/attr.h\n@@ -227,4 +227,13 @@ void git_attr_set_direction(enum git_attr_direction new_direction);\n \n void attr_start(void);\n \n+/* Return the system gitattributes file. */\n+const char *git_attr_system_file(void);\n+\n+/* Return the global gitattributes file, if any. */\n+const char *git_attr_global_file(void);\n+\n+/* Return whether the system gitattributes file is enabled and should be used. */\n+int git_attr_system_is_enabled(void);\n+\n #endif /* ATTR_H */\n"},{"id":"478853","messageId":"20230626190008.644769-7-sandals@crustytoothpaste.net","threadId":"59899","inReplyTo":"20230626190008.644769-1-sandals@crustytoothpaste.net","subject":"[PATCH v2 6/7] var: add attributes files locations","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2023-06-26T19:00:07Z","receivedAt":"2023-06-26T19:01:19Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"From: \"brian m. carlson\" \n\nCurrently, there are some programs which would like to read and parse\nthe gitattributes files at the global or system levels. However, it's\nnot always obvious where these files live, especially for the system\nfile, which may have been hard-coded at compile time or computed\ndynamically based on the runtime prefix.\n\nIt's not reasonable to expect all callers of Git to intuitively know\nwhere the Git distributor or user has configured these locations to\nbe, so add some entries to allow us to determine their location. Honor\nthe GIT_ATTR_NOSYSTEM environment variable if one is specified. Expose\nthe accessor functions in a way that we can reuse them from within the\nvar code.\n\nIn order to make our paths consistent on Windows and also use the same\nform as paths use in \"git rev-parse\", let's normalize the path before we\nreturn it. This results in Windows-style paths that use slashes, which\nis convenient for making our tests function in a consistent way across\nplatforms. Note that this requires that some of our values be freed, so\nlet's add a flag about whether the value needs to be freed and use it\naccordingly.\n\nSigned-off-by: brian m. carlson \n---\n Documentation/git-var.txt | 6 ++++++\n builtin/var.c | 29 +++++++++++++++++++++++++++++\n t/t0007-git-var.sh | 20 ++++++++++++++++++++\n 3 files changed, 55 insertions(+)\n\ndiff --git a/Documentation/git-var.txt b/Documentation/git-var.txt\nindex f0f647e14a..dfbe5cb52b 100644\n--- a/Documentation/git-var.txt\n+++ b/Documentation/git-var.txt\n@@ -74,6 +74,12 @@ GIT_DEFAULT_BRANCH::\n GIT_SHELL_PATH::\n The path of the binary providing the POSIX shell for commands which use the shell.\n \n+GIT_ATTR_SYSTEM::\n+ The path to the system linkgit:gitattributes[5] file, if one is enabled.\n+\n+GIT_ATTR_GLOBAL::\n+ The path to the global (per-user) linkgit:gitattributes[5] file.\n+\n SEE ALSO\n --------\n linkgit:git-commit-tree[1]\ndiff --git a/builtin/var.c b/builtin/var.c\nindex ce6a2231ca..7cec05a49a 100644\n--- a/builtin/var.c\n+++ b/builtin/var.c\n@@ -4,6 +4,7 @@\n * Copyright (C) Eric Biederman, 2005\n */\n #include \"builtin.h\"\n+#include \"attr.h\"\n #include \"config.h\"\n #include \"editor.h\"\n #include \"ident.h\"\n@@ -51,6 +52,26 @@ static char *shell_path(int flag)\n \treturn xstrdup(SHELL_PATH);\n }\n \n+static char *git_attr_val_system(int flag)\n+{\n+\tif (git_attr_system_is_enabled()) {\n+\t\tchar *file = xstrdup(git_attr_system_file());\n+\t\tnormalize_path_copy(file, file);\n+\t\treturn file;\n+\t}\n+\treturn NULL;\n+}\n+\n+static char *git_attr_val_global(int flag)\n+{\n+\tchar *file = xstrdup(git_attr_global_file());\n+\tif (file) {\n+\t\tnormalize_path_copy(file, file);\n+\t\treturn file;\n+\t}\n+\treturn NULL;\n+}\n+\n struct git_var {\n \tconst char *name;\n \tchar *(*read)(int);\n@@ -84,6 +105,14 @@ static struct git_var git_vars[] = {\n \t\t.name = \"GIT_SHELL_PATH\",\n \t\t.read = shell_path,\n \t},\n+\t{\n+\t\t.name = \"GIT_ATTR_SYSTEM\",\n+\t\t.read = git_attr_val_system,\n+\t},\n+\t{\n+\t\t.name = \"GIT_ATTR_GLOBAL\",\n+\t\t.read = git_attr_val_global,\n+\t},\n \t{\n \t\t.name = \"\",\n \t\t.read = NULL,\ndiff --git a/t/t0007-git-var.sh b/t/t0007-git-var.sh\nindex e35f07afcb..374d9f49e9 100755\n--- a/t/t0007-git-var.sh\n+++ b/t/t0007-git-var.sh\n@@ -162,6 +162,26 @@ test_expect_success MINGW 'GIT_SHELL_PATH points to a suitable shell' '\n \tesac\n '\n \n+test_expect_success 'GIT_ATTR_SYSTEM produces expected output' '\n+\ttest_must_fail env GIT_ATTR_NOSYSTEM=1 git var GIT_ATTR_SYSTEM &&\n+\t(\n+\t\tsane_unset GIT_ATTR_NOSYSTEM &&\n+\t\tsystempath=$(git var GIT_ATTR_SYSTEM) &&\n+\t\ttest \"$systempath\" != \"\"\n+\t)\n+'\n+\n+test_expect_success 'GIT_ATTR_GLOBAL points to the correct location' '\n+\tTRASHDIR=\"$(test-tool path-utils normalize_path_copy \"$(pwd)\")\" &&\n+\tglobalpath=$(XDG_CONFIG_HOME=\"$TRASHDIR/.config\" git var GIT_ATTR_GLOBAL) &&\n+\ttest \"$globalpath\" = \"$TRASHDIR/.config/git/attributes\" &&\n+\t(\n+\t\tsane_unset XDG_CONFIG_HOME &&\n+\t\tglobalpath=$(HOME=\"$TRASHDIR\" git var GIT_ATTR_GLOBAL) &&\n+\t\ttest \"$globalpath\" = \"$TRASHDIR/.config/git/attributes\"\n+\t)\n+'\n+\n # For git var -l, we check only a representative variable;\n # testing the whole output would make our test too brittle with\n # respect to unrelated changes in the test suite's environment.\n"},{"id":"478854","messageId":"20230626190008.644769-8-sandals@crustytoothpaste.net","threadId":"59899","inReplyTo":"20230626190008.644769-1-sandals@crustytoothpaste.net","subject":"[PATCH v2 7/7] var: add config file locations","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2023-06-26T19:00:08Z","receivedAt":"2023-06-26T19:01:22Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"From: \"brian m. carlson\" \n\nMuch like with attributes files, sometimes programs would like to know\nthe location of configuration files at the global or system levels.\nHowever, it isn't always clear where these may live, especially for the\nsystem file, which may have been hard-coded at compile time or computed\ndynamically based on the runtime prefix.\n\nSince other parties cannot intuitively know how Git was compiled and\nwhere it looks for these files, help them by providing variables that\ncan be queried. Because we have multiple paths for global config\nvalues, print them in order from highest to lowest priority, and be sure\nto split on newlines so that \"git var -l\" produces two entries for the\nglobal value.\n\nHowever, be careful not to split all values on newlines, since our\neditor values could well contain such characters, and we don't want to\nsplit them in such a case.\n\nNote in the documentation that some values may contain multiple paths\nand that callers should be prepared for that fact. This helps people\nwrite code that will continue to work in the event we allow multiple\nitems elsewhere in the future.\n\nSigned-off-by: brian m. carlson \n---\n Documentation/git-var.txt | 14 ++++++++\n builtin/var.c | 68 ++++++++++++++++++++++++++++++++++++++-\n t/t0007-git-var.sh | 65 +++++++++++++++++++++++++++++++++++++\n 3 files changed, 146 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/git-var.txt b/Documentation/git-var.txt\nindex dfbe5cb52b..c38fb3968b 100644\n--- a/Documentation/git-var.txt\n+++ b/Documentation/git-var.txt\n@@ -80,6 +80,20 @@ GIT_ATTR_SYSTEM::\n GIT_ATTR_GLOBAL::\n The path to the global (per-user) linkgit:gitattributes[5] file.\n \n+GIT_CONFIG_SYSTEM::\n+ The path to the system configuration file, if one is enabled.\n+\n+GIT_CONFIG_GLOBAL::\n+ The path to the global (per-user) configuration files, if any.\n+\n+Most path values contain only one value. However, some can contain multiple\n+values, which are separated by newlines, and are listed in order from highest to\n+lowest priority. Callers should be prepared for any such path value to contain\n+multiple items.\n+\n+Note that paths are printed even if they do not exist, but not if they are\n+disabled by other environment variables.\n+\n SEE ALSO\n --------\n linkgit:git-commit-tree[1]\ndiff --git a/builtin/var.c b/builtin/var.c\nindex 7cec05a49a..bd4e331312 100644\n--- a/builtin/var.c\n+++ b/builtin/var.c\n@@ -72,50 +72,106 @@ static char *git_attr_val_global(int flag)\n \treturn NULL;\n }\n \n+static char *git_config_val_system(int flag)\n+{\n+\tif (git_config_system()) {\n+\t\tchar *file = git_system_config();\n+\t\tnormalize_path_copy(file, file);\n+\t\treturn file;\n+\t}\n+\treturn NULL;\n+}\n+\n+static char *git_config_val_global(int flag)\n+{\n+\tstruct strbuf buf = STRBUF_INIT;\n+\tchar *user, *xdg;\n+\tsize_t unused;\n+\n+\tgit_global_config(&user, &xdg);\n+\tif (xdg && *xdg) {\n+\t\tnormalize_path_copy(xdg, xdg);\n+\t\tstrbuf_addf(&buf, \"%s\\n\", xdg);\n+\t}\n+\tif (user && *user) {\n+\t\tnormalize_path_copy(user, user);\n+\t\tstrbuf_addf(&buf, \"%s\\n\", user);\n+\t}\n+\tfree(xdg);\n+\tfree(user);\n+\tstrbuf_trim_trailing_newline(&buf);\n+\tif (buf.len == 0) {\n+\t\tstrbuf_release(&buf);\n+\t\treturn NULL;\n+\t}\n+\treturn strbuf_detach(&buf, &unused);\n+}\n+\n struct git_var {\n \tconst char *name;\n \tchar *(*read)(int);\n+\tint multivalued;\n };\n static struct git_var git_vars[] = {\n \t{\n \t\t.name = \"GIT_COMMITTER_IDENT\",\n \t\t.read = committer,\n+\t\t.multivalued = 0,\n \t},\n \t{\n \t\t.name = \"GIT_AUTHOR_IDENT\",\n \t\t.read = author,\n+\t\t.multivalued = 0,\n \t},\n \t{\n \t\t.name = \"GIT_EDITOR\",\n \t\t.read = editor,\n+\t\t.multivalued = 0,\n \t},\n \t{\n \t\t.name = \"GIT_SEQUENCE_EDITOR\",\n \t\t.read = sequence_editor,\n+\t\t.multivalued = 0,\n \t},\n \t{\n \t\t.name = \"GIT_PAGER\",\n \t\t.read = pager,\n+\t\t.multivalued = 0,\n \t},\n \t{\n \t\t.name = \"GIT_DEFAULT_BRANCH\",\n \t\t.read = default_branch,\n+\t\t.multivalued = 0,\n \t},\n \t{\n \t\t.name = \"GIT_SHELL_PATH\",\n \t\t.read = shell_path,\n+\t\t.multivalued = 0,\n \t},\n \t{\n \t\t.name = \"GIT_ATTR_SYSTEM\",\n \t\t.read = git_attr_val_system,\n+\t\t.multivalued = 0,\n \t},\n \t{\n \t\t.name = \"GIT_ATTR_GLOBAL\",\n \t\t.read = git_attr_val_global,\n+\t\t.multivalued = 0,\n+\t},\n+\t{\n+\t\t.name = \"GIT_CONFIG_SYSTEM\",\n+\t\t.read = git_config_val_system,\n+\t\t.multivalued = 0,\n+\t},\n+\t{\n+\t\t.name = \"GIT_CONFIG_GLOBAL\",\n+\t\t.read = git_config_val_global,\n+\t\t.multivalued = 1,\n \t},\n \t{\n \t\t.name = \"\",\n \t\t.read = NULL,\n+\t\t.multivalued = 0,\n \t},\n };\n \n@@ -126,7 +182,17 @@ static void list_vars(void)\n \n \tfor (ptr = git_vars; ptr->read; ptr++)\n \t\tif ((val = ptr->read(0))) {\n-\t\t\tprintf(\"%s=%s\\n\", ptr->name, val);\n+\t\t\tif (ptr->multivalued && *val) {\n+\t\t\t\tstruct string_list list = STRING_LIST_INIT_DUP;\n+\t\t\t\tint i;\n+\n+\t\t\t\tstring_list_split(&list, val, '\\n', -1);\n+\t\t\t\tfor (i = 0; i < list.nr; i++)\n+\t\t\t\t\tprintf(\"%s=%s\\n\", ptr->name, list.items[i].string);\n+\t\t\t\tstring_list_clear(&list, 0);\n+\t\t\t} else {\n+\t\t\t\tprintf(\"%s=%s\\n\", ptr->name, val);\n+\t\t\t}\n \t\t\tfree(val);\n \t\t}\n }\ndiff --git a/t/t0007-git-var.sh b/t/t0007-git-var.sh\nindex 374d9f49e9..8cb597f99c 100755\n--- a/t/t0007-git-var.sh\n+++ b/t/t0007-git-var.sh\n@@ -182,6 +182,49 @@ test_expect_success 'GIT_ATTR_GLOBAL points to the correct location' '\n \t)\n '\n \n+test_expect_success 'GIT_CONFIG_SYSTEM points to the correct location' '\n+\tTRASHDIR=\"$(test-tool path-utils normalize_path_copy \"$(pwd)\")\" &&\n+\ttest_must_fail env GIT_CONFIG_NOSYSTEM=1 git var GIT_CONFIG_SYSTEM &&\n+\t(\n+\t\tsane_unset GIT_CONFIG_NOSYSTEM &&\n+\t\tsystempath=$(git var GIT_CONFIG_SYSTEM) &&\n+\t\ttest \"$systempath\" != \"\" &&\n+\t\tsystempath=$(GIT_CONFIG_SYSTEM=/dev/null git var GIT_CONFIG_SYSTEM) &&\n+\t\tif test_have_prereq MINGW\n+\t\tthen\n+\t\t\ttest \"$systempath\" = \"nul\"\n+\t\telse\n+\t\t\ttest \"$systempath\" = \"/dev/null\"\n+\t\tfi &&\n+\t\tsystempath=$(GIT_CONFIG_SYSTEM=\"$TRASHDIR/gitconfig\" git var GIT_CONFIG_SYSTEM) &&\n+\t\ttest \"$systempath\" = \"$TRASHDIR/gitconfig\"\n+\t)\n+'\n+\n+test_expect_success 'GIT_CONFIG_GLOBAL points to the correct location' '\n+\tTRASHDIR=\"$(test-tool path-utils normalize_path_copy \"$(pwd)\")\" &&\n+\tHOME=\"$TRASHDIR\" XDG_CONFIG_HOME=\"$TRASHDIR/foo\" git var GIT_CONFIG_GLOBAL >actual &&\n+\techo \"$TRASHDIR/foo/git/config\" >expected &&\n+\techo \"$TRASHDIR/.gitconfig\" >>expected &&\n+\ttest_cmp expected actual &&\n+\t(\n+\t\tsane_unset XDG_CONFIG_HOME &&\n+\t\tHOME=\"$TRASHDIR\" git var GIT_CONFIG_GLOBAL >actual &&\n+\t\techo \"$TRASHDIR/.config/git/config\" >expected &&\n+\t\techo \"$TRASHDIR/.gitconfig\" >>expected &&\n+\t\ttest_cmp expected actual &&\n+\t\tglobalpath=$(GIT_CONFIG_GLOBAL=/dev/null git var GIT_CONFIG_GLOBAL) &&\n+\t\tif test_have_prereq MINGW\n+\t\tthen\n+\t\t\ttest \"$globalpath\" = \"nul\"\n+\t\telse\n+\t\t\ttest \"$globalpath\" = \"/dev/null\"\n+\t\tfi &&\n+\t\tglobalpath=$(GIT_CONFIG_GLOBAL=\"$TRASHDIR/gitconfig\" git var GIT_CONFIG_GLOBAL) &&\n+\t\ttest \"$globalpath\" = \"$TRASHDIR/gitconfig\"\n+\t)\n+'\n+\n # For git var -l, we check only a representative variable;\n # testing the whole output would make our test too brittle with\n # respect to unrelated changes in the test suite's environment.\n@@ -199,6 +242,28 @@ test_expect_success 'git var -l lists config' '\n \ttest_cmp expect actual.bare\n '\n \n+test_expect_success 'git var -l lists multiple global configs' '\n+\tTRASHDIR=\"$(test-tool path-utils normalize_path_copy \"$(pwd)\")\" &&\n+\tHOME=\"$TRASHDIR\" XDG_CONFIG_HOME=\"$TRASHDIR/foo\" git var -l >actual &&\n+\tgrep \"^GIT_CONFIG_GLOBAL=\" actual >filtered &&\n+\techo \"GIT_CONFIG_GLOBAL=$TRASHDIR/foo/git/config\" >expected &&\n+\techo \"GIT_CONFIG_GLOBAL=$TRASHDIR/.gitconfig\" >>expected &&\n+\ttest_cmp expected filtered\n+'\n+\n+test_expect_success 'git var -l does not split multiline editors' '\n+\t(\n+\t\tGIT_EDITOR=\"!f() {\n+\t\t\techo Hello!\n+\t\t}; f\" &&\n+\t\texport GIT_EDITOR &&\n+\t\techo \"GIT_EDITOR=$GIT_EDITOR\" >expected &&\n+\t\tgit var -l >var &&\n+\t\tsed -n -e \"/^GIT_EDITOR/,\\$p\" var | head -n 3 >actual &&\n+\t\ttest_cmp expected actual\n+\t)\n+'\n+\n test_expect_success 'listing and asking for variables are exclusive' '\n \ttest_must_fail git var -l GIT_COMMITTER_IDENT\n '\n"},{"id":"478856","messageId":"xmqqy1k63slu.fsf@gitster.g","threadId":"59899","inReplyTo":"20230626190008.644769-5-sandals@crustytoothpaste.net","subject":"Re: [PATCH v2 4/7] var: adjust memory allocation for strings","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-06-26T19:56:29Z","receivedAt":"2023-06-26T19:56:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"brian m. carlson\" writes:\n\n> From: \"brian m. carlson\" \n>\n> Right now, all of our values are constants whose allocation is managed\n> elsewhere. However, in the future, we'll have some variables whose\n> memory we will need to free. To keep things consistent, let's make each\n> of our functions allocate its own memory and make the caller responsible\n> for freeing it.\n>\n> Signed-off-by: brian m. carlson \n> ---\n> builtin/var.c | 45 +++++++++++++++++++++++++++++----------------\n> 1 file changed, 29 insertions(+), 16 deletions(-)\n\nMaking everybody allocate like this patch does is also fine, but\nFWIW, with [3/7], we can selectively set \".need_free = 1\" for\nminority of elements in the array that returns an allocated piece of\nmemory without making the source code too noisy (as the array\nelements for existing majority can omit \".need_free = 0\" and leave\nit to the default initialization), so I am not opposed to the \"we\nset .need_free bit only for those that we allocate\" approach taken\nwith the previous round.\n\nThanks.\n"},{"id":"478857","messageId":"xmqqttuu3si7.fsf@gitster.g","threadId":"59899","inReplyTo":"20230626190008.644769-6-sandals@crustytoothpaste.net","subject":"Re: [PATCH v2 5/7] attr: expose and rename accessor functions","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-06-26T19:58:40Z","receivedAt":"2023-06-26T19:58:47Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"brian m. carlson\" writes:\n\n> From: \"brian m. carlson\" \n>\n> Right now, the functions which determine the current system and global\n> gitattributes files are not exposed. We'd like to use them in a future\n> commit, but they're not ideally named. Rename them to something more\n> suitable as a public interface, expose them, and document them.\n>\n> Signed-off-by: brian m. carlson \n> ---\n> attr.c | 14 +++++++-------\n> attr.h | 9 +++++++++\n> 2 files changed, 16 insertions(+), 7 deletions(-)\n\nI very much like the updated names. Thanks for a clean-up that is\nlong overdue for about a dozen years or so.\n\n"},{"id":"478858","messageId":"xmqqpm5i3sbu.fsf@gitster.g","threadId":"59899","inReplyTo":"20230626190008.644769-8-sandals@crustytoothpaste.net","subject":"Re: [PATCH v2 7/7] var: add config file locations","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-06-26T20:02:29Z","receivedAt":"2023-06-26T20:03:46Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"brian m. carlson\" writes:\n\n> struct git_var {\n> \tconst char *name;\n> \tchar *(*read)(int);\n> +\tint multivalued;\n> };\n> static struct git_var git_vars[] = {\n> \t{\n> \t\t.name = \"GIT_COMMITTER_IDENT\",\n> \t\t.read = committer,\n> +\t\t.multivalued = 0,\n> \t},\n> ...\n> +\t{\n> +\t\t.name = \"GIT_CONFIG_GLOBAL\",\n> +\t\t.read = git_config_val_global,\n> +\t\t.multivalued = 1,\n> \t},\n\nAs the majority of the variables are singletons and multi-valued\nvariables are expected to be in minority even in the future, we\nwould probably want to leave \".multivalued = 0\" out of these array\nelements and instead let the default zero initialization to take\nplace, which would make the resulting source less noisy.\n\nOther than that, looks good.\n\nThanks.\n"},{"id":"478866","messageId":"20230627070557.GB1226768@coredump.intra.peff.net","threadId":"59899","inReplyTo":"20230626190008.644769-7-sandals@crustytoothpaste.net","subject":"Re: [PATCH v2 6/7] var: add attributes files locations","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2023-06-27T07:05:57Z","receivedAt":"2023-06-27T07:06:02Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Jun 26, 2023 at 07:00:07PM +0000, brian m. carlson wrote:\n\n> @@ -51,6 +52,26 @@ static char *shell_path(int flag)\n> \treturn xstrdup(SHELL_PATH);\n> }\n> \n> +static char *git_attr_val_system(int flag)\n> +{\n> +\tif (git_attr_system_is_enabled()) {\n> +\t\tchar *file = xstrdup(git_attr_system_file());\n> +\t\tnormalize_path_copy(file, file);\n> +\t\treturn file;\n> +\t}\n> +\treturn NULL;\n> +}\n\nThese new ones would ideally mark the \"flag\" variable with the UNUSED\nattribute (in preparation for building with -Wunused-parameter).\n\nI can also come through later and fix them up in a separate patch. It's\nslightly awkward, just because I was about to post a patch that fixed\nthe existing functions in that file, and I'd have to either rebase on\ntop, or make a second pass once this is merged.\n\nThat said, I also renamed the \"flag\" variable in my patch because it's\nsuper confusing (see my patch below for reference). So adjusting your\nnew callers to match (without my changes) would be a little weird. The\nleast-weird thing would be sticking my patch at the front of your\nseries, but I don't want to make you do extra work.\n\nSo I dunno. I'm mostly giving a heads-up, and seeing if you (or other\nreviewers in the thread) have an idea to make this \"flag\" thing less\nawful. I'm also happy to just do my topic separately, and then\neventually circle back after yours is merged.\n\n-- >8 --\nSubject: [PATCH] var: mark unused parameters in git_var callbacks\n\nWe abstract the set of variables into a table, with a \"read\" callback to\nprovide the value of each. Each callback takes a \"flag\" argument, but\nmost callbacks don't make use of it.\n\nThis flag is a bit odd. It may be set to IDENT_STRICT, which make sense\nfor ident-based callbacks, but is just confusing for things like\nGIT_EDITOR.\n\nAt first glance, it seems like this is just a hack to let us directly\nstick the generic git_committer_info() and git_author_info() functions\ninto our table. And we'd be better off to wrap them with local functions\nwhich pass IDENT_STRICT, and have our callbacks take no option at all.\n\nBut that doesn't quite work. We pass IDENT_STRICT when the caller asks\nfor a specific variable, but otherwise do not (so that \"git var -l\" does\nnot bail if the committer ident cannot be formed).\n\nSo we really do need to pass in the flag to each invocation, even if the\nindividual callback doesn't care about it. Let's mark the unused ones so\nthat -Wunused-parameter does not complain. And while we're here, let's\nrename them so that it's clear that the flag values we get will be from\nthe IDENT_* set. That may prevent confusion for future readers of the\ncode.\n\nAnother option would be to define our own local \"strict\" flag for the\ncallbacks, and then have wrappers that translate that to IDENT_STRICT\nwhere it matters. But that would be more boilerplate for little gain\n(most functions would still ignore the \"strict\" flag anyway).\n\nSigned-off-by: Jeff King \n---\n builtin/var.c | 8 ++++----\n 1 file changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin/var.c b/builtin/var.c\nindex 2149998980..10ee62f84c 100644\n--- a/builtin/var.c\n+++ b/builtin/var.c\n@@ -12,17 +12,17 @@\n \n static const char var_usage[] = \"git var (-l | )\";\n \n-static const char *editor(int flag)\n+static const char *editor(int ident_flag UNUSED)\n {\n \treturn git_editor();\n }\n \n-static const char *sequence_editor(int flag)\n+static const char *sequence_editor(int ident_flag UNUSED)\n {\n \treturn git_sequence_editor();\n }\n \n-static const char *pager(int flag)\n+static const char *pager(int ident_flag UNUSED)\n {\n \tconst char *pgm = git_pager(1);\n \n@@ -31,7 +31,7 @@ static const char *pager(int flag)\n \treturn pgm;\n }\n \n-static const char *default_branch(int flag)\n+static const char *default_branch(int ident_flag UNUSED)\n {\n \treturn git_default_branch_name(1);\n }\n-- \n2.41.0.490.g2678ffb796\n\n"},{"id":"478898","messageId":"ZJsKa6ZNq7nnh0gf@tapette.crustytoothpaste.net","threadId":"59899","inReplyTo":"20230627070557.GB1226768@coredump.intra.peff.net","subject":"Re: [PATCH v2 6/7] var: add attributes files locations","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2023-06-27T16:12:27Z","receivedAt":"2023-06-27T16:12:33Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2023-06-27 at 07:05:57, Jeff King wrote:\n> On Mon, Jun 26, 2023 at 07:00:07PM +0000, brian m. carlson wrote:\n> \n> > @@ -51,6 +52,26 @@ static char *shell_path(int flag)\n> > \treturn xstrdup(SHELL_PATH);\n> > }\n> > \n> > +static char *git_attr_val_system(int flag)\n> > +{\n> > +\tif (git_attr_system_is_enabled()) {\n> > +\t\tchar *file = xstrdup(git_attr_system_file());\n> > +\t\tnormalize_path_copy(file, file);\n> > +\t\treturn file;\n> > +\t}\n> > +\treturn NULL;\n> > +}\n> \n> These new ones would ideally mark the \"flag\" variable with the UNUSED\n> attribute (in preparation for building with -Wunused-parameter).\n> \n> I can also come through later and fix them up in a separate patch. It's\n> slightly awkward, just because I was about to post a patch that fixed\n> the existing functions in that file, and I'd have to either rebase on\n> top, or make a second pass once this is merged.\n> \n> That said, I also renamed the \"flag\" variable in my patch because it's\n> super confusing (see my patch below for reference). So adjusting your\n> new callers to match (without my changes) would be a little weird. The\n> least-weird thing would be sticking my patch at the front of your\n> series, but I don't want to make you do extra work.\n> \n> So I dunno. I'm mostly giving a heads-up, and seeing if you (or other\n> reviewers in the thread) have an idea to make this \"flag\" thing less\n> awful. I'm also happy to just do my topic separately, and then\n> eventually circle back after yours is merged.\n\nI've picked up your patch as the first patch in the series and will send\nit out in v3 in just a few minutes. Since I plan to have v3 be the last\nround of this series, I'll let you send out any further changes as\nfixups on top of that.\n-- \nbrian m. carlson (he/him or they/them)\nToronto, Ontario, CA\n"},{"id":"478899","messageId":"20230627161902.754472-2-sandals@crustytoothpaste.net","threadId":"59899","inReplyTo":"20230627161902.754472-1-sandals@crustytoothpaste.net","subject":"[PATCH v3 1/8] var: mark unused parameters in git_var callbacks","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2023-06-27T16:18:55Z","receivedAt":"2023-06-27T16:19:42Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"From: Jeff King \n\nWe abstract the set of variables into a table, with a \"read\" callback to\nprovide the value of each. Each callback takes a \"flag\" argument, but\nmost callbacks don't make use of it.\n\nThis flag is a bit odd. It may be set to IDENT_STRICT, which make sense\nfor ident-based callbacks, but is just confusing for things like\nGIT_EDITOR.\n\nAt first glance, it seems like this is just a hack to let us directly\nstick the generic git_committer_info() and git_author_info() functions\ninto our table. And we'd be better off to wrap them with local functions\nwhich pass IDENT_STRICT, and have our callbacks take no option at all.\n\nBut that doesn't quite work. We pass IDENT_STRICT when the caller asks\nfor a specific variable, but otherwise do not (so that \"git var -l\" does\nnot bail if the committer ident cannot be formed).\n\nSo we really do need to pass in the flag to each invocation, even if the\nindividual callback doesn't care about it. Let's mark the unused ones so\nthat -Wunused-parameter does not complain. And while we're here, let's\nrename them so that it's clear that the flag values we get will be from\nthe IDENT_* set. That may prevent confusion for future readers of the\ncode.\n\nAnother option would be to define our own local \"strict\" flag for the\ncallbacks, and then have wrappers that translate that to IDENT_STRICT\nwhere it matters. But that would be more boilerplate for little gain\n(most functions would still ignore the \"strict\" flag anyway).\n\nSigned-off-by: Jeff King \nSigned-off-by: brian m. carlson \n---\n builtin/var.c | 8 ++++----\n 1 file changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin/var.c b/builtin/var.c\nindex 2149998980..10ee62f84c 100644\n--- a/builtin/var.c\n+++ b/builtin/var.c\n@@ -12,17 +12,17 @@\n \n static const char var_usage[] = \"git var (-l | )\";\n \n-static const char *editor(int flag)\n+static const char *editor(int ident_flag UNUSED)\n {\n \treturn git_editor();\n }\n \n-static const char *sequence_editor(int flag)\n+static const char *sequence_editor(int ident_flag UNUSED)\n {\n \treturn git_sequence_editor();\n }\n \n-static const char *pager(int flag)\n+static const char *pager(int ident_flag UNUSED)\n {\n \tconst char *pgm = git_pager(1);\n \n@@ -31,7 +31,7 @@ static const char *pager(int flag)\n \treturn pgm;\n }\n \n-static const char *default_branch(int flag)\n+static const char *default_branch(int ident_flag UNUSED)\n {\n \treturn git_default_branch_name(1);\n }\n"},{"id":"478900","messageId":"20230627161902.754472-3-sandals@crustytoothpaste.net","threadId":"59899","inReplyTo":"20230627161902.754472-1-sandals@crustytoothpaste.net","subject":"[PATCH v3 2/8] t: add a function to check executable bit","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2023-06-27T16:18:56Z","receivedAt":"2023-06-27T16:20:08Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"From: \"brian m. carlson\" \n\nIn line with our other helper functions for paths, let's add a function\nto check whether a path is executable, and if not, print a suitable\nerror message. Document this function, and note that it must only be\nused under the POSIXPERM prerequisite, since it doesn't otherwise work\non Windows.\n\nSigned-off-by: brian m. carlson \n---\n t/README | 6 ++++++\n t/test-lib-functions.sh | 9 +++++++++\n 2 files changed, 15 insertions(+)\n\ndiff --git a/t/README b/t/README\nindex bdfac4cceb..3e155011de 100644\n--- a/t/README\n+++ b/t/README\n@@ -1098,6 +1098,12 @@ see test-lib-functions.sh for the full list and their options.\n the symbolic link in the file system and a part that does; then only\n the latter part need be protected by a SYMLINKS prerequisite (see below).\n \n+ - test_path_is_executable\n+\n+ This tests whether a file is executable and prints an error message\n+ if not. This must be used only under the POSIXPERM prerequisite\n+ (see below).\n+\n - test_oid_init\n \n This function loads facts and useful object IDs related to the hash\ndiff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh\nindex b3864e22e9..2fa716c567 100644\n--- a/t/test-lib-functions.sh\n+++ b/t/test-lib-functions.sh\n@@ -910,6 +910,15 @@ test_path_is_symlink () {\n \tfi\n }\n \n+test_path_is_executable () {\n+\ttest \"$#\" -ne 1 && BUG \"1 param\"\n+\tif ! test -x \"$1\"\n+\tthen\n+\t\techo \"$1 is not executable\"\n+\t\tfalse\n+\tfi\n+}\n+\n # Check if the directory exists and is empty as expected, barf otherwise.\n test_dir_is_empty () {\n \ttest \"$#\" -ne 1 && BUG \"1 param\"\n"},{"id":"478901","messageId":"20230627161902.754472-5-sandals@crustytoothpaste.net","threadId":"59899","inReplyTo":"20230627161902.754472-1-sandals@crustytoothpaste.net","subject":"[PATCH v3 4/8] var: format variable structure with C99 initializers","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2023-06-27T16:18:58Z","receivedAt":"2023-06-27T16:20:08Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"From: \"brian m. carlson\" \n\nRight now, we have only two items in our variable struct. However, in\nthe future, we're going to add two more items. To help keep our diffs\nnice and tidy and make this structure easier to read, switch to use\nC99-style initializers for our data.\n\nSigned-off-by: brian m. carlson \n---\n builtin/var.c | 40 ++++++++++++++++++++++++++++++++--------\n 1 file changed, 32 insertions(+), 8 deletions(-)\n\ndiff --git a/builtin/var.c b/builtin/var.c\nindex bd340c5717..379564a399 100644\n--- a/builtin/var.c\n+++ b/builtin/var.c\n@@ -46,14 +46,38 @@ struct git_var {\n \tconst char *(*read)(int);\n };\n static struct git_var git_vars[] = {\n-\t{ \"GIT_COMMITTER_IDENT\", git_committer_info },\n-\t{ \"GIT_AUTHOR_IDENT\", git_author_info },\n-\t{ \"GIT_EDITOR\", editor },\n-\t{ \"GIT_SEQUENCE_EDITOR\", sequence_editor },\n-\t{ \"GIT_PAGER\", pager },\n-\t{ \"GIT_DEFAULT_BRANCH\", default_branch },\n-\t{ \"GIT_SHELL_PATH\", shell_path },\n-\t{ \"\", NULL },\n+\t{\n+\t\t.name = \"GIT_COMMITTER_IDENT\",\n+\t\t.read = git_committer_info,\n+\t},\n+\t{\n+\t\t.name = \"GIT_AUTHOR_IDENT\",\n+\t\t.read = git_author_info,\n+\t},\n+\t{\n+\t\t.name = \"GIT_EDITOR\",\n+\t\t.read = editor,\n+\t},\n+\t{\n+\t\t.name = \"GIT_SEQUENCE_EDITOR\",\n+\t\t.read = sequence_editor,\n+\t},\n+\t{\n+\t\t.name = \"GIT_PAGER\",\n+\t\t.read = pager,\n+\t},\n+\t{\n+\t\t.name = \"GIT_DEFAULT_BRANCH\",\n+\t\t.read = default_branch,\n+\t},\n+\t{\n+\t\t.name = \"GIT_SHELL_PATH\",\n+\t\t.read = shell_path,\n+\t},\n+\t{\n+\t\t.name = \"\",\n+\t\t.read = NULL,\n+\t},\n };\n \n static void list_vars(void)\n"},{"id":"478902","messageId":"20230627161902.754472-4-sandals@crustytoothpaste.net","threadId":"59899","inReplyTo":"20230627161902.754472-1-sandals@crustytoothpaste.net","subject":"[PATCH v3 3/8] var: add support for listing the shell","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2023-06-27T16:18:57Z","receivedAt":"2023-06-27T16:20:08Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"From: \"brian m. carlson\" \n\nOn most Unix systems, finding a suitable shell is easy: one simply uses\n\"sh\" with an appropriate PATH value. However, in many Windows\nenvironments, the shell is shipped alongside Git, and it may or may not\nbe in PATH, even if Git is.\n\nIn such an environment, it can be very helpful to query Git for the\nshell it's using, since other tools may want to use the same shell as\nwell. To help them out, let's add a variable, GIT_SHELL_PATH, that\npoints to the location of the shell.\n\nOn Unix, we know our shell must be executable to be functional, so\nassume that the distributor has correctly configured their environment,\nand use that as a basic test. On Git for Windows, we know that our\nshell will be one of a few fixed values, all of which end in \"sh\" (such\nas \"bash\"). This seems like it might be a nice test on Unix as well,\nsince it is customary for all shells to end in \"sh\", but there probably\nexist such systems that don't have such a configuration, so be careful\nhere not to break them.\n\nSigned-off-by: brian m. carlson \n---\n Documentation/git-var.txt | 3 +++\n builtin/var.c | 6 ++++++\n t/t0007-git-var.sh | 15 +++++++++++++++\n 3 files changed, 24 insertions(+)\n\ndiff --git a/Documentation/git-var.txt b/Documentation/git-var.txt\nindex f40202b8e3..f0f647e14a 100644\n--- a/Documentation/git-var.txt\n+++ b/Documentation/git-var.txt\n@@ -71,6 +71,9 @@ endif::git-default-pager[]\n GIT_DEFAULT_BRANCH::\n The name of the first branch created in newly initialized repositories.\n \n+GIT_SHELL_PATH::\n+ The path of the binary providing the POSIX shell for commands which use the shell.\n+\n SEE ALSO\n --------\n linkgit:git-commit-tree[1]\ndiff --git a/builtin/var.c b/builtin/var.c\nindex 10ee62f84c..bd340c5717 100644\n--- a/builtin/var.c\n+++ b/builtin/var.c\n@@ -36,6 +36,11 @@ static const char *default_branch(int ident_flag UNUSED)\n \treturn git_default_branch_name(1);\n }\n \n+static const char *shell_path(int ident_flag UNUSED)\n+{\n+\treturn SHELL_PATH;\n+}\n+\n struct git_var {\n \tconst char *name;\n \tconst char *(*read)(int);\n@@ -47,6 +52,7 @@ static struct git_var git_vars[] = {\n \t{ \"GIT_SEQUENCE_EDITOR\", sequence_editor },\n \t{ \"GIT_PAGER\", pager },\n \t{ \"GIT_DEFAULT_BRANCH\", default_branch },\n+\t{ \"GIT_SHELL_PATH\", shell_path },\n \t{ \"\", NULL },\n };\n \ndiff --git a/t/t0007-git-var.sh b/t/t0007-git-var.sh\nindex eeb8539c1b..e35f07afcb 100755\n--- a/t/t0007-git-var.sh\n+++ b/t/t0007-git-var.sh\n@@ -147,6 +147,21 @@ test_expect_success 'get GIT_SEQUENCE_EDITOR with configuration and environment\n \t)\n '\n \n+test_expect_success POSIXPERM 'GIT_SHELL_PATH points to a valid executable' '\n+\tshellpath=$(git var GIT_SHELL_PATH) &&\n+\ttest_path_is_executable \"$shellpath\"\n+'\n+\n+# We know in this environment that our shell will be one of a few fixed values\n+# that all end in \"sh\".\n+test_expect_success MINGW 'GIT_SHELL_PATH points to a suitable shell' '\n+\tshellpath=$(git var GIT_SHELL_PATH) &&\n+\tcase \"$shellpath\" in\n+\t*sh) ;;\n+\t*) return 1;;\n+\tesac\n+'\n+\n # For git var -l, we check only a representative variable;\n # testing the whole output would make our test too brittle with\n # respect to unrelated changes in the test suite's environment.\n"},{"id":"478903","messageId":"20230627161902.754472-1-sandals@crustytoothpaste.net","threadId":"59899","inReplyTo":"20230622195059.320593-1-sandals@crustytoothpaste.net","subject":"[PATCH v3 0/8] Additional variables for git var","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2023-06-27T16:18:54Z","receivedAt":"2023-06-27T16:20:08Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On many Unix systems, we have a good idea where Git's configuration\nfiles and the shell it uses are located. However, there are some\nsystems where that's not the case, such as Windows and with Homebrew,\nwhere the expected files might be found in another location.\n\nRight now, programs who would like to access things like the system\ngitattributes or config file have to guess where the distributor of Git\nplaced these files, and with runtime relocation, it's not even\nguaranteed that these will be in a fixed place from invocation to\ninvocation. As a result, we need a way to query Git about the location\nof these files.\n\nThis series introduces five new configuration variables that refer to\nthe shell path, the system and global gitattributes files, and the\nsystem and global config files. The global values are not technically\nneeded, since they should be computable from the environment alone, but\nthey are added to make life easier for users.\n\nMy intention is that this is the last round of this series, but if folks\nhave other comments, please let me know.\n\nChanges since v2:\n* Pick up Peff's patch.\n* Rename function arguments.\n* Mark unused function arguments as unused.\n\nChanges since v1:\n* Format variables with C99 initializers.\n* Minimize use of temporary files in the tests.\n* Remove debugging statement.\n* Avoid grep where possible in tests.\n* Duplicate memory rather than optionally choosing whether to free data.\n* Add and document test_file_is_executable.\n* Possibly more things which I have forgotten.\n\nJeff King (1):\n var: mark unused parameters in git_var callbacks\n\nbrian m. carlson (7):\n t: add a function to check executable bit\n var: add support for listing the shell\n var: format variable structure with C99 initializers\n var: adjust memory allocation for strings\n attr: expose and rename accessor functions\n var: add attributes files locations\n var: add config file locations\n\n Documentation/git-var.txt | 23 ++++++\n attr.c | 14 ++--\n attr.h | 9 ++\n builtin/var.c | 167 +++++++++++++++++++++++++++++++++-----\n t/README | 6 ++\n t/t0007-git-var.sh | 100 +++++++++++++++++++++++\n t/test-lib-functions.sh | 9 ++\n 7 files changed, 301 insertions(+), 27 deletions(-)\n\nRange-diff against v2:\n-: ---------- > 1: 5ed9f04f5b var: mark unused parameters in git_var callbacks\n1: 20b7b85e98 = 2: e7b676b6ae t: add a function to check executable bit\n2: 7d92b4155f ! 3: eae2f049cc var: add support for listing the shell\n @@ Documentation/git-var.txt: endif::git-default-pager[]\n linkgit:git-commit-tree[1]\n \n ## builtin/var.c ##\n -@@ builtin/var.c: static const char *default_branch(int flag)\n +@@ builtin/var.c: static const char *default_branch(int ident_flag UNUSED)\n \treturn git_default_branch_name(1);\n }\n \n -+static const char *shell_path(int flag)\n ++static const char *shell_path(int ident_flag UNUSED)\n +{\n +\treturn SHELL_PATH;\n +}\n3: 29c338d59c = 4: 884f7a2967 var: format variable structure with C99 initializers\n4: 2a2a762c44 ! 5: c609b0a05c var: adjust memory allocation for strings\n @@ builtin/var.c\n \n static const char var_usage[] = \"git var (-l | )\";\n \n --static const char *editor(int flag)\n -+static char *committer(int flag)\n +-static const char *editor(int ident_flag UNUSED)\n ++static char *committer(int ident_flag)\n {\n -\treturn git_editor();\n -+\treturn xstrdup_or_null(git_committer_info(flag));\n ++\treturn xstrdup_or_null(git_committer_info(ident_flag));\n }\n \n --static const char *sequence_editor(int flag)\n -+static char *author(int flag)\n +-static const char *sequence_editor(int ident_flag UNUSED)\n ++static char *author(int ident_flag)\n {\n -\treturn git_sequence_editor();\n -+\treturn xstrdup_or_null(git_author_info(flag));\n ++\treturn xstrdup_or_null(git_author_info(ident_flag));\n }\n \n --static const char *pager(int flag)\n -+static char *editor(int flag)\n +-static const char *pager(int ident_flag UNUSED)\n ++static char *editor(int ident_flag UNUSED)\n +{\n +\treturn xstrdup_or_null(git_editor());\n +}\n +\n -+static char *sequence_editor(int flag)\n ++static char *sequence_editor(int ident_flag UNUSED)\n +{\n +\treturn xstrdup_or_null(git_sequence_editor());\n +}\n +\n -+static char *pager(int flag)\n ++static char *pager(int ident_flag UNUSED)\n {\n \tconst char *pgm = git_pager(1);\n \n @@ builtin/var.c\n +\treturn xstrdup(pgm);\n }\n \n --static const char *default_branch(int flag)\n -+static char *default_branch(int flag)\n +-static const char *default_branch(int ident_flag UNUSED)\n ++static char *default_branch(int ident_flag UNUSED)\n {\n -\treturn git_default_branch_name(1);\n +\treturn xstrdup_or_null(git_default_branch_name(1));\n }\n \n --static const char *shell_path(int flag)\n -+static char *shell_path(int flag)\n +-static const char *shell_path(int ident_flag UNUSED)\n ++static char *shell_path(int ident_flag UNUSED)\n {\n -\treturn SHELL_PATH;\n +\treturn xstrdup(SHELL_PATH);\n5: c0c5c59df9 = 6: d029af8675 attr: expose and rename accessor functions\n6: 49a04bd142 ! 7: 334df489f0 var: add attributes files locations\n @@ builtin/var.c\n #include \"config.h\"\n #include \"editor.h\"\n #include \"ident.h\"\n -@@ builtin/var.c: static char *shell_path(int flag)\n +@@ builtin/var.c: static char *shell_path(int ident_flag UNUSED)\n \treturn xstrdup(SHELL_PATH);\n }\n \n -+static char *git_attr_val_system(int flag)\n ++static char *git_attr_val_system(int ident_flag UNUSED)\n +{\n +\tif (git_attr_system_is_enabled()) {\n +\t\tchar *file = xstrdup(git_attr_system_file());\n @@ builtin/var.c: static char *shell_path(int flag)\n +\treturn NULL;\n +}\n +\n -+static char *git_attr_val_global(int flag)\n ++static char *git_attr_val_global(int ident_flag UNUSED)\n +{\n +\tchar *file = xstrdup(git_attr_global_file());\n +\tif (file) {\n7: a8b4d9396b ! 8: abf079a923 var: add config file locations\n @@ Documentation/git-var.txt: GIT_ATTR_SYSTEM::\n linkgit:git-commit-tree[1]\n \n ## builtin/var.c ##\n -@@ builtin/var.c: static char *git_attr_val_global(int flag)\n +@@ builtin/var.c: static char *git_attr_val_global(int ident_flag UNUSED)\n \treturn NULL;\n }\n \n -+static char *git_config_val_system(int flag)\n ++static char *git_config_val_system(int ident_flag UNUSED)\n +{\n +\tif (git_config_system()) {\n +\t\tchar *file = git_system_config();\n @@ builtin/var.c: static char *git_attr_val_global(int flag)\n +\treturn NULL;\n +}\n +\n -+static char *git_config_val_global(int flag)\n ++static char *git_config_val_global(int ident_flag UNUSED)\n +{\n +\tstruct strbuf buf = STRBUF_INIT;\n +\tchar *user, *xdg;\n @@ builtin/var.c: static char *git_attr_val_global(int flag)\n };\n static struct git_var git_vars[] = {\n \t{\n - \t\t.name = \"GIT_COMMITTER_IDENT\",\n - \t\t.read = committer,\n -+\t\t.multivalued = 0,\n - \t},\n - \t{\n - \t\t.name = \"GIT_AUTHOR_IDENT\",\n - \t\t.read = author,\n -+\t\t.multivalued = 0,\n - \t},\n - \t{\n - \t\t.name = \"GIT_EDITOR\",\n - \t\t.read = editor,\n -+\t\t.multivalued = 0,\n - \t},\n - \t{\n - \t\t.name = \"GIT_SEQUENCE_EDITOR\",\n - \t\t.read = sequence_editor,\n -+\t\t.multivalued = 0,\n - \t},\n - \t{\n - \t\t.name = \"GIT_PAGER\",\n - \t\t.read = pager,\n -+\t\t.multivalued = 0,\n - \t},\n - \t{\n - \t\t.name = \"GIT_DEFAULT_BRANCH\",\n - \t\t.read = default_branch,\n -+\t\t.multivalued = 0,\n - \t},\n - \t{\n - \t\t.name = \"GIT_SHELL_PATH\",\n - \t\t.read = shell_path,\n -+\t\t.multivalued = 0,\n - \t},\n - \t{\n - \t\t.name = \"GIT_ATTR_SYSTEM\",\n - \t\t.read = git_attr_val_system,\n -+\t\t.multivalued = 0,\n - \t},\n - \t{\n +@@ builtin/var.c: static struct git_var git_vars[] = {\n \t\t.name = \"GIT_ATTR_GLOBAL\",\n \t\t.read = git_attr_val_global,\n -+\t\t.multivalued = 0,\n -+\t},\n + \t},\n +\t{\n +\t\t.name = \"GIT_CONFIG_SYSTEM\",\n +\t\t.read = git_config_val_system,\n -+\t\t.multivalued = 0,\n +\t},\n +\t{\n +\t\t.name = \"GIT_CONFIG_GLOBAL\",\n +\t\t.read = git_config_val_global,\n +\t\t.multivalued = 1,\n - \t},\n ++\t},\n \t{\n \t\t.name = \"\",\n \t\t.read = NULL,\n -+\t\t.multivalued = 0,\n - \t},\n - };\n - \n @@ builtin/var.c: static void list_vars(void)\n \n \tfor (ptr = git_vars; ptr->read; ptr++)\n"},{"id":"478904","messageId":"20230627161902.754472-6-sandals@crustytoothpaste.net","threadId":"59899","inReplyTo":"20230627161902.754472-1-sandals@crustytoothpaste.net","subject":"[PATCH v3 5/8] var: adjust memory allocation for strings","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2023-06-27T16:18:59Z","receivedAt":"2023-06-27T16:20:11Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"From: \"brian m. carlson\" \n\nRight now, all of our values are constants whose allocation is managed\nelsewhere. However, in the future, we'll have some variables whose\nmemory we will need to free. To keep things consistent, let's make each\nof our functions allocate its own memory and make the caller responsible\nfor freeing it.\n\nSigned-off-by: brian m. carlson \n---\n builtin/var.c | 45 +++++++++++++++++++++++++++++----------------\n 1 file changed, 29 insertions(+), 16 deletions(-)\n\ndiff --git a/builtin/var.c b/builtin/var.c\nindex 379564a399..d6f9f495c9 100644\n--- a/builtin/var.c\n+++ b/builtin/var.c\n@@ -12,47 +12,57 @@\n \n static const char var_usage[] = \"git var (-l | )\";\n \n-static const char *editor(int ident_flag UNUSED)\n+static char *committer(int ident_flag)\n {\n-\treturn git_editor();\n+\treturn xstrdup_or_null(git_committer_info(ident_flag));\n }\n \n-static const char *sequence_editor(int ident_flag UNUSED)\n+static char *author(int ident_flag)\n {\n-\treturn git_sequence_editor();\n+\treturn xstrdup_or_null(git_author_info(ident_flag));\n }\n \n-static const char *pager(int ident_flag UNUSED)\n+static char *editor(int ident_flag UNUSED)\n+{\n+\treturn xstrdup_or_null(git_editor());\n+}\n+\n+static char *sequence_editor(int ident_flag UNUSED)\n+{\n+\treturn xstrdup_or_null(git_sequence_editor());\n+}\n+\n+static char *pager(int ident_flag UNUSED)\n {\n \tconst char *pgm = git_pager(1);\n \n \tif (!pgm)\n \t\tpgm = \"cat\";\n-\treturn pgm;\n+\treturn xstrdup(pgm);\n }\n \n-static const char *default_branch(int ident_flag UNUSED)\n+static char *default_branch(int ident_flag UNUSED)\n {\n-\treturn git_default_branch_name(1);\n+\treturn xstrdup_or_null(git_default_branch_name(1));\n }\n \n-static const char *shell_path(int ident_flag UNUSED)\n+static char *shell_path(int ident_flag UNUSED)\n {\n-\treturn SHELL_PATH;\n+\treturn xstrdup(SHELL_PATH);\n }\n \n struct git_var {\n \tconst char *name;\n-\tconst char *(*read)(int);\n+\tchar *(*read)(int);\n };\n static struct git_var git_vars[] = {\n \t{\n \t\t.name = \"GIT_COMMITTER_IDENT\",\n-\t\t.read = git_committer_info,\n+\t\t.read = committer,\n \t},\n \t{\n \t\t.name = \"GIT_AUTHOR_IDENT\",\n-\t\t.read = git_author_info,\n+\t\t.read = author,\n \t},\n \t{\n \t\t.name = \"GIT_EDITOR\",\n@@ -83,11 +93,13 @@ static struct git_var git_vars[] = {\n static void list_vars(void)\n {\n \tstruct git_var *ptr;\n-\tconst char *val;\n+\tchar *val;\n \n \tfor (ptr = git_vars; ptr->read; ptr++)\n-\t\tif ((val = ptr->read(0)))\n+\t\tif ((val = ptr->read(0))) {\n \t\t\tprintf(\"%s=%s\\n\", ptr->name, val);\n+\t\t\tfree(val);\n+\t\t}\n }\n \n static const struct git_var *get_git_var(const char *var)\n@@ -113,7 +125,7 @@ static int show_config(const char *var, const char *value, void *cb)\n int cmd_var(int argc, const char **argv, const char *prefix UNUSED)\n {\n \tconst struct git_var *git_var;\n-\tconst char *val;\n+\tchar *val;\n \n \tif (argc != 2)\n \t\tusage(var_usage);\n@@ -134,6 +146,7 @@ int cmd_var(int argc, const char **argv, const char *prefix UNUSED)\n \t\treturn 1;\n \n \tprintf(\"%s\\n\", val);\n+\tfree(val);\n \n \treturn 0;\n }\n"},{"id":"478905","messageId":"20230627161902.754472-9-sandals@crustytoothpaste.net","threadId":"59899","inReplyTo":"20230627161902.754472-1-sandals@crustytoothpaste.net","subject":"[PATCH v3 8/8] var: add config file locations","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2023-06-27T16:19:02Z","receivedAt":"2023-06-27T16:20:13Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"From: \"brian m. carlson\" \n\nMuch like with attributes files, sometimes programs would like to know\nthe location of configuration files at the global or system levels.\nHowever, it isn't always clear where these may live, especially for the\nsystem file, which may have been hard-coded at compile time or computed\ndynamically based on the runtime prefix.\n\nSince other parties cannot intuitively know how Git was compiled and\nwhere it looks for these files, help them by providing variables that\ncan be queried. Because we have multiple paths for global config\nvalues, print them in order from highest to lowest priority, and be sure\nto split on newlines so that \"git var -l\" produces two entries for the\nglobal value.\n\nHowever, be careful not to split all values on newlines, since our\neditor values could well contain such characters, and we don't want to\nsplit them in such a case.\n\nNote in the documentation that some values may contain multiple paths\nand that callers should be prepared for that fact. This helps people\nwrite code that will continue to work in the event we allow multiple\nitems elsewhere in the future.\n\nSigned-off-by: brian m. carlson \n---\n Documentation/git-var.txt | 14 +++++++++\n builtin/var.c | 57 +++++++++++++++++++++++++++++++++-\n t/t0007-git-var.sh | 65 +++++++++++++++++++++++++++++++++++++++\n 3 files changed, 135 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/git-var.txt b/Documentation/git-var.txt\nindex dfbe5cb52b..c38fb3968b 100644\n--- a/Documentation/git-var.txt\n+++ b/Documentation/git-var.txt\n@@ -80,6 +80,20 @@ GIT_ATTR_SYSTEM::\n GIT_ATTR_GLOBAL::\n The path to the global (per-user) linkgit:gitattributes[5] file.\n \n+GIT_CONFIG_SYSTEM::\n+ The path to the system configuration file, if one is enabled.\n+\n+GIT_CONFIG_GLOBAL::\n+ The path to the global (per-user) configuration files, if any.\n+\n+Most path values contain only one value. However, some can contain multiple\n+values, which are separated by newlines, and are listed in order from highest to\n+lowest priority. Callers should be prepared for any such path value to contain\n+multiple items.\n+\n+Note that paths are printed even if they do not exist, but not if they are\n+disabled by other environment variables.\n+\n SEE ALSO\n --------\n linkgit:git-commit-tree[1]\ndiff --git a/builtin/var.c b/builtin/var.c\nindex 79f7bdf55f..ef45710a20 100644\n--- a/builtin/var.c\n+++ b/builtin/var.c\n@@ -72,9 +72,45 @@ static char *git_attr_val_global(int ident_flag UNUSED)\n \treturn NULL;\n }\n \n+static char *git_config_val_system(int ident_flag UNUSED)\n+{\n+\tif (git_config_system()) {\n+\t\tchar *file = git_system_config();\n+\t\tnormalize_path_copy(file, file);\n+\t\treturn file;\n+\t}\n+\treturn NULL;\n+}\n+\n+static char *git_config_val_global(int ident_flag UNUSED)\n+{\n+\tstruct strbuf buf = STRBUF_INIT;\n+\tchar *user, *xdg;\n+\tsize_t unused;\n+\n+\tgit_global_config(&user, &xdg);\n+\tif (xdg && *xdg) {\n+\t\tnormalize_path_copy(xdg, xdg);\n+\t\tstrbuf_addf(&buf, \"%s\\n\", xdg);\n+\t}\n+\tif (user && *user) {\n+\t\tnormalize_path_copy(user, user);\n+\t\tstrbuf_addf(&buf, \"%s\\n\", user);\n+\t}\n+\tfree(xdg);\n+\tfree(user);\n+\tstrbuf_trim_trailing_newline(&buf);\n+\tif (buf.len == 0) {\n+\t\tstrbuf_release(&buf);\n+\t\treturn NULL;\n+\t}\n+\treturn strbuf_detach(&buf, &unused);\n+}\n+\n struct git_var {\n \tconst char *name;\n \tchar *(*read)(int);\n+\tint multivalued;\n };\n static struct git_var git_vars[] = {\n \t{\n@@ -113,6 +149,15 @@ static struct git_var git_vars[] = {\n \t\t.name = \"GIT_ATTR_GLOBAL\",\n \t\t.read = git_attr_val_global,\n \t},\n+\t{\n+\t\t.name = \"GIT_CONFIG_SYSTEM\",\n+\t\t.read = git_config_val_system,\n+\t},\n+\t{\n+\t\t.name = \"GIT_CONFIG_GLOBAL\",\n+\t\t.read = git_config_val_global,\n+\t\t.multivalued = 1,\n+\t},\n \t{\n \t\t.name = \"\",\n \t\t.read = NULL,\n@@ -126,7 +171,17 @@ static void list_vars(void)\n \n \tfor (ptr = git_vars; ptr->read; ptr++)\n \t\tif ((val = ptr->read(0))) {\n-\t\t\tprintf(\"%s=%s\\n\", ptr->name, val);\n+\t\t\tif (ptr->multivalued && *val) {\n+\t\t\t\tstruct string_list list = STRING_LIST_INIT_DUP;\n+\t\t\t\tint i;\n+\n+\t\t\t\tstring_list_split(&list, val, '\\n', -1);\n+\t\t\t\tfor (i = 0; i < list.nr; i++)\n+\t\t\t\t\tprintf(\"%s=%s\\n\", ptr->name, list.items[i].string);\n+\t\t\t\tstring_list_clear(&list, 0);\n+\t\t\t} else {\n+\t\t\t\tprintf(\"%s=%s\\n\", ptr->name, val);\n+\t\t\t}\n \t\t\tfree(val);\n \t\t}\n }\ndiff --git a/t/t0007-git-var.sh b/t/t0007-git-var.sh\nindex 374d9f49e9..8cb597f99c 100755\n--- a/t/t0007-git-var.sh\n+++ b/t/t0007-git-var.sh\n@@ -182,6 +182,49 @@ test_expect_success 'GIT_ATTR_GLOBAL points to the correct location' '\n \t)\n '\n \n+test_expect_success 'GIT_CONFIG_SYSTEM points to the correct location' '\n+\tTRASHDIR=\"$(test-tool path-utils normalize_path_copy \"$(pwd)\")\" &&\n+\ttest_must_fail env GIT_CONFIG_NOSYSTEM=1 git var GIT_CONFIG_SYSTEM &&\n+\t(\n+\t\tsane_unset GIT_CONFIG_NOSYSTEM &&\n+\t\tsystempath=$(git var GIT_CONFIG_SYSTEM) &&\n+\t\ttest \"$systempath\" != \"\" &&\n+\t\tsystempath=$(GIT_CONFIG_SYSTEM=/dev/null git var GIT_CONFIG_SYSTEM) &&\n+\t\tif test_have_prereq MINGW\n+\t\tthen\n+\t\t\ttest \"$systempath\" = \"nul\"\n+\t\telse\n+\t\t\ttest \"$systempath\" = \"/dev/null\"\n+\t\tfi &&\n+\t\tsystempath=$(GIT_CONFIG_SYSTEM=\"$TRASHDIR/gitconfig\" git var GIT_CONFIG_SYSTEM) &&\n+\t\ttest \"$systempath\" = \"$TRASHDIR/gitconfig\"\n+\t)\n+'\n+\n+test_expect_success 'GIT_CONFIG_GLOBAL points to the correct location' '\n+\tTRASHDIR=\"$(test-tool path-utils normalize_path_copy \"$(pwd)\")\" &&\n+\tHOME=\"$TRASHDIR\" XDG_CONFIG_HOME=\"$TRASHDIR/foo\" git var GIT_CONFIG_GLOBAL >actual &&\n+\techo \"$TRASHDIR/foo/git/config\" >expected &&\n+\techo \"$TRASHDIR/.gitconfig\" >>expected &&\n+\ttest_cmp expected actual &&\n+\t(\n+\t\tsane_unset XDG_CONFIG_HOME &&\n+\t\tHOME=\"$TRASHDIR\" git var GIT_CONFIG_GLOBAL >actual &&\n+\t\techo \"$TRASHDIR/.config/git/config\" >expected &&\n+\t\techo \"$TRASHDIR/.gitconfig\" >>expected &&\n+\t\ttest_cmp expected actual &&\n+\t\tglobalpath=$(GIT_CONFIG_GLOBAL=/dev/null git var GIT_CONFIG_GLOBAL) &&\n+\t\tif test_have_prereq MINGW\n+\t\tthen\n+\t\t\ttest \"$globalpath\" = \"nul\"\n+\t\telse\n+\t\t\ttest \"$globalpath\" = \"/dev/null\"\n+\t\tfi &&\n+\t\tglobalpath=$(GIT_CONFIG_GLOBAL=\"$TRASHDIR/gitconfig\" git var GIT_CONFIG_GLOBAL) &&\n+\t\ttest \"$globalpath\" = \"$TRASHDIR/gitconfig\"\n+\t)\n+'\n+\n # For git var -l, we check only a representative variable;\n # testing the whole output would make our test too brittle with\n # respect to unrelated changes in the test suite's environment.\n@@ -199,6 +242,28 @@ test_expect_success 'git var -l lists config' '\n \ttest_cmp expect actual.bare\n '\n \n+test_expect_success 'git var -l lists multiple global configs' '\n+\tTRASHDIR=\"$(test-tool path-utils normalize_path_copy \"$(pwd)\")\" &&\n+\tHOME=\"$TRASHDIR\" XDG_CONFIG_HOME=\"$TRASHDIR/foo\" git var -l >actual &&\n+\tgrep \"^GIT_CONFIG_GLOBAL=\" actual >filtered &&\n+\techo \"GIT_CONFIG_GLOBAL=$TRASHDIR/foo/git/config\" >expected &&\n+\techo \"GIT_CONFIG_GLOBAL=$TRASHDIR/.gitconfig\" >>expected &&\n+\ttest_cmp expected filtered\n+'\n+\n+test_expect_success 'git var -l does not split multiline editors' '\n+\t(\n+\t\tGIT_EDITOR=\"!f() {\n+\t\t\techo Hello!\n+\t\t}; f\" &&\n+\t\texport GIT_EDITOR &&\n+\t\techo \"GIT_EDITOR=$GIT_EDITOR\" >expected &&\n+\t\tgit var -l >var &&\n+\t\tsed -n -e \"/^GIT_EDITOR/,\\$p\" var | head -n 3 >actual &&\n+\t\ttest_cmp expected actual\n+\t)\n+'\n+\n test_expect_success 'listing and asking for variables are exclusive' '\n \ttest_must_fail git var -l GIT_COMMITTER_IDENT\n '\n"},{"id":"478906","messageId":"20230627161902.754472-8-sandals@crustytoothpaste.net","threadId":"59899","inReplyTo":"20230627161902.754472-1-sandals@crustytoothpaste.net","subject":"[PATCH v3 7/8] var: add attributes files locations","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2023-06-27T16:19:01Z","receivedAt":"2023-06-27T16:20:15Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"From: \"brian m. carlson\" \n\nCurrently, there are some programs which would like to read and parse\nthe gitattributes files at the global or system levels. However, it's\nnot always obvious where these files live, especially for the system\nfile, which may have been hard-coded at compile time or computed\ndynamically based on the runtime prefix.\n\nIt's not reasonable to expect all callers of Git to intuitively know\nwhere the Git distributor or user has configured these locations to\nbe, so add some entries to allow us to determine their location. Honor\nthe GIT_ATTR_NOSYSTEM environment variable if one is specified. Expose\nthe accessor functions in a way that we can reuse them from within the\nvar code.\n\nIn order to make our paths consistent on Windows and also use the same\nform as paths use in \"git rev-parse\", let's normalize the path before we\nreturn it. This results in Windows-style paths that use slashes, which\nis convenient for making our tests function in a consistent way across\nplatforms. Note that this requires that some of our values be freed, so\nlet's add a flag about whether the value needs to be freed and use it\naccordingly.\n\nSigned-off-by: brian m. carlson \n---\n Documentation/git-var.txt | 6 ++++++\n builtin/var.c | 29 +++++++++++++++++++++++++++++\n t/t0007-git-var.sh | 20 ++++++++++++++++++++\n 3 files changed, 55 insertions(+)\n\ndiff --git a/Documentation/git-var.txt b/Documentation/git-var.txt\nindex f0f647e14a..dfbe5cb52b 100644\n--- a/Documentation/git-var.txt\n+++ b/Documentation/git-var.txt\n@@ -74,6 +74,12 @@ GIT_DEFAULT_BRANCH::\n GIT_SHELL_PATH::\n The path of the binary providing the POSIX shell for commands which use the shell.\n \n+GIT_ATTR_SYSTEM::\n+ The path to the system linkgit:gitattributes[5] file, if one is enabled.\n+\n+GIT_ATTR_GLOBAL::\n+ The path to the global (per-user) linkgit:gitattributes[5] file.\n+\n SEE ALSO\n --------\n linkgit:git-commit-tree[1]\ndiff --git a/builtin/var.c b/builtin/var.c\nindex d6f9f495c9..79f7bdf55f 100644\n--- a/builtin/var.c\n+++ b/builtin/var.c\n@@ -4,6 +4,7 @@\n * Copyright (C) Eric Biederman, 2005\n */\n #include \"builtin.h\"\n+#include \"attr.h\"\n #include \"config.h\"\n #include \"editor.h\"\n #include \"ident.h\"\n@@ -51,6 +52,26 @@ static char *shell_path(int ident_flag UNUSED)\n \treturn xstrdup(SHELL_PATH);\n }\n \n+static char *git_attr_val_system(int ident_flag UNUSED)\n+{\n+\tif (git_attr_system_is_enabled()) {\n+\t\tchar *file = xstrdup(git_attr_system_file());\n+\t\tnormalize_path_copy(file, file);\n+\t\treturn file;\n+\t}\n+\treturn NULL;\n+}\n+\n+static char *git_attr_val_global(int ident_flag UNUSED)\n+{\n+\tchar *file = xstrdup(git_attr_global_file());\n+\tif (file) {\n+\t\tnormalize_path_copy(file, file);\n+\t\treturn file;\n+\t}\n+\treturn NULL;\n+}\n+\n struct git_var {\n \tconst char *name;\n \tchar *(*read)(int);\n@@ -84,6 +105,14 @@ static struct git_var git_vars[] = {\n \t\t.name = \"GIT_SHELL_PATH\",\n \t\t.read = shell_path,\n \t},\n+\t{\n+\t\t.name = \"GIT_ATTR_SYSTEM\",\n+\t\t.read = git_attr_val_system,\n+\t},\n+\t{\n+\t\t.name = \"GIT_ATTR_GLOBAL\",\n+\t\t.read = git_attr_val_global,\n+\t},\n \t{\n \t\t.name = \"\",\n \t\t.read = NULL,\ndiff --git a/t/t0007-git-var.sh b/t/t0007-git-var.sh\nindex e35f07afcb..374d9f49e9 100755\n--- a/t/t0007-git-var.sh\n+++ b/t/t0007-git-var.sh\n@@ -162,6 +162,26 @@ test_expect_success MINGW 'GIT_SHELL_PATH points to a suitable shell' '\n \tesac\n '\n \n+test_expect_success 'GIT_ATTR_SYSTEM produces expected output' '\n+\ttest_must_fail env GIT_ATTR_NOSYSTEM=1 git var GIT_ATTR_SYSTEM &&\n+\t(\n+\t\tsane_unset GIT_ATTR_NOSYSTEM &&\n+\t\tsystempath=$(git var GIT_ATTR_SYSTEM) &&\n+\t\ttest \"$systempath\" != \"\"\n+\t)\n+'\n+\n+test_expect_success 'GIT_ATTR_GLOBAL points to the correct location' '\n+\tTRASHDIR=\"$(test-tool path-utils normalize_path_copy \"$(pwd)\")\" &&\n+\tglobalpath=$(XDG_CONFIG_HOME=\"$TRASHDIR/.config\" git var GIT_ATTR_GLOBAL) &&\n+\ttest \"$globalpath\" = \"$TRASHDIR/.config/git/attributes\" &&\n+\t(\n+\t\tsane_unset XDG_CONFIG_HOME &&\n+\t\tglobalpath=$(HOME=\"$TRASHDIR\" git var GIT_ATTR_GLOBAL) &&\n+\t\ttest \"$globalpath\" = \"$TRASHDIR/.config/git/attributes\"\n+\t)\n+'\n+\n # For git var -l, we check only a representative variable;\n # testing the whole output would make our test too brittle with\n # respect to unrelated changes in the test suite's environment.\n"},{"id":"478907","messageId":"20230627161902.754472-7-sandals@crustytoothpaste.net","threadId":"59899","inReplyTo":"20230627161902.754472-1-sandals@crustytoothpaste.net","subject":"[PATCH v3 6/8] attr: expose and rename accessor functions","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2023-06-27T16:19:00Z","receivedAt":"2023-06-27T16:20:19Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"From: \"brian m. carlson\" \n\nRight now, the functions which determine the current system and global\ngitattributes files are not exposed. We'd like to use them in a future\ncommit, but they're not ideally named. Rename them to something more\nsuitable as a public interface, expose them, and document them.\n\nSigned-off-by: brian m. carlson \n---\n attr.c | 14 +++++++-------\n attr.h | 9 +++++++++\n 2 files changed, 16 insertions(+), 7 deletions(-)\n\ndiff --git a/attr.c b/attr.c\nindex d45d34058d..66203ce322 100644\n--- a/attr.c\n+++ b/attr.c\n@@ -870,7 +870,7 @@ static struct attr_stack *read_attr(struct index_state *istate,\n \treturn res;\n }\n \n-static const char *git_etc_gitattributes(void)\n+const char *git_attr_system_file(void)\n {\n \tstatic const char *system_wide;\n \tif (!system_wide)\n@@ -878,7 +878,7 @@ static const char *git_etc_gitattributes(void)\n \treturn system_wide;\n }\n \n-static const char *get_home_gitattributes(void)\n+const char *git_attr_global_file(void)\n {\n \tif (!git_attributes_file)\n \t\tgit_attributes_file = xdg_config_home(\"attributes\");\n@@ -886,7 +886,7 @@ static const char *get_home_gitattributes(void)\n \treturn git_attributes_file;\n }\n \n-static int git_attr_system(void)\n+int git_attr_system_is_enabled(void)\n {\n \treturn !git_env_bool(\"GIT_ATTR_NOSYSTEM\", 0);\n }\n@@ -920,14 +920,14 @@ static void bootstrap_attr_stack(struct index_state *istate,\n \tpush_stack(stack, e, NULL, 0);\n \n \t/* system-wide frame */\n-\tif (git_attr_system()) {\n-\t\te = read_attr_from_file(git_etc_gitattributes(), flags);\n+\tif (git_attr_system_is_enabled()) {\n+\t\te = read_attr_from_file(git_attr_system_file(), flags);\n \t\tpush_stack(stack, e, NULL, 0);\n \t}\n \n \t/* home directory */\n-\tif (get_home_gitattributes()) {\n-\t\te = read_attr_from_file(get_home_gitattributes(), flags);\n+\tif (git_attr_global_file()) {\n+\t\te = read_attr_from_file(git_attr_global_file(), flags);\n \t\tpush_stack(stack, e, NULL, 0);\n \t}\n \ndiff --git a/attr.h b/attr.h\nindex 676bd17ce2..2b745df405 100644\n--- a/attr.h\n+++ b/attr.h\n@@ -227,4 +227,13 @@ void git_attr_set_direction(enum git_attr_direction new_direction);\n \n void attr_start(void);\n \n+/* Return the system gitattributes file. */\n+const char *git_attr_system_file(void);\n+\n+/* Return the global gitattributes file, if any. */\n+const char *git_attr_global_file(void);\n+\n+/* Return whether the system gitattributes file is enabled and should be used. */\n+int git_attr_system_is_enabled(void);\n+\n #endif /* ATTR_H */\n"},{"id":"478913","messageId":"xmqqzg4khjq0.fsf@gitster.g","threadId":"59899","inReplyTo":"ZJsKa6ZNq7nnh0gf@tapette.crustytoothpaste.net","subject":"Re: [PATCH v2 6/7] var: add attributes files locations","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-06-27T17:56:55Z","receivedAt":"2023-06-27T17:57:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"brian m. carlson\" writes:\n\n>> So I dunno. I'm mostly giving a heads-up, and seeing if you (or other\n>> reviewers in the thread) have an idea to make this \"flag\" thing less\n>> awful. I'm also happy to just do my topic separately, and then\n>> eventually circle back after yours is merged.\n>\n> I've picked up your patch as the first patch in the series and will send\n> it out in v3 in just a few minutes. Since I plan to have v3 be the last\n> round of this series, I'll let you send out any further changes as\n> fixups on top of that.\n\nSounds like a sensible way to go, at least to me. \n\nThanks for working well together. I wish all conflicting topics\nworked nicely with each other like these ;-)\n\n"},{"id":"478930","messageId":"20230627201636.GA1358527@coredump.intra.peff.net","threadId":"59899","inReplyTo":"ZJsKa6ZNq7nnh0gf@tapette.crustytoothpaste.net","subject":"Re: [PATCH v2 6/7] var: add attributes files locations","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2023-06-27T20:16:36Z","receivedAt":"2023-06-27T20:18:10Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Jun 27, 2023 at 04:12:27PM +0000, brian m. carlson wrote:\n\n> > So I dunno. I'm mostly giving a heads-up, and seeing if you (or other\n> > reviewers in the thread) have an idea to make this \"flag\" thing less\n> > awful. I'm also happy to just do my topic separately, and then\n> > eventually circle back after yours is merged.\n> \n> I've picked up your patch as the first patch in the series and will send\n> it out in v3 in just a few minutes. Since I plan to have v3 be the last\n> round of this series, I'll let you send out any further changes as\n> fixups on top of that.\n\nThanks, that is optimal from my perspective. :)\n\nThe resulting series looks good to me, both the changes you integrated\nfrom me, but also the whole thing overall. It is a little funny to stuff\nthe GIT_CONFIG_GLOBAL values into a string only to re-split them (versus\npassing around a string_list itself), but I think it's a reasonable\ncompromise given the function interface.\n\n-Peff\n"}]}