{"thread":{"id":"59828","subject":"[PATCH 0/6] [RFC] Lazy-loaded default Git config","startedAt":"2023-06-02T14:33:49Z","lastAt":"2023-06-08T18:19:23Z","messageCount":8,"participants":["Derrick Stolee via GitGitGadget","Glen Choo"],"isPatch":true,"patchVersion":1,"patchTotal":6},"messages":[{"id":"477962","messageId":"pull.1539.git.1685716420.gitgitgadget@gmail.com","threadId":"59828","inReplyTo":null,"subject":"[PATCH 0/6] [RFC] Lazy-loaded default Git config","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2023-06-02T14:33:34Z","receivedAt":"2023-06-02T14:33:49Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"While working on the stronger protections around replace refs config [1], I\nwanted to think about a way to scale that concept of ensuring that we've\nread the config before accessing the global to a wider scope of config keys.\n\n[1]\nhttps://lore.kernel.org/git/pull.1537.v2.git.1685716157.gitgitgadget@gmail.com/\n\nIf you wish to apply these patches, then do so on top of v2 of [1].\n\nThis RFC attempt to explore this space, to get a feeling whether this is\nsolving a worthwhile problem and what this system should look like.\n\nOne example option is presented here, but I'm not super-confident in it and\nonly implemented enough translations such that I could gather enough\nexamples to know this would work at least a little bit.\n\nThe basic idea is to create a new 'global-config' library that has enums for\ndifferent config keys, and from each enum value we can load the config\nstring and the stored global value, as well as access an indicator that the\nvalue has been loaded from config yet.\n\nIn this way, we can guard accesses to the global state through a method\nwhich will lazy-load the config value as needed. This avoids the kind of bug\nwhere a builtin forgets to initialize config through a git_config() call at\nthe right time.\n\nOne of the issues that becomes particularly important is that some of our\nglobals are accessed before config is available. Specifically, in patch 6,\nwe replace the ignore_case global with the method accessor. This is used in\nfspathcmp() to toggle between strcasecmp() and strcmp(). However, this\nmethod is called as part of the process to set up the Git directory, which\nmust happen before we can start loading config. The current behavior is to\nalways use the default of 0 at this time.\n\nTo get around this, I introduce a declare_config_available() method which is\ncalled at the right time in git.c and test-tool.c. Before this is called, we\nwill silently pass the default value instead of loading from config. Without\nthis behavior, our test suite would fail in a BUG() statement due to the\ninconsistent use of the config library.\n\nMy hope with submitting this RFC is that we could come to some conclusions\non these questions:\n\n * Is this a worthwhile category of bug to protect against?\n * Is wrapping global state in accessor methods the right way to protect\n   against that class of bugs?\n * Are there other benefits to doing this?\n * Are there reasons why this cure is worse than the disease?\n\nThere are a few things that I personally find could use improvement and I\nwould like to update before submitting this for full review. (I chose to not\nspend extra time re-working this until there is agreement on the general\ndirection.) Here are the things I've identified so far:\n\n 1. The (static) global state in global-config.c is split across two arrays\n    and a bitmask. It would be better to have a single array of structs so\n    we can have the default values described right next to the config key\n    strings.\n 2. The current accessor method is limited to returning ints, and is named\n    as such. But many of the keys it replace are actually boolean-valued.\n    The current implementation tries ...get_maybe_bool() before then trying\n    ...get_int(), which would extend values like core.filemode to allow\n    taking the value \"3\". This could be fixed by using the struct approach\n    and signalling that some int-typed globals only allow boolean values.\n    (This holds the same for int-valued things that shouldn't take \"true\".)\n 3. The method is currently scoped globally, and does not allow for\n    repository-scoped values. None of these globals are currently scoped to\n    repositories, but it would be less intrusive to do repository-scoping\n    later if the accessor method allowed repository scoping and then looked\n    at the key-specific settings as to whether we should look globally or in\n    a repository.\n 4. The replacement of a constant like 'ignore_case' with a method like\n    'get_int_config_global(INT_CONFIG_IGNORE_CASE)' is quite verbose and\n    ugly. I specifically chose this approach instead of dedicated methods\n    like 'replace_refs_enabled()' in [1] because I didn't think that\n    approach would scale. However, we could perhaps use macros as a way to\n    reduce the verbosity, especially when deciding to add repository-scoping\n    as necessary. For instance, we could define 'get_ignore_case()' to be\n    'get_int_config_global(the_repository, INT_CONFIG_IGNORE_CASE)' in\n    global-config.h to avoid spreading this verbosity across the codebase.\n    Alternatively, we could define these as inline methods if that is\n    preferred.\n\nThank you for your help determining the right way forward here.\n\nThanks, -Stolee\n\nDerrick Stolee (6):\n  config: create new global config helpers\n  config: add trust_executable_bit to global config\n  config: move trust_ctime to global config\n  config: move quote_path_fully to global config\n  config: move has_symlinks to global config\n  config: move ignore_case to global config\n\n Makefile                                |  1 +\n apply.c                                 |  6 ++-\n builtin/difftool.c                      |  4 +-\n builtin/mv.c                            |  2 +-\n cache.h                                 |  9 ++++-\n combine-diff.c                          |  2 +-\n config.c                                | 24 -----------\n dir.c                                   | 23 ++++++-----\n entry.c                                 |  3 +-\n environment.c                           |  4 --\n environment.h                           |  4 --\n git.c                                   |  4 ++\n global-config.c                         | 54 +++++++++++++++++++++++++\n global-config.h                         | 30 ++++++++++++++\n merge-recursive.c                       | 11 ++---\n name-hash.c                             |  6 +--\n quote.c                                 |  6 +--\n quote.h                                 |  2 -\n read-cache.c                            | 22 +++++-----\n t/helper/test-lazy-init-name-hash.c     |  5 ---\n t/helper/test-tool.c                    |  3 ++\n t/perf/p0004-lazy-init-name-hash.sh     |  6 +++\n t/t3008-ls-files-lazy-init-name-hash.sh |  1 +\n unpack-trees.c                          |  2 +-\n 24 files changed, 156 insertions(+), 78 deletions(-)\n create mode 100644 global-config.c\n create mode 100644 global-config.h\n\n\nbase-commit: 4c7dbeb8c6dd6ab4c1903196967d5e0906a293c2\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1539%2Fderrickstolee%2Fdefault-config-safety-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1539/derrickstolee/default-config-safety-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/1539\n-- \ngitgitgadget\n"},{"id":"477963","messageId":"a42dd9397d07b2dc4a0d7e75bfe1af2e46cad262.1685716420.git.gitgitgadget@gmail.com","threadId":"59828","inReplyTo":"pull.1539.git.1685716420.gitgitgadget@gmail.com","subject":"[PATCH 1/6] config: create new global config helpers","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2023-06-02T14:33:35Z","receivedAt":"2023-06-02T14:33:50Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee <derrickstolee@github.com>\n\nGit's default config is loaded by each builtin at some point during its\ncmd_*() method. If this is forgotten, then Git can behave strangely\ncompared to user expectations.\n\nTo avoid this kind of bug, create a new system for loading common\n\"global\" config (config not currently scoped to a repository struct).\n\nThe basic idea is that we will add config values one-by-one to the\nint_config_key enum (and non-int values can follow a similar pattern\nlater).\n\nTo access the value, a consumer calls get_int_config_global() with the\nappropriate enum value. This method will check if the config has been\nloaded already (using the 64-bit-for-now global_ints_initialized\nbitmask) and decide whether to use the stored value or to load a value\nfrom config.\n\nWhile this method is not currently used by any consumer, there are cases\nwhere some of our default config settings are looked up by global\nvariable before Git has a chance to load config. This cannot be helped,\ndue to paths not being initialized at that point in time. We could\nremove the dependencies on those values, but it would require changing\nthings in some difficult ways or lead to unnecessary duplication across\nmethods.\n\nFor now, create the declare_config_available() method, which is called\nin cmd_main() when it is appropriate to \"unlock\" looking up these values\nfrom config. Before this method is called, get_int_config_global() will\nreturn the default value.\n\nSigned-off-by: Derrick Stolee <derrickstolee@github.com>\n---\n Makefile             |  1 +\n git.c                |  4 ++++\n global-config.c      | 44 ++++++++++++++++++++++++++++++++++++++++++++\n global-config.h      | 25 +++++++++++++++++++++++++\n t/helper/test-tool.c |  3 +++\n 5 files changed, 77 insertions(+)\n create mode 100644 global-config.c\n create mode 100644 global-config.h\n\ndiff --git a/Makefile b/Makefile\nindex e440728c246..d088589d818 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -1033,6 +1033,7 @@ LIB_OBJS += fsmonitor-ipc.o\n LIB_OBJS += fsmonitor-settings.o\n LIB_OBJS += gettext.o\n LIB_OBJS += git-zlib.o\n+LIB_OBJS += global-config.o\n LIB_OBJS += gpg-interface.o\n LIB_OBJS += graph.o\n LIB_OBJS += grep.o\ndiff --git a/git.c b/git.c\nindex 3252d4c7661..72632e659d0 100644\n--- a/git.c\n+++ b/git.c\n@@ -12,6 +12,7 @@\n #include \"shallow.h\"\n #include \"trace.h\"\n #include \"trace2.h\"\n+#include \"global-config.h\"\n \n #define RUN_SETUP\t\t(1<<0)\n #define RUN_SETUP_GENTLY\t(1<<1)\n@@ -443,6 +444,9 @@ static int run_builtin(struct cmd_struct *p, int argc, const char **argv)\n \tif (!help && p->option & NEED_WORK_TREE)\n \t\tsetup_work_tree();\n \n+\t/* At this point, we can allow loading config. */\n+\tdeclare_config_available();\n+\n \ttrace_argv_printf(argv, \"trace: built-in: git\");\n \ttrace2_cmd_name(p->cmd);\n \ttrace2_cmd_list_config();\ndiff --git a/global-config.c b/global-config.c\nnew file mode 100644\nindex 00000000000..db9643afd7a\n--- /dev/null\n+++ b/global-config.c\n@@ -0,0 +1,44 @@\n+#include \"git-compat-util.h\"\n+#include \"global-config.h\"\n+#include \"config.h\"\n+\n+static int global_ints[] = {\n+\t[INT_CONFIG_NONE] = 0, /* unused*/\n+};\n+\n+/* Bitmask for the enum. */\n+static uint64_t global_ints_initialized;\n+\n+static const char *global_int_names[] = {\n+\t[INT_CONFIG_NONE] = NULL, /* unused*/\n+};\n+\n+static int config_available;\n+\n+void declare_config_available(void)\n+{\n+\tconfig_available = 1;\n+}\n+\n+int get_int_config_global(enum int_config_key key)\n+{\n+\tuint64_t key_index;\n+\n+\tif (key < 0 || key >= sizeof(global_ints))\n+\t\tBUG(\"invalid int_config_key %d\", key);\n+\n+\tkey_index = (uint64_t)1 << key;\n+\n+\t/*\n+\t * Is it too early to load from config?\n+\t * Have we already loaded from config?\n+\t */\n+\tif (!config_available || (global_ints_initialized & key_index))\n+\t\treturn global_ints[key];\n+\tglobal_ints_initialized |= key_index;\n+\n+\t/* Try getting a boolean value before trying an int. */\n+\tif (git_config_get_maybe_bool(global_int_names[key], &global_ints[key]) < 0)\n+\t\tgit_config_get_int(global_int_names[key], &global_ints[key]);\n+\treturn global_ints[key];\n+}\ndiff --git a/global-config.h b/global-config.h\nnew file mode 100644\nindex 00000000000..407dff19ee9\n--- /dev/null\n+++ b/global-config.h\n@@ -0,0 +1,25 @@\n+#ifndef GLOBAL_CONFIG_H\n+#define GLOBAL_CONFIG_H\n+\n+enum int_config_key {\n+\tINT_CONFIG_NONE = 0,\n+};\n+\n+/**\n+ * During initial process loading, the config system is not quite available.\n+ * The config global system needs an indicator that the process is ready\n+ * to read config. Before this method is called, it will return the\n+ * default values.\n+ */\n+void declare_config_available(void);\n+\n+/**\n+ * Given a config key (by enum), return its value.\n+ *\n+ * If declare_config_available() has not been called, then this returns\n+ * the default value. Otherwise, it guarantees that the value has been\n+ * filled from config before returning the value.\n+ */\n+int get_int_config_global(enum int_config_key key);\n+\n+#endif /* GLOBAL_CONFIG_H */\ndiff --git a/t/helper/test-tool.c b/t/helper/test-tool.c\nindex abe8a785eb6..36340d36307 100644\n--- a/t/helper/test-tool.c\n+++ b/t/helper/test-tool.c\n@@ -3,6 +3,7 @@\n #include \"test-tool-utils.h\"\n #include \"trace2.h\"\n #include \"parse-options.h\"\n+#include \"global-config.h\"\n \n static const char * const test_tool_usage[] = {\n \t\"test-tool [-C <directory>] <command [<arguments>...]]\",\n@@ -127,6 +128,8 @@ int cmd_main(int argc, const char **argv)\n \tif (working_directory && chdir(working_directory) < 0)\n \t\tdie(\"Could not cd to '%s'\", working_directory);\n \n+\tdeclare_config_available();\n+\n \tfor (i = 0; i < ARRAY_SIZE(cmds); i++) {\n \t\tif (!strcmp(cmds[i].name, argv[1])) {\n \t\t\targv++;\n-- \ngitgitgadget\n\n"},{"id":"477964","messageId":"837b5360b40f992351f489a0ae05fedf49884c6e.1685716420.git.gitgitgadget@gmail.com","threadId":"59828","inReplyTo":"pull.1539.git.1685716420.gitgitgadget@gmail.com","subject":"[PATCH 2/6] config: add trust_executable_bit to global config","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2023-06-02T14:33:36Z","receivedAt":"2023-06-02T14:33:52Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee <derrickstolee@github.com>\n\nThe 'trust_executable_bit' global represents the core.filemode config\nvalue. It is loaded when git_default_config() is used to iterate over\nthe config keys (which includes several other similar iterators) during\nthe start of a Git process. The timing of this load depends on each\nprocess, and happens even if the global is never used.\n\nInstead, use the new global config system to load it on-demand, if\nneeded. Since this is the first use of the system, let's describe the\nprocess:\n\n 1. Create an enum value representing the value. This is named based on\n    the fact that we expect an 'int' type.\n\n 2. Update the default to 1 to match the existing global.\n\n 3. Update the config name so we can load it from the config set.\n\n 4. Replace uses of the global with get_int_config_global().\n\n 5. Remove the global from environment.(c|h).\n\nThe \"This needs a better name\" comment has been around since 17712991a59\n(Add \".git/config\" file parser, 2005-10-10), and in that original\ncontext it clearly mentions \"core.filemode\" as the bad name, not the\ncontaining method. Now that we are removing this global assignment (and\nnot changing the name any time soon) we delete the comment, too.\n\nSigned-off-by: Derrick Stolee <derrickstolee@github.com>\n---\n cache.h         | 7 ++++++-\n config.c        | 5 -----\n environment.c   | 1 -\n environment.h   | 1 -\n global-config.c | 2 ++\n global-config.h | 1 +\n read-cache.c    | 5 +++--\n 7 files changed, 12 insertions(+), 10 deletions(-)\n\ndiff --git a/cache.h b/cache.h\nindex 71e2fe74c4f..3e737d12ea8 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -9,6 +9,7 @@\n #include \"pathspec.h\"\n #include \"object.h\"\n #include \"statinfo.h\"\n+#include \"global-config.h\"\n \n #if defined(DT_UNKNOWN) && !defined(NO_D_TYPE_IN_DIRENT)\n #define DTYPE(de)\t((de)->d_type)\n@@ -160,10 +161,14 @@ static inline unsigned create_ce_flags(unsigned stage)\n static inline unsigned int ce_mode_from_stat(const struct cache_entry *ce,\n \t\t\t\t\t     unsigned int mode)\n {\n-\textern int trust_executable_bit, has_symlinks;\n+\textern int has_symlinks;\n+\tint trust_executable_bit;\n+\n \tif (!has_symlinks && S_ISREG(mode) &&\n \t    ce && S_ISLNK(ce->ce_mode))\n \t\treturn ce->ce_mode;\n+\n+\ttrust_executable_bit = get_int_config_global(INT_CONFIG_TRUST_EXECUTABLE_BIT);\n \tif (!trust_executable_bit && S_ISREG(mode)) {\n \t\tif (ce && S_ISREG(ce->ce_mode))\n \t\t\treturn ce->ce_mode;\ndiff --git a/config.c b/config.c\nindex d0ce902af39..ffe6bea80a6 100644\n--- a/config.c\n+++ b/config.c\n@@ -1561,11 +1561,6 @@ int git_config_color(char *dest, const char *var, const char *value)\n \n static int git_default_core_config(const char *var, const char *value, void *cb)\n {\n-\t/* This needs a better name */\n-\tif (!strcmp(var, \"core.filemode\")) {\n-\t\ttrust_executable_bit = git_config_bool(var, value);\n-\t\treturn 0;\n-\t}\n \tif (!strcmp(var, \"core.trustctime\")) {\n \t\ttrust_ctime = git_config_bool(var, value);\n \t\treturn 0;\ndiff --git a/environment.c b/environment.c\nindex e198b48081a..890705f0f69 100644\n--- a/environment.c\n+++ b/environment.c\n@@ -30,7 +30,6 @@\n #include \"wrapper.h\"\n #include \"write-or-die.h\"\n \n-int trust_executable_bit = 1;\n int trust_ctime = 1;\n int check_stat = 1;\n int has_symlinks = 1;\ndiff --git a/environment.h b/environment.h\nindex a63f0c6a24f..66d88612085 100644\n--- a/environment.h\n+++ b/environment.h\n@@ -111,7 +111,6 @@ void set_git_work_tree(const char *tree);\n #define ALTERNATE_DB_ENVIRONMENT \"GIT_ALTERNATE_OBJECT_DIRECTORIES\"\n \n /* Environment bits from configuration mechanism */\n-extern int trust_executable_bit;\n extern int trust_ctime;\n extern int check_stat;\n extern int has_symlinks;\ndiff --git a/global-config.c b/global-config.c\nindex db9643afd7a..b6fd16b14f6 100644\n--- a/global-config.c\n+++ b/global-config.c\n@@ -4,6 +4,7 @@\n \n static int global_ints[] = {\n \t[INT_CONFIG_NONE] = 0, /* unused*/\n+\t[INT_CONFIG_TRUST_EXECUTABLE_BIT] = 1,\n };\n \n /* Bitmask for the enum. */\n@@ -11,6 +12,7 @@ static uint64_t global_ints_initialized;\n \n static const char *global_int_names[] = {\n \t[INT_CONFIG_NONE] = NULL, /* unused*/\n+\t[INT_CONFIG_TRUST_EXECUTABLE_BIT] = \"core.filemode\",\n };\n \n static int config_available;\ndiff --git a/global-config.h b/global-config.h\nindex 407dff19ee9..e66c3f4ac77 100644\n--- a/global-config.h\n+++ b/global-config.h\n@@ -3,6 +3,7 @@\n \n enum int_config_key {\n \tINT_CONFIG_NONE = 0,\n+\tINT_CONFIG_TRUST_EXECUTABLE_BIT,\n };\n \n /**\ndiff --git a/read-cache.c b/read-cache.c\nindex e919af3c77a..8d14a029ce4 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -336,7 +336,7 @@ static int ce_match_stat_basic(const struct cache_entry *ce, struct stat *st)\n \t\t/* We consider only the owner x bit to be relevant for\n \t\t * \"mode changes\"\n \t\t */\n-\t\tif (trust_executable_bit &&\n+\t\tif (get_int_config_global(INT_CONFIG_TRUST_EXECUTABLE_BIT) &&\n \t\t    (0100 & (ce->ce_mode ^ st->st_mode)))\n \t\t\tchanged |= MODE_CHANGED;\n \t\tbreak;\n@@ -806,7 +806,8 @@ int add_to_index(struct index_state *istate, const char *path, struct stat *st,\n \t\tce->ce_flags |= CE_INTENT_TO_ADD;\n \n \n-\tif (trust_executable_bit && has_symlinks) {\n+\tif (get_int_config_global(INT_CONFIG_TRUST_EXECUTABLE_BIT) &&\n+\t    has_symlinks) {\n \t\tce->ce_mode = create_ce_mode(st_mode);\n \t} else {\n \t\t/* If there is an existing entry, pick the mode bits and type\n-- \ngitgitgadget\n\n"},{"id":"477965","messageId":"d03a79f6f65758b8226d64e5d20d94f789f58919.1685716420.git.gitgitgadget@gmail.com","threadId":"59828","inReplyTo":"pull.1539.git.1685716420.gitgitgadget@gmail.com","subject":"[PATCH 3/6] config: move trust_ctime to global config","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2023-06-02T14:33:37Z","receivedAt":"2023-06-02T14:33:54Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee <derrickstolee@github.com>\n\nSigned-off-by: Derrick Stolee <derrickstolee@github.com>\n---\n config.c        | 4 ----\n environment.c   | 1 -\n environment.h   | 1 -\n global-config.c | 2 ++\n global-config.h | 1 +\n read-cache.c    | 6 ++++--\n 6 files changed, 7 insertions(+), 8 deletions(-)\n\ndiff --git a/config.c b/config.c\nindex ffe6bea80a6..d955a16b372 100644\n--- a/config.c\n+++ b/config.c\n@@ -1561,10 +1561,6 @@ int git_config_color(char *dest, const char *var, const char *value)\n \n static int git_default_core_config(const char *var, const char *value, void *cb)\n {\n-\tif (!strcmp(var, \"core.trustctime\")) {\n-\t\ttrust_ctime = git_config_bool(var, value);\n-\t\treturn 0;\n-\t}\n \tif (!strcmp(var, \"core.checkstat\")) {\n \t\tif (!strcasecmp(value, \"default\"))\n \t\t\tcheck_stat = 1;\ndiff --git a/environment.c b/environment.c\nindex 890705f0f69..f42f79f7f1b 100644\n--- a/environment.c\n+++ b/environment.c\n@@ -30,7 +30,6 @@\n #include \"wrapper.h\"\n #include \"write-or-die.h\"\n \n-int trust_ctime = 1;\n int check_stat = 1;\n int has_symlinks = 1;\n int minimum_abbrev = 4, default_abbrev = -1;\ndiff --git a/environment.h b/environment.h\nindex 66d88612085..362681f63e7 100644\n--- a/environment.h\n+++ b/environment.h\n@@ -111,7 +111,6 @@ void set_git_work_tree(const char *tree);\n #define ALTERNATE_DB_ENVIRONMENT \"GIT_ALTERNATE_OBJECT_DIRECTORIES\"\n \n /* Environment bits from configuration mechanism */\n-extern int trust_ctime;\n extern int check_stat;\n extern int has_symlinks;\n extern int minimum_abbrev, default_abbrev;\ndiff --git a/global-config.c b/global-config.c\nindex b6fd16b14f6..5bd6bc45418 100644\n--- a/global-config.c\n+++ b/global-config.c\n@@ -5,6 +5,7 @@\n static int global_ints[] = {\n \t[INT_CONFIG_NONE] = 0, /* unused*/\n \t[INT_CONFIG_TRUST_EXECUTABLE_BIT] = 1,\n+\t[INT_CONFIG_TRUST_CTIME] = 1,\n };\n \n /* Bitmask for the enum. */\n@@ -13,6 +14,7 @@ static uint64_t global_ints_initialized;\n static const char *global_int_names[] = {\n \t[INT_CONFIG_NONE] = NULL, /* unused*/\n \t[INT_CONFIG_TRUST_EXECUTABLE_BIT] = \"core.filemode\",\n+\t[INT_CONFIG_TRUST_CTIME] = \"core.trustctime\",\n };\n \n static int config_available;\ndiff --git a/global-config.h b/global-config.h\nindex e66c3f4ac77..a6d8e5adc4e 100644\n--- a/global-config.h\n+++ b/global-config.h\n@@ -4,6 +4,7 @@\n enum int_config_key {\n \tINT_CONFIG_NONE = 0,\n \tINT_CONFIG_TRUST_EXECUTABLE_BIT,\n+\tINT_CONFIG_TRUST_CTIME,\n };\n \n /**\ndiff --git a/read-cache.c b/read-cache.c\nindex 8d14a029ce4..9e5a6ba2c76 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -192,14 +192,16 @@ int match_stat_data(const struct stat_data *sd, struct stat *st)\n \n \tif (sd->sd_mtime.sec != (unsigned int)st->st_mtime)\n \t\tchanged |= MTIME_CHANGED;\n-\tif (trust_ctime && check_stat &&\n+\tif (get_int_config_global(INT_CONFIG_TRUST_CTIME) &&\n+\t    check_stat &&\n \t    sd->sd_ctime.sec != (unsigned int)st->st_ctime)\n \t\tchanged |= CTIME_CHANGED;\n \n #ifdef USE_NSEC\n \tif (check_stat && sd->sd_mtime.nsec != ST_MTIME_NSEC(*st))\n \t\tchanged |= MTIME_CHANGED;\n-\tif (trust_ctime && check_stat &&\n+\tif (get_int_config_global(INT_CONFIG_TRUST_CTIME) &&\n+\t    check_stat &&\n \t    sd->sd_ctime.nsec != ST_CTIME_NSEC(*st))\n \t\tchanged |= CTIME_CHANGED;\n #endif\n-- \ngitgitgadget\n\n"},{"id":"477966","messageId":"15d65d3e05d11de69e36f2a7b83c06b2727ff8f1.1685716420.git.gitgitgadget@gmail.com","threadId":"59828","inReplyTo":"pull.1539.git.1685716420.gitgitgadget@gmail.com","subject":"[PATCH 4/6] config: move quote_path_fully to global config","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2023-06-02T14:33:38Z","receivedAt":"2023-06-02T14:33:57Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee <derrickstolee@github.com>\n\nSigned-off-by: Derrick Stolee <derrickstolee@github.com>\n---\n config.c        | 5 -----\n global-config.c | 2 ++\n global-config.h | 1 +\n quote.c         | 6 +++---\n quote.h         | 2 --\n 5 files changed, 6 insertions(+), 10 deletions(-)\n\ndiff --git a/config.c b/config.c\nindex d955a16b372..6b4051a4eae 100644\n--- a/config.c\n+++ b/config.c\n@@ -1568,11 +1568,6 @@ static int git_default_core_config(const char *var, const char *value, void *cb)\n \t\t\tcheck_stat = 0;\n \t}\n \n-\tif (!strcmp(var, \"core.quotepath\")) {\n-\t\tquote_path_fully = git_config_bool(var, value);\n-\t\treturn 0;\n-\t}\n-\n \tif (!strcmp(var, \"core.symlinks\")) {\n \t\thas_symlinks = git_config_bool(var, value);\n \t\treturn 0;\ndiff --git a/global-config.c b/global-config.c\nindex 5bd6bc45418..395d21e0381 100644\n--- a/global-config.c\n+++ b/global-config.c\n@@ -6,6 +6,7 @@ static int global_ints[] = {\n \t[INT_CONFIG_NONE] = 0, /* unused*/\n \t[INT_CONFIG_TRUST_EXECUTABLE_BIT] = 1,\n \t[INT_CONFIG_TRUST_CTIME] = 1,\n+\t[INT_CONFIG_QUOTE_PATH_FULLY] = 1,\n };\n \n /* Bitmask for the enum. */\n@@ -15,6 +16,7 @@ static const char *global_int_names[] = {\n \t[INT_CONFIG_NONE] = NULL, /* unused*/\n \t[INT_CONFIG_TRUST_EXECUTABLE_BIT] = \"core.filemode\",\n \t[INT_CONFIG_TRUST_CTIME] = \"core.trustctime\",\n+\t[INT_CONFIG_QUOTE_PATH_FULLY] = \"core.quotepath\",\n };\n \n static int config_available;\ndiff --git a/global-config.h b/global-config.h\nindex a6d8e5adc4e..fbe5fccb1a1 100644\n--- a/global-config.h\n+++ b/global-config.h\n@@ -5,6 +5,7 @@ enum int_config_key {\n \tINT_CONFIG_NONE = 0,\n \tINT_CONFIG_TRUST_EXECUTABLE_BIT,\n \tINT_CONFIG_TRUST_CTIME,\n+\tINT_CONFIG_QUOTE_PATH_FULLY,\n };\n \n /**\ndiff --git a/quote.c b/quote.c\nindex 43c739671ed..25ca0473881 100644\n--- a/quote.c\n+++ b/quote.c\n@@ -4,8 +4,7 @@\n #include \"quote.h\"\n #include \"strbuf.h\"\n #include \"strvec.h\"\n-\n-int quote_path_fully = 1;\n+#include \"global-config.h\"\n \n static inline int need_bs_quote(char c)\n {\n@@ -212,7 +211,7 @@ int sq_dequote_to_strvec(char *arg, struct strvec *array)\n }\n \n /* 1 means: quote as octal\n- * 0 means: quote as octal if (quote_path_fully)\n+ * 0 means: quote as octal if core.quotePath is true\n  * -1 means: never quote\n  * c: quote as \"\\\\c\"\n  */\n@@ -233,6 +232,7 @@ static signed char const cq_lookup[256] = {\n \n static inline int cq_must_quote(char c)\n {\n+\tint quote_path_fully = get_int_config_global(INT_CONFIG_QUOTE_PATH_FULLY);\n \treturn cq_lookup[(unsigned char)c] + quote_path_fully > 0;\n }\n \ndiff --git a/quote.h b/quote.h\nindex 0300c291041..87ff458b06d 100644\n--- a/quote.h\n+++ b/quote.h\n@@ -3,8 +3,6 @@\n \n struct strbuf;\n \n-extern int quote_path_fully;\n-\n /* Help to copy the thing properly quoted for the shell safety.\n  * any single quote is replaced with '\\'', any exclamation point\n  * is replaced with '\\!', and the whole thing is enclosed in a\n-- \ngitgitgadget\n\n"},{"id":"477967","messageId":"2b4198c09cb6c04c60608d19072d419503dfe5df.1685716421.git.gitgitgadget@gmail.com","threadId":"59828","inReplyTo":"pull.1539.git.1685716420.gitgitgadget@gmail.com","subject":"[PATCH 6/6] config: move ignore_case to global config","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2023-06-02T14:33:40Z","receivedAt":"2023-06-02T14:33:59Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee <derrickstolee@github.com>\n\nThe core.ignoreCase config setting is currently stored in the\nignore_case global. Add it to the list of config globals as\nINT_CONFIG_IGNORE_CASE instead as a precaution. This allows us to load\nthe config only when needed.\n\nThere is a subtle use of force-enabling this global in 'test-tool\ninit-name-hash', since it is trying to test a feature that is only on\nwhen core.ignoreCase is true. Instead of forcing it on in-memory, adjust\nthe test scripts to set the config.\n\nSigned-off-by: Derrick Stolee <derrickstolee@github.com>\n---\n apply.c                                 |  3 ++-\n builtin/mv.c                            |  2 +-\n config.c                                |  5 -----\n dir.c                                   | 23 ++++++++++++++---------\n environment.c                           |  1 -\n environment.h                           |  1 -\n global-config.c                         |  2 ++\n global-config.h                         |  1 +\n merge-recursive.c                       |  8 ++++----\n name-hash.c                             |  6 +++---\n read-cache.c                            |  8 ++++----\n t/helper/test-lazy-init-name-hash.c     |  5 -----\n t/perf/p0004-lazy-init-name-hash.sh     |  6 ++++++\n t/t3008-ls-files-lazy-init-name-hash.sh |  1 +\n unpack-trees.c                          |  2 +-\n 15 files changed, 39 insertions(+), 35 deletions(-)\n\ndiff --git a/apply.c b/apply.c\nindex 17092d2bb12..4292d595062 100644\n--- a/apply.c\n+++ b/apply.c\n@@ -3882,7 +3882,8 @@ static int path_is_beyond_symlink_1(struct apply_state *state, struct strbuf *na\n \t\t\tstruct cache_entry *ce;\n \n \t\t\tce = index_file_exists(state->repo->index, name->buf,\n-\t\t\t\t\t       name->len, ignore_case);\n+\t\t\t\t\t       name->len,\n+\t\t\t\t\t       get_int_config_global(INT_CONFIG_IGNORE_CASE));\n \t\t\tif (ce && S_ISLNK(ce->ce_mode))\n \t\t\t\treturn 1;\n \t\t} else {\ndiff --git a/builtin/mv.c b/builtin/mv.c\nindex 32935af48e6..e7a0280ad34 100644\n--- a/builtin/mv.c\n+++ b/builtin/mv.c\n@@ -359,7 +359,7 @@ dir_check:\n \t\t\tgoto act_on_entry;\n \t\t}\n \t\tif (lstat(dst, &st) == 0 &&\n-\t\t    (!ignore_case || strcasecmp(src, dst))) {\n+\t\t    (!get_int_config_global(INT_CONFIG_IGNORE_CASE) || strcasecmp(src, dst))) {\n \t\t\tbad = _(\"destination exists\");\n \t\t\tif (force) {\n \t\t\t\t/*\ndiff --git a/config.c b/config.c\nindex e104bc704ae..4292a3be416 100644\n--- a/config.c\n+++ b/config.c\n@@ -1568,11 +1568,6 @@ static int git_default_core_config(const char *var, const char *value, void *cb)\n \t\t\tcheck_stat = 0;\n \t}\n \n-\tif (!strcmp(var, \"core.ignorecase\")) {\n-\t\tignore_case = git_config_bool(var, value);\n-\t\treturn 0;\n-\t}\n-\n \tif (!strcmp(var, \"core.attributesfile\"))\n \t\treturn git_config_pathname(&git_attributes_file, var, value);\n \ndiff --git a/dir.c b/dir.c\nindex aa840995c40..45f09bcbe4d 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -84,7 +84,8 @@ int count_slashes(const char *s)\n \n int fspathcmp(const char *a, const char *b)\n {\n-\treturn ignore_case ? strcasecmp(a, b) : strcmp(a, b);\n+\treturn get_int_config_global(INT_CONFIG_IGNORE_CASE) ?\n+\t\tstrcasecmp(a, b) : strcmp(a, b);\n }\n \n int fspatheq(const char *a, const char *b)\n@@ -94,12 +95,14 @@ int fspatheq(const char *a, const char *b)\n \n int fspathncmp(const char *a, const char *b, size_t count)\n {\n-\treturn ignore_case ? strncasecmp(a, b, count) : strncmp(a, b, count);\n+\treturn get_int_config_global(INT_CONFIG_IGNORE_CASE) ?\n+\t\tstrncasecmp(a, b, count) : strncmp(a, b, count);\n }\n \n unsigned int fspathhash(const char *str)\n {\n-\treturn ignore_case ? strihash(str) : strhash(str);\n+\treturn get_int_config_global(INT_CONFIG_IGNORE_CASE) ?\n+\t\tstrihash(str) : strhash(str);\n }\n \n int git_fnmatch(const struct pathspec_item *item,\n@@ -148,7 +151,7 @@ static int fnmatch_icase_mem(const char *pattern, int patternlen,\n \t\tuse_str = str_buf.buf;\n \t}\n \n-\tif (ignore_case)\n+\tif (get_int_config_global(INT_CONFIG_IGNORE_CASE))\n \t\tflags |= WM_CASEFOLD;\n \tmatch_status = wildmatch(use_pat, use_str, flags);\n \n@@ -1749,7 +1752,8 @@ static struct dir_entry *dir_add_name(struct dir_struct *dir,\n \t\t\t\t      struct index_state *istate,\n \t\t\t\t      const char *pathname, int len)\n {\n-\tif (index_file_exists(istate, pathname, len, ignore_case))\n+\tif (index_file_exists(istate, pathname, len,\n+\t\t\t      get_int_config_global(INT_CONFIG_IGNORE_CASE)))\n \t\treturn NULL;\n \n \tALLOC_GROW(dir->entries, dir->nr+1, dir->internal.alloc);\n@@ -1786,7 +1790,8 @@ static enum exist_status directory_exists_in_index_icase(struct index_state *ist\n \tif (index_dir_exists(istate, dirname, len))\n \t\treturn index_directory;\n \n-\tce = index_file_exists(istate, dirname, len, ignore_case);\n+\tce = index_file_exists(istate, dirname, len,\n+\t\t\t       get_int_config_global(INT_CONFIG_IGNORE_CASE));\n \tif (ce && S_ISGITLINK(ce->ce_mode))\n \t\treturn index_gitdir;\n \n@@ -1805,7 +1810,7 @@ static enum exist_status directory_exists_in_index(struct index_state *istate,\n {\n \tint pos;\n \n-\tif (ignore_case)\n+\tif (get_int_config_global(INT_CONFIG_IGNORE_CASE))\n \t\treturn directory_exists_in_index_icase(istate, dirname, len);\n \n \tpos = index_name_pos(istate, dirname, len);\n@@ -2313,7 +2318,7 @@ static enum path_treatment treat_path(struct dir_struct *dir,\n \n \t/* Always exclude indexed files */\n \thas_path_in_index = !!index_file_exists(istate, path->buf, path->len,\n-\t\t\t\t\t\tignore_case);\n+\t\t\t\t\t\tget_int_config_global(INT_CONFIG_IGNORE_CASE));\n \tif (dtype != DT_DIR && has_path_in_index)\n \t\treturn path_none;\n \n@@ -3067,7 +3072,7 @@ static int cmp_icase(char a, char b)\n {\n \tif (a == b)\n \t\treturn 0;\n-\tif (ignore_case)\n+\tif (get_int_config_global(INT_CONFIG_IGNORE_CASE))\n \t\treturn toupper(a) - toupper(b);\n \treturn a - b;\n }\ndiff --git a/environment.c b/environment.c\nindex 312e006feb0..44cbaa3e0f9 100644\n--- a/environment.c\n+++ b/environment.c\n@@ -32,7 +32,6 @@\n \n int check_stat = 1;\n int minimum_abbrev = 4, default_abbrev = -1;\n-int ignore_case;\n int assume_unchanged;\n int prefer_symlink_refs;\n int is_bare_repository_cfg = -1; /* unspecified */\ndiff --git a/environment.h b/environment.h\nindex fb2cfa9c1aa..d77f4b233f8 100644\n--- a/environment.h\n+++ b/environment.h\n@@ -113,7 +113,6 @@ void set_git_work_tree(const char *tree);\n /* Environment bits from configuration mechanism */\n extern int check_stat;\n extern int minimum_abbrev, default_abbrev;\n-extern int ignore_case;\n extern int assume_unchanged;\n extern int prefer_symlink_refs;\n extern int warn_ambiguous_refs;\ndiff --git a/global-config.c b/global-config.c\nindex 526ced5b24c..779b894cf07 100644\n--- a/global-config.c\n+++ b/global-config.c\n@@ -8,6 +8,7 @@ static int global_ints[] = {\n \t[INT_CONFIG_TRUST_CTIME] = 1,\n \t[INT_CONFIG_QUOTE_PATH_FULLY] = 1,\n \t[INT_CONFIG_HAS_SYMLINKS] = 1,\n+\t[INT_CONFIG_IGNORE_CASE] = 0,\n };\n \n /* Bitmask for the enum. */\n@@ -19,6 +20,7 @@ static const char *global_int_names[] = {\n \t[INT_CONFIG_TRUST_CTIME] = \"core.trustctime\",\n \t[INT_CONFIG_QUOTE_PATH_FULLY] = \"core.quotepath\",\n \t[INT_CONFIG_HAS_SYMLINKS] = \"core.symlinks\",\n+\t[INT_CONFIG_IGNORE_CASE] = \"core.ignorecase\",\n };\n \n static int config_available;\ndiff --git a/global-config.h b/global-config.h\nindex 2532f426e2b..cb32503fbd9 100644\n--- a/global-config.h\n+++ b/global-config.h\n@@ -7,6 +7,7 @@ enum int_config_key {\n \tINT_CONFIG_TRUST_CTIME,\n \tINT_CONFIG_QUOTE_PATH_FULLY,\n \tINT_CONFIG_HAS_SYMLINKS,\n+\tINT_CONFIG_IGNORE_CASE,\n };\n \n /**\ndiff --git a/merge-recursive.c b/merge-recursive.c\nindex 10fdd14a642..ec806be4753 100644\n--- a/merge-recursive.c\n+++ b/merge-recursive.c\n@@ -709,10 +709,9 @@ static int remove_file(struct merge_options *opt, int clean,\n \t\t\treturn -1;\n \t}\n \tif (update_working_directory) {\n-\t\tif (ignore_case) {\n+\t\tif (get_int_config_global(INT_CONFIG_IGNORE_CASE)) {\n \t\t\tstruct cache_entry *ce;\n-\t\t\tce = index_file_exists(opt->repo->index, path, strlen(path),\n-\t\t\t\t\t       ignore_case);\n+\t\t\tce = index_file_exists(opt->repo->index, path, strlen(path), 1);\n \t\t\tif (ce && ce_stage(ce) == 0 && strcmp(path, ce->name))\n \t\t\t\treturn 0;\n \t\t}\n@@ -875,7 +874,8 @@ static int was_dirty(struct merge_options *opt, const char *path)\n \t\treturn !dirty;\n \n \tce = index_file_exists(opt->priv->unpack_opts.src_index,\n-\t\t\t       path, strlen(path), ignore_case);\n+\t\t\t       path, strlen(path),\n+\t\t\t       get_int_config_global(INT_CONFIG_IGNORE_CASE));\n \tdirty = verify_uptodate(ce, &opt->priv->unpack_opts) != 0;\n \treturn dirty;\n }\ndiff --git a/name-hash.c b/name-hash.c\nindex fb13716e430..7c32e2be2e9 100644\n--- a/name-hash.c\n+++ b/name-hash.c\n@@ -119,7 +119,7 @@ static void hash_index_entry(struct index_state *istate, struct cache_entry *ce)\n \t\thashmap_add(&istate->name_hash, &ce->ent);\n \t}\n \n-\tif (ignore_case)\n+\tif (get_int_config_global(INT_CONFIG_IGNORE_CASE))\n \t\tadd_dir_entry(istate, ce);\n }\n \n@@ -200,7 +200,7 @@ static int lookup_lazy_params(struct index_state *istate)\n \t * code to build the \"istate->name_hash\".  We don't\n \t * need the complexity here.\n \t */\n-\tif (!ignore_case)\n+\tif (!get_int_config_global(INT_CONFIG_IGNORE_CASE))\n \t\treturn 0;\n \n \tnr_cpus = online_cpus();\n@@ -642,7 +642,7 @@ void remove_name_hash(struct index_state *istate, struct cache_entry *ce)\n \tce->ce_flags &= ~CE_HASHED;\n \thashmap_remove(&istate->name_hash, &ce->ent, ce);\n \n-\tif (ignore_case)\n+\tif (get_int_config_global(INT_CONFIG_IGNORE_CASE))\n \t\tremove_dir_entry(istate, ce);\n }\n \ndiff --git a/read-cache.c b/read-cache.c\nindex b80a54133f9..444b37aff63 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -828,12 +828,12 @@ int add_to_index(struct index_state *istate, const char *path, struct stat *st,\n \t * case of the file being added to the repository matches (is folded into) the existing\n \t * entry's directory case.\n \t */\n-\tif (ignore_case) {\n+\tif (get_int_config_global(INT_CONFIG_IGNORE_CASE)) {\n \t\tadjust_dirname_case(istate, ce->name);\n \t}\n \tif (!(flags & ADD_CACHE_RENORMALIZE)) {\n \t\talias = index_file_exists(istate, ce->name,\n-\t\t\t\t\t  ce_namelen(ce), ignore_case);\n+\t\t\t\t\t  ce_namelen(ce), get_int_config_global(INT_CONFIG_IGNORE_CASE));\n \t\tif (alias &&\n \t\t    !ce_stage(alias) &&\n \t\t    !ie_match_stat(istate, alias, st, ce_option)) {\n@@ -854,7 +854,7 @@ int add_to_index(struct index_state *istate, const char *path, struct stat *st,\n \t} else\n \t\tset_object_name_for_intent_to_add_entry(ce);\n \n-\tif (ignore_case && alias && different_name(ce, alias))\n+\tif (get_int_config_global(INT_CONFIG_IGNORE_CASE) && alias && different_name(ce, alias))\n \t\tce = create_alias_ce(istate, ce, alias);\n \tce->ce_flags |= CE_ADDED;\n \n@@ -1024,7 +1024,7 @@ static int verify_dotfile(const char *rest, unsigned mode)\n \tswitch (*rest) {\n \t/*\n \t * \".git\" followed by NUL or slash is bad. Note that we match\n-\t * case-insensitively here, even if ignore_case is not set.\n+\t * case-insensitively here, regardless of core.ignoreCase.\n \t * This outlaws \".GIT\" everywhere out of an abundance of caution,\n \t * since there's really no good reason to allow it.\n \t *\ndiff --git a/t/helper/test-lazy-init-name-hash.c b/t/helper/test-lazy-init-name-hash.c\nindex f23d983c118..5b69f90d888 100644\n--- a/t/helper/test-lazy-init-name-hash.c\n+++ b/t/helper/test-lazy-init-name-hash.c\n@@ -212,11 +212,6 @@ int cmd__lazy_init_name_hash(int argc, const char **argv)\n \n \targc = parse_options(argc, argv, prefix, options, usage, 0);\n \n-\t/*\n-\t * istate->dir_hash is only created when ignore_case is set.\n-\t */\n-\tignore_case = 1;\n-\n \tif (dump) {\n \t\tif (perf || analyze > 0)\n \t\t\tdie(\"cannot combine dump, perf, or analyze\");\ndiff --git a/t/perf/p0004-lazy-init-name-hash.sh b/t/perf/p0004-lazy-init-name-hash.sh\nindex 85be14e4ddb..90dad259161 100755\n--- a/t/perf/p0004-lazy-init-name-hash.sh\n+++ b/t/perf/p0004-lazy-init-name-hash.sh\n@@ -6,6 +6,12 @@ test_description='Tests multi-threaded lazy_init_name_hash'\n test_perf_large_repo\n test_checkout_worktree\n \n+test_expect_success 'initialize core.ignorecase' '\n+\t# This is needed since the name-hash structure is ignored when\n+\t# core.ignorecase is false.\n+\tgit config core.ignorecase true\n+'\n+\n test_expect_success 'verify both methods build the same hashmaps' '\n \ttest-tool lazy-init-name-hash --dump --single >out.single &&\n \tif test-tool lazy-init-name-hash --dump --multi >out.multi\ndiff --git a/t/t3008-ls-files-lazy-init-name-hash.sh b/t/t3008-ls-files-lazy-init-name-hash.sh\nindex 51d3dffaa66..54bde980769 100755\n--- a/t/t3008-ls-files-lazy-init-name-hash.sh\n+++ b/t/t3008-ls-files-lazy-init-name-hash.sh\n@@ -22,6 +22,7 @@ test_expect_success 'no buffer overflow in lazy_init_name_hash' '\n \t) |\n \tsed \"s/^/100644 $EMPTY_BLOB\t/\" |\n \tgit update-index --index-info &&\n+\tgit config core.ignoreCase true &&\n \ttest-tool lazy-init-name-hash -m\n '\n \ndiff --git a/unpack-trees.c b/unpack-trees.c\nindex c0732aa0c2d..b678745e699 100644\n--- a/unpack-trees.c\n+++ b/unpack-trees.c\n@@ -2405,7 +2405,7 @@ static int check_ok_to_remove(const char *name, int len, int dtype,\n \t *\n \t * Ignore that lstat() if it matches.\n \t */\n-\tif (ignore_case && icase_exists(o, name, len, st))\n+\tif (get_int_config_global(INT_CONFIG_IGNORE_CASE) && icase_exists(o, name, len, st))\n \t\treturn 0;\n \n \tif (o->internal.dir &&\n-- \ngitgitgadget\n"},{"id":"477968","messageId":"a154008619790f7a60f2bba91db7b0fe29e67e1a.1685716420.git.gitgitgadget@gmail.com","threadId":"59828","inReplyTo":"pull.1539.git.1685716420.gitgitgadget@gmail.com","subject":"[PATCH 5/6] config: move has_symlinks to global config","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2023-06-02T14:33:39Z","receivedAt":"2023-06-02T14:34:01Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee <derrickstolee@github.com>\n\nSigned-off-by: Derrick Stolee <derrickstolee@github.com>\n---\n apply.c            | 3 ++-\n builtin/difftool.c | 4 ++--\n cache.h            | 4 ++--\n combine-diff.c     | 2 +-\n config.c           | 5 -----\n entry.c            | 3 ++-\n environment.c      | 1 -\n environment.h      | 1 -\n global-config.c    | 2 ++\n global-config.h    | 1 +\n merge-recursive.c  | 3 ++-\n read-cache.c       | 5 +++--\n 12 files changed, 17 insertions(+), 17 deletions(-)\n\ndiff --git a/apply.c b/apply.c\nindex 3636bc14c2d..17092d2bb12 100644\n--- a/apply.c\n+++ b/apply.c\n@@ -4384,7 +4384,8 @@ static int try_create_file(struct apply_state *state, const char *path,\n \t\treturn !!mkdir(path, 0777);\n \t}\n \n-\tif (has_symlinks && S_ISLNK(mode))\n+\tif (get_int_config_global(INT_CONFIG_HAS_SYMLINKS) &&\n+\t    S_ISLNK(mode))\n \t\t/* Although buf:size is counted string, it also is NUL\n \t\t * terminated.\n \t\t */\ndiff --git a/builtin/difftool.c b/builtin/difftool.c\nindex f09d24d37f9..f51c9f71d5d 100644\n--- a/builtin/difftool.c\n+++ b/builtin/difftool.c\n@@ -292,7 +292,7 @@ static char *get_symlink(const struct object_id *oid, const char *path)\n \tif (is_null_oid(oid)) {\n \t\t/* The symlink is unknown to Git so read from the filesystem */\n \t\tstruct strbuf link = STRBUF_INIT;\n-\t\tif (has_symlinks) {\n+\t\tif (get_int_config_global(INT_CONFIG_HAS_SYMLINKS)) {\n \t\t\tif (strbuf_readlink(&link, path, strlen(path)))\n \t\t\t\tdie(_(\"could not read symlink %s\"), path);\n \t\t} else if (strbuf_read_file(&link, path, 128))\n@@ -723,7 +723,7 @@ int cmd_difftool(int argc, const char **argv, const char *prefix)\n \tstruct child_process child = CHILD_PROCESS_INIT;\n \n \tgit_config(difftool_config, NULL);\n-\tsymlinks = has_symlinks;\n+\tsymlinks = get_int_config_global(INT_CONFIG_HAS_SYMLINKS);\n \n \targc = parse_options(argc, argv, prefix, builtin_difftool_options,\n \t\t\t     builtin_difftool_usage, PARSE_OPT_KEEP_UNKNOWN_OPT |\ndiff --git a/cache.h b/cache.h\nindex 3e737d12ea8..4d270e114a2 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -161,10 +161,10 @@ static inline unsigned create_ce_flags(unsigned stage)\n static inline unsigned int ce_mode_from_stat(const struct cache_entry *ce,\n \t\t\t\t\t     unsigned int mode)\n {\n-\textern int has_symlinks;\n \tint trust_executable_bit;\n \n-\tif (!has_symlinks && S_ISREG(mode) &&\n+\tif (!get_int_config_global(INT_CONFIG_HAS_SYMLINKS) &&\n+\t    S_ISREG(mode) &&\n \t    ce && S_ISLNK(ce->ce_mode))\n \t\treturn ce->ce_mode;\n \ndiff --git a/combine-diff.c b/combine-diff.c\nindex f7e9fb57473..1de0a340b24 100644\n--- a/combine-diff.c\n+++ b/combine-diff.c\n@@ -1086,7 +1086,7 @@ static void show_patch_diff(struct combine_diff_path *elem, int num_parent,\n \t\t\t/* if symlinks don't work, assume symlink if all parents\n \t\t\t * are symlinks\n \t\t\t */\n-\t\t\tis_file = has_symlinks;\n+\t\t\tis_file = get_int_config_global(INT_CONFIG_HAS_SYMLINKS);\n \t\t\tfor (i = 0; !is_file && i < num_parent; i++)\n \t\t\t\tis_file = !S_ISLNK(elem->parent[i].mode);\n \t\t\tif (!is_file)\ndiff --git a/config.c b/config.c\nindex 6b4051a4eae..e104bc704ae 100644\n--- a/config.c\n+++ b/config.c\n@@ -1568,11 +1568,6 @@ static int git_default_core_config(const char *var, const char *value, void *cb)\n \t\t\tcheck_stat = 0;\n \t}\n \n-\tif (!strcmp(var, \"core.symlinks\")) {\n-\t\thas_symlinks = git_config_bool(var, value);\n-\t\treturn 0;\n-\t}\n-\n \tif (!strcmp(var, \"core.ignorecase\")) {\n \t\tignore_case = git_config_bool(var, value);\n \t\treturn 0;\ndiff --git a/entry.c b/entry.c\nindex d89e61fa641..0c33aeb8aa9 100644\n--- a/entry.c\n+++ b/entry.c\n@@ -308,7 +308,8 @@ static int write_entry(struct cache_entry *ce, char *path, struct conv_attrs *ca\n \t\t * We can't make a real symlink; write out a regular file entry\n \t\t * with the symlink destination as its contents.\n \t\t */\n-\t\tif (!has_symlinks || to_tempfile)\n+\t\tif (!get_int_config_global(INT_CONFIG_HAS_SYMLINKS) ||\n+\t\t    to_tempfile)\n \t\t\tgoto write_file_entry;\n \n \t\tret = symlink(new_blob, path);\ndiff --git a/environment.c b/environment.c\nindex f42f79f7f1b..312e006feb0 100644\n--- a/environment.c\n+++ b/environment.c\n@@ -31,7 +31,6 @@\n #include \"write-or-die.h\"\n \n int check_stat = 1;\n-int has_symlinks = 1;\n int minimum_abbrev = 4, default_abbrev = -1;\n int ignore_case;\n int assume_unchanged;\ndiff --git a/environment.h b/environment.h\nindex 362681f63e7..fb2cfa9c1aa 100644\n--- a/environment.h\n+++ b/environment.h\n@@ -112,7 +112,6 @@ void set_git_work_tree(const char *tree);\n \n /* Environment bits from configuration mechanism */\n extern int check_stat;\n-extern int has_symlinks;\n extern int minimum_abbrev, default_abbrev;\n extern int ignore_case;\n extern int assume_unchanged;\ndiff --git a/global-config.c b/global-config.c\nindex 395d21e0381..526ced5b24c 100644\n--- a/global-config.c\n+++ b/global-config.c\n@@ -7,6 +7,7 @@ static int global_ints[] = {\n \t[INT_CONFIG_TRUST_EXECUTABLE_BIT] = 1,\n \t[INT_CONFIG_TRUST_CTIME] = 1,\n \t[INT_CONFIG_QUOTE_PATH_FULLY] = 1,\n+\t[INT_CONFIG_HAS_SYMLINKS] = 1,\n };\n \n /* Bitmask for the enum. */\n@@ -17,6 +18,7 @@ static const char *global_int_names[] = {\n \t[INT_CONFIG_TRUST_EXECUTABLE_BIT] = \"core.filemode\",\n \t[INT_CONFIG_TRUST_CTIME] = \"core.trustctime\",\n \t[INT_CONFIG_QUOTE_PATH_FULLY] = \"core.quotepath\",\n+\t[INT_CONFIG_HAS_SYMLINKS] = \"core.symlinks\",\n };\n \n static int config_available;\ndiff --git a/global-config.h b/global-config.h\nindex fbe5fccb1a1..2532f426e2b 100644\n--- a/global-config.h\n+++ b/global-config.h\n@@ -6,6 +6,7 @@ enum int_config_key {\n \tINT_CONFIG_TRUST_EXECUTABLE_BIT,\n \tINT_CONFIG_TRUST_CTIME,\n \tINT_CONFIG_QUOTE_PATH_FULLY,\n+\tINT_CONFIG_HAS_SYMLINKS,\n };\n \n /**\ndiff --git a/merge-recursive.c b/merge-recursive.c\nindex 9875bdb11cb..10fdd14a642 100644\n--- a/merge-recursive.c\n+++ b/merge-recursive.c\n@@ -983,7 +983,8 @@ static int update_file_flags(struct merge_options *opt,\n \t\t\tgoto free_buf;\n \t\t}\n \t\tif (S_ISREG(contents->mode) ||\n-\t\t    (!has_symlinks && S_ISLNK(contents->mode))) {\n+\t\t    (!get_int_config_global(INT_CONFIG_HAS_SYMLINKS) &&\n+\t\t     S_ISLNK(contents->mode))) {\n \t\t\tint fd;\n \t\t\tint mode = (contents->mode & 0100 ? 0777 : 0666);\n \ndiff --git a/read-cache.c b/read-cache.c\nindex 9e5a6ba2c76..b80a54133f9 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -344,7 +344,8 @@ static int ce_match_stat_basic(const struct cache_entry *ce, struct stat *st)\n \t\tbreak;\n \tcase S_IFLNK:\n \t\tif (!S_ISLNK(st->st_mode) &&\n-\t\t    (has_symlinks || !S_ISREG(st->st_mode)))\n+\t\t    (get_int_config_global(INT_CONFIG_HAS_SYMLINKS) ||\n+\t\t     !S_ISREG(st->st_mode)))\n \t\t\tchanged |= TYPE_CHANGED;\n \t\tbreak;\n \tcase S_IFGITLINK:\n@@ -809,7 +810,7 @@ int add_to_index(struct index_state *istate, const char *path, struct stat *st,\n \n \n \tif (get_int_config_global(INT_CONFIG_TRUST_EXECUTABLE_BIT) &&\n-\t    has_symlinks) {\n+\t    get_int_config_global(INT_CONFIG_HAS_SYMLINKS)) {\n \t\tce->ce_mode = create_ce_mode(st_mode);\n \t} else {\n \t\t/* If there is an existing entry, pick the mode bits and type\n-- \ngitgitgadget\n\n"},{"id":"478172","messageId":"kl6lbkhpzujf.fsf@chooglen-macbookpro.roam.corp.google.com","threadId":"59828","inReplyTo":"pull.1539.git.1685716420.gitgitgadget@gmail.com","subject":"Re: [PATCH 0/6] [RFC] Lazy-loaded default Git config","fromName":"Glen Choo","fromEmail":"chooglen@google.com","sentAt":"2023-06-08T18:19:16Z","receivedAt":"2023-06-08T18:19:23Z","isPatch":true,"sender":{"key":"glencbz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58092771?v=4"},"body":"Hi Stolee!\n\nThanks for coming to the Libification discussion today, it was nice to\nbe able to discuss this idea with a bigger group.\n\nAs is custom, I'll repost my own thoughts here. If folks are interested\nin a summary of the whole discussion, you can find the notes at\n\n  https://docs.google.com/document/d/1mw_qPPgfQUv1gfKMwmvZjpYaOOzxcNH2N8bRbvBWfIQ/edit#heading=h.fcm7uhwlk55z\n\n\"Derrick Stolee via GitGitGadget\" <gitgitgadget@gmail.com> writes:\n\n>  * Is this a worthwhile category of bug to protect against?\n\nIf we look past the concrete bugs and consider the general problem of\nprocess-wide state being hard to use correctly, I think this is\ndefinitely worth solving.\n\nAnd in case anyone doubts the the current interface is hard to use\ncorrectly: the state lives in poorly scoped global variables that need\nto be manually initialized by calling git_config() at the right time\nwith git_default_config. It's also hard to remember to do this for your\nbuiltin because some builtins call git_default_config in _their own_\nouter config_fn_t, and others call git_config(git_default_config).\n\n>  * Is wrapping global state in accessor methods the right way to protect\n>    against that class of bugs?\n\nLazy-loading via accessor methods seem slightly excessive. The crux of\nthe problem is that we haven't initialized the values before they are\nread, so I think we can get away with plain fields in a struct as long\nas they are always initialized (e.g. the builtin author doesn't need to\nremember to do this). We could initialize all of the fields during the\nsetup process, where you placed declare_config_available(), at which\npoint config should be available.\n\nIf we use config hash lookups to intialize the values we want,\npre-initializing like this shouldn't be noticeably more wasteful\ncompared to lazy-loading, since in either case, we are already parsing\nall of the config into the in-memory config sets and looking them up\nwith hash lookups. Pre-initializing does a small bit more work upfront\nby assigning to the fields, but it means that accessing the settings\nlater can be faster since we can avoid the \"getter method\" calls.\n\n>  * Are there other benefits to doing this?\n\nYes! I'm generally excited about encapsulating loose global variables\ninto an appropriate abstraction, e.g. it was something I was thinking\nabout this when I was working on protected config (which is loose global\nstate, but also sort of a collection of repo-wide settings). It'll be\nextremely relevant for libification, since this paves the way to\neventually encapsulate the process-wide state in a way that makes it\nseparable from the library code.\n\n>  * Are there reasons why this cure is worse than the disease?\n\nThis seems quite similar to what we're doing with repo-settings.c (which\nalso has its own problems with initialization), and I'd like to avoid\nhaving two similar-looking-but-different systems in the long run. For\nthis to go forward, I'd like to see either an explicit goal to deprecate\nrepo-settings.c (e.g. this new system handles repository settings in\naddition to process-wide settings), or a reasoned separation between\nthem.\n"}]}