{"thread":{"id":"50977","subject":"[PATCH] Honor core.precomposeUnicode in more places","startedAt":"2019-04-23T17:31:04Z","lastAt":"2019-07-26T19:47:47Z","messageCount":6,"participants":["Elijah Newren","Torsten Bögershausen","Junio C Hamano","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"374321","messageId":"20190423173056.28523-1-newren@gmail.com","threadId":"50977","inReplyTo":null,"subject":"[PATCH] Honor core.precomposeUnicode in more places","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2019-04-23T17:30:56Z","receivedAt":"2019-04-23T17:31:04Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Mac's HFS (\"Hilarious FileSystem\"? \"Halfwitted FileSystem\"?) --\nwhere git sets core.precomposeUnicode to true automatically by git\ninit/clone -- when a user creates a simple unicode refname (in NFC\nformat) such as españa:\n\n $ git branch españa\n\ndifferent commands would display the branch name differently. For\nexample, git branch, git log --decorate, and git fast-export all used\n\n 65 73 70 61 c3 b1 61 (or \"espa\\xc3\\xb1a\")\n\n(NFC form) while show-ref would use\n\n 65 73 70 61 6e cc 83 61 (or \"espan\\xcc\\x83a\")\n\n(NFD form). A stress test for git filter-repo was tripped up by this\ninconsistency, though digging in I found that the problems could\ncompound; for example, if the user ran\n\n $ git pack-refs --all\n\nand then tried to check out the branch, they would be met with:\n\n $ git checkout españa\n error: pathspec 'españa' did not match any file(s) known to git\n\n $ git checkout españa --\n fatal: invalid reference: españa\n\n $ git branch\n españa\n * master\n\nNote that the user could run the `git branch` command first and copy and\npaste the `españa` portion of the output and still see the same two\nerrors. Also, if the user added --no-prune to the pack-refs command,\nthen they would see three branches: master, españa, and españa (those\nlast two are NFC vs. NFD forms, even if they render the same).\n\nFurther, if the user had the `españa` branch checked out before\nrunning `git pack-refs --all`, the user would be greeted with (note\nthat I'm trimming trailing output with an ellipsis):\n\n $ git rev-parse HEAD\n fatal: ambiguous argument 'HEAD': unknown revision or path...\n\n $ git status\n On branch españa\n\n No commits yet...\n\nOr worse, if the user didn't check this stuff first, running `git\ncommit` will create a new commit with all changes of all of history\nbeing squashed into it.\n\nIn addition to pack-refs, one could also get into this state with\nupload-pack or anything that calls either pack-refs or upload-pack (e.g.\ngc or clone).\n\nAdd code in a few places (pack-refs, show-ref, upload-pack) to check and\nhonor the setting of core.precomposeUnicode to avoid these bugs.\n\nSigned-off-by: Elijah Newren \n---\n builtin/pack-refs.c | 2 ++\n builtin/show-ref.c | 3 +++\n upload-pack.c | 2 ++\n 3 files changed, 7 insertions(+)\n\ndiff --git a/builtin/pack-refs.c b/builtin/pack-refs.c\nindex f3353564f9..cfbd5c36c7 100644\n--- a/builtin/pack-refs.c\n+++ b/builtin/pack-refs.c\n@@ -1,4 +1,5 @@\n #include \"builtin.h\"\n+#include \"config.h\"\n #include \"parse-options.h\"\n #include \"refs.h\"\n #include \"repository.h\"\n@@ -16,6 +17,7 @@ int cmd_pack_refs(int argc, const char **argv, const char *prefix)\n \t\tOPT_BIT(0, \"prune\", &flags, N_(\"prune loose refs (default)\"), PACK_REFS_PRUNE),\n \t\tOPT_END(),\n \t};\n+\tgit_config(git_default_config, NULL);\n \tif (parse_options(argc, argv, prefix, opts, pack_refs_usage, 0))\n \t\tusage_with_options(pack_refs_usage, opts);\n \treturn refs_pack_refs(get_main_ref_store(the_repository), flags);\ndiff --git a/builtin/show-ref.c b/builtin/show-ref.c\nindex 6a706c02a6..6456da70cc 100644\n--- a/builtin/show-ref.c\n+++ b/builtin/show-ref.c\n@@ -1,5 +1,6 @@\n #include \"builtin.h\"\n #include \"cache.h\"\n+#include \"config.h\"\n #include \"refs.h\"\n #include \"object-store.h\"\n #include \"object.h\"\n@@ -182,6 +183,8 @@ static const struct option show_ref_options[] = {\n \n int cmd_show_ref(int argc, const char **argv, const char *prefix)\n {\n+\tgit_config(git_default_config, NULL);\n+\n \targc = parse_options(argc, argv, prefix, show_ref_options,\n \t\t\t show_ref_usage, 0);\n \ndiff --git a/upload-pack.c b/upload-pack.c\nindex d098ef5982..159f751ea4 100644\n--- a/upload-pack.c\n+++ b/upload-pack.c\n@@ -1064,6 +1064,8 @@ static int upload_pack_config(const char *var, const char *value, void *unused)\n \t\tallow_ref_in_want = git_config_bool(var, value);\n \t} else if (!strcmp(\"uploadpack.allowsidebandall\", var)) {\n \t\tallow_sideband_all = git_config_bool(var, value);\n+\t} else if (!strcmp(\"core.precomposeunicode\", var)) {\n+\t\tprecomposed_unicode = git_config_bool(var, value);\n \t}\n \n \tif (current_config_scope() != CONFIG_SCOPE_REPO) {\n-- \n2.21.0.420.g4906d192b3\n\n"},{"id":"374323","messageId":"20190423182924.r6mkwrl2o7pcwjoa@tb-raspi4","threadId":"50977","inReplyTo":"20190423173056.28523-1-newren@gmail.com","subject":"Re: [PATCH] Honor core.precomposeUnicode in more places","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2019-04-23T18:29:24Z","receivedAt":"2019-04-23T18:29:29Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"On Tue, Apr 23, 2019 at 10:30:56AM -0700, Elijah Newren wrote:\n> On Mac's HFS (\"Hilarious FileSystem\"? \"Halfwitted FileSystem\"?) --\n\nHow about \"Hierarchical File System\" ?\n\n> where git sets core.precomposeUnicode to true automatically by git\n> init/clone -- when a user creates a simple unicode refname (in NFC\n> format) such as españa:\n>\n> $ git branch españa\n>\n> different commands would display the branch name differently. For\n> example, git branch, git log --decorate, and git fast-export all used\n>\n> 65 73 70 61 c3 b1 61 (or \"espa\\xc3\\xb1a\")\n>\n> (NFC form) while show-ref would use\n>\n> 65 73 70 61 6e cc 83 61 (or \"espan\\xcc\\x83a\")\n>\n> (NFD form). A stress test for git filter-repo was tripped up by this\n> inconsistency, though digging in I found that the problems could\n> compound; for example, if the user ran\n>\n> $ git pack-refs --all\n>\n> and then tried to check out the branch, they would be met with:\n>\n> $ git checkout españa\n> error: pathspec 'españa' did not match any file(s) known to git\n>\n> $ git checkout españa --\n> fatal: invalid reference: españa\n>\n> $ git branch\n> españa\n> * master\n>\n> Note that the user could run the `git branch` command first and copy and\n> paste the `españa` portion of the output and still see the same two\n> errors. Also, if the user added --no-prune to the pack-refs command,\n> then they would see three branches: master, españa, and españa (those\n> last two are NFC vs. NFD forms, even if they render the same).\n>\n> Further, if the user had the `españa` branch checked out before\n> running `git pack-refs --all`, the user would be greeted with (note\n> that I'm trimming trailing output with an ellipsis):\n>\n> $ git rev-parse HEAD\n> fatal: ambiguous argument 'HEAD': unknown revision or path...\n>\n> $ git status\n> On branch españa\n>\n> No commits yet...\n>\n> Or worse, if the user didn't check this stuff first, running `git\n> commit` will create a new commit with all changes of all of history\n> being squashed into it.\n>\n> In addition to pack-refs, one could also get into this state with\n> upload-pack or anything that calls either pack-refs or upload-pack (e.g.\n> gc or clone).\n>\n> Add code in a few places (pack-refs, show-ref, upload-pack) to check and\n> honor the setting of core.precomposeUnicode to avoid these bugs.\n\nThat's all correct, one minor question below.\n>\n> Signed-off-by: Elijah Newren \n> ---\n> builtin/pack-refs.c | 2 ++\n> builtin/show-ref.c | 3 +++\n> upload-pack.c | 2 ++\n> 3 files changed, 7 insertions(+)\n>\n> diff --git a/builtin/pack-refs.c b/builtin/pack-refs.c\n> index f3353564f9..cfbd5c36c7 100644\n> --- a/builtin/pack-refs.c\n> +++ b/builtin/pack-refs.c\n> @@ -1,4 +1,5 @@\n> #include \"builtin.h\"\n> +#include \"config.h\"\n> #include \"parse-options.h\"\n> #include \"refs.h\"\n> #include \"repository.h\"\n> @@ -16,6 +17,7 @@ int cmd_pack_refs(int argc, const char **argv, const char *prefix)\n> \t\tOPT_BIT(0, \"prune\", &flags, N_(\"prune loose refs (default)\"), PACK_REFS_PRUNE),\n> \t\tOPT_END(),\n> \t};\n> +\tgit_config(git_default_config, NULL);\n> \tif (parse_options(argc, argv, prefix, opts, pack_refs_usage, 0))\n> \t\tusage_with_options(pack_refs_usage, opts);\n\nI wonder if we could move the call to git_config() into parse_options(),\n(or another common place) but I haven't checked the details yet.\nSame below for show_ref().\n\nAnd thankks for picking this up.\n\n> \treturn refs_pack_refs(get_main_ref_store(the_repository), flags);\n> diff --git a/builtin/show-ref.c b/builtin/show-ref.c\n> index 6a706c02a6..6456da70cc 100644\n> --- a/builtin/show-ref.c\n> +++ b/builtin/show-ref.c\n> @@ -1,5 +1,6 @@\n> #include \"builtin.h\"\n> #include \"cache.h\"\n> +#include \"config.h\"\n> #include \"refs.h\"\n> #include \"object-store.h\"\n> #include \"object.h\"\n> @@ -182,6 +183,8 @@ static const struct option show_ref_options[] = {\n>\n> int cmd_show_ref(int argc, const char **argv, const char *prefix)\n> {\n> +\tgit_config(git_default_config, NULL);\n> +\n> \targc = parse_options(argc, argv, prefix, show_ref_options,\n> \t\t\t show_ref_usage, 0);\n>\n> diff --git a/upload-pack.c b/upload-pack.c\n> index d098ef5982..159f751ea4 100644\n> --- a/upload-pack.c\n> +++ b/upload-pack.c\n> @@ -1064,6 +1064,8 @@ static int upload_pack_config(const char *var, const char *value, void *unused)\n> \t\tallow_ref_in_want = git_config_bool(var, value);\n> \t} else if (!strcmp(\"uploadpack.allowsidebandall\", var)) {\n> \t\tallow_sideband_all = git_config_bool(var, value);\n> +\t} else if (!strcmp(\"core.precomposeunicode\", var)) {\n> +\t\tprecomposed_unicode = git_config_bool(var, value);\n> \t}\n>\n> \tif (current_config_scope() != CONFIG_SCOPE_REPO) {\n> --\n> 2.21.0.420.g4906d192b3\n>\n"},{"id":"374324","messageId":"CABPp-BGbDN2DVwJKh3gdQ1wDZmGuevAYRhhfe_MCyunABDsEqg@mail.gmail.com","threadId":"50977","inReplyTo":"20190423182924.r6mkwrl2o7pcwjoa@tb-raspi4","subject":"Re: [PATCH] Honor core.precomposeUnicode in more places","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2019-04-23T19:06:45Z","receivedAt":"2019-04-23T19:07:01Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Tue, Apr 23, 2019 at 11:29 AM Torsten Bögershausen wrote:\n>\n> On Tue, Apr 23, 2019 at 10:30:56AM -0700, Elijah Newren wrote:\n> > On Mac's HFS (\"Hilarious FileSystem\"? \"Halfwitted FileSystem\"?) --\n>\n> How about \"Hierarchical File System\" ?\n\nSorry, I should have removed my draft commentary before submitting.\nYes, you are of course right.\n\n...\n> > Add code in a few places (pack-refs, show-ref, upload-pack) to check and\n> > honor the setting of core.precomposeUnicode to avoid these bugs.\n>\n> That's all correct, one minor question below.\n...\n\n> > @@ -16,6 +17,7 @@ int cmd_pack_refs(int argc, const char **argv, const char *prefix)\n> > OPT_BIT(0, \"prune\", &flags, N_(\"prune loose refs (default)\"), PACK_REFS_PRUNE),\n> > OPT_END(),\n> > };\n> > + git_config(git_default_config, NULL);\n> > if (parse_options(argc, argv, prefix, opts, pack_refs_usage, 0))\n> > usage_with_options(pack_refs_usage, opts);\n>\n> I wonder if we could move the call to git_config() into parse_options(),\n> (or another common place) but I haven't checked the details yet.\n> Same below for show_ref().\n\nInteresting idea. However, moving it into parse_options() presumes\nthat either `git_default_config` will always be passed to\ngit_config(), or else the arguments needed by git_config() will be\nexplicitly passed to parse_options. A quick grep through the source\ncode suggests that only about 1/3 of callsites pass git_default_config\nto git_config(), and passing a config-related callback function to\nparse_options seems a little weird to me. Plus, the fact that 2/3 of\nthe callsites use a special config callback function suggests that\nwe'd still not catch all cases with such a change, much like I had to\nupdate the upload_pack_config special callback below.\n\n> And thankks for picking this up.\n>\n> > return refs_pack_refs(get_main_ref_store(the_repository), flags);\n> > diff --git a/builtin/show-ref.c b/builtin/show-ref.c\n> > index 6a706c02a6..6456da70cc 100644\n> > --- a/builtin/show-ref.c\n> > +++ b/builtin/show-ref.c\n> > @@ -1,5 +1,6 @@\n> > #include \"builtin.h\"\n> > #include \"cache.h\"\n> > +#include \"config.h\"\n> > #include \"refs.h\"\n> > #include \"object-store.h\"\n> > #include \"object.h\"\n> > @@ -182,6 +183,8 @@ static const struct option show_ref_options[] = {\n> >\n> > int cmd_show_ref(int argc, const char **argv, const char *prefix)\n> > {\n> > + git_config(git_default_config, NULL);\n> > +\n> > argc = parse_options(argc, argv, prefix, show_ref_options,\n> > show_ref_usage, 0);\n> >\n> > diff --git a/upload-pack.c b/upload-pack.c\n> > index d098ef5982..159f751ea4 100644\n> > --- a/upload-pack.c\n> > +++ b/upload-pack.c\n> > @@ -1064,6 +1064,8 @@ static int upload_pack_config(const char *var, const char *value, void *unused)\n> > allow_ref_in_want = git_config_bool(var, value);\n> > } else if (!strcmp(\"uploadpack.allowsidebandall\", var)) {\n> > allow_sideband_all = git_config_bool(var, value);\n> > + } else if (!strcmp(\"core.precomposeunicode\", var)) {\n> > + precomposed_unicode = git_config_bool(var, value);\n> > }\n> >\n> > if (current_config_scope() != CONFIG_SCOPE_REPO) {\n> > --\n> > 2.21.0.420.g4906d192b3\n> >\n"},{"id":"374361","messageId":"xmqq5zr4414e.fsf@gitster-ct.c.googlers.com","threadId":"50977","inReplyTo":"CABPp-BGbDN2DVwJKh3gdQ1wDZmGuevAYRhhfe_MCyunABDsEqg@mail.gmail.com","subject":"Re: [PATCH] Honor core.precomposeUnicode in more places","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-04-24T01:56:33Z","receivedAt":"2019-04-24T01:56:44Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Elijah Newren writes:\n\n> ..., and passing a config-related callback function to\n> parse_options seems a little weird to me.\n\nA little? That's a moderate understatement.\n\nIf parse_options API were the ONLY thing that gets affected by\nprecompose_unicode, having an \"if the config has not been read yet,\nread only that configuration variable and nothing else\" there might\nmake sense, but that is not the case (i.e. the variable also affects\nhow readdir() works). So the alternatives that make sense are \n\n (1) we stick to the current \"make sure we read that variable\n sufficiently early in the main flow of the program\" pattern,\n which this patch does, or\n\n (2) we switch to \"make sure the variable is read before we need it\"\n pattern, i.e. add that \"read the single config variable from\n the file, if it is not yet read\" call to both parse_options()\n and opendir()---if the set of operations affected by the\n precompose_unicode variable grows, we'd need to add the same\n call to the new places, too.\n\nI think (1) is good at least for now.\n\n>> > diff --git a/upload-pack.c b/upload-pack.c\n>> > index d098ef5982..159f751ea4 100644\n>> > --- a/upload-pack.c\n>> > +++ b/upload-pack.c\n>> > @@ -1064,6 +1064,8 @@ static int upload_pack_config(const char *var, const char *value, void *unused)\n>> > allow_ref_in_want = git_config_bool(var, value);\n>> > } else if (!strcmp(\"uploadpack.allowsidebandall\", var)) {\n>> > allow_sideband_all = git_config_bool(var, value);\n>> > + } else if (!strcmp(\"core.precomposeunicode\", var)) {\n>> > + precomposed_unicode = git_config_bool(var, value);\n>> > }\n\nWhat the other hunks wanted to do was quite obvious (i.e. before\ncalling parse_options(), make sure we know precomposed_unicode is\nset appropriately, so that argv[] can be tweaked correctly). But\nthis one was a bit less clear.\n\nIt turns out that upload-pack deliberately avoids using the default\nconfig callback, but tries to limit itself to the minimally needed\nset, so this hunk adds the precomposed_unicode to it. By doing so,\nwe trigger another effect of precomposed_unicode, i.e. tweaking the\npaths we read out of readdir(), so that the refs we have to offer to\nthe other side are all normalzied to the precomposed form.\n\nMakes sense. Thanks.\n\n"},{"id":"374503","messageId":"20190425145854.5185-1-newren@gmail.com","threadId":"50977","inReplyTo":"20190423173056.28523-1-newren@gmail.com","subject":"[PATCH v2] Honor core.precomposeUnicode in more places","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2019-04-25T14:58:54Z","receivedAt":"2019-04-25T14:59:03Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Mac's HFS where git sets core.precomposeUnicode to true automatically\nby git init/clone, when a user creates a simple unicode refname (in NFC\nformat) such as españa:\n\n $ git branch españa\n\ndifferent commands would display the branch name differently. For\nexample, git branch, git log --decorate, and git fast-export all used\n\n 65 73 70 61 c3 b1 61 (or \"espa\\xc3\\xb1a\")\n\n(NFC form) while show-ref would use\n\n 65 73 70 61 6e cc 83 61 (or \"espan\\xcc\\x83a\")\n\n(NFD form). A stress test for git filter-repo was tripped up by this\ninconsistency, though digging in I found that the problems could\ncompound; for example, if the user ran\n\n $ git pack-refs --all\n\nand then tried to check out the branch, they would be met with:\n\n $ git checkout españa\n error: pathspec 'españa' did not match any file(s) known to git\n\n $ git checkout españa --\n fatal: invalid reference: españa\n\n $ git branch\n españa\n * master\n\nNote that the user could run the `git branch` command first and copy and\npaste the `españa` portion of the output and still see the same two\nerrors. Also, if the user added --no-prune to the pack-refs command,\nthen they would see three branches: master, españa, and españa (those\nlast two are NFC vs. NFD forms, even if they render the same).\n\nFurther, if the user had the `españa` branch checked out before\nrunning `git pack-refs --all`, the user would be greeted with (note\nthat I'm trimming trailing output with an ellipsis):\n\n $ git rev-parse HEAD\n fatal: ambiguous argument 'HEAD': unknown revision or path...\n\n $ git status\n On branch españa\n\n No commits yet...\n\nOr worse, if the user didn't check this stuff first, running `git\ncommit` will create a new commit with all changes of all of history\nbeing squashed into it.\n\nIn addition to pack-refs, one could also get into this state with\nupload-pack or anything that calls either pack-refs or upload-pack (e.g.\ngc or clone).\n\nAdd code in a few places (pack-refs, show-ref, upload-pack) to check and\nhonor the setting of core.precomposeUnicode to avoid these bugs.\n\nSigned-off-by: Elijah Newren \n---\nChanges since v1:\n * Excised unnecessary portion of the first sentence of the commit message\n\n builtin/pack-refs.c | 2 ++\n builtin/show-ref.c | 3 +++\n upload-pack.c | 2 ++\n 3 files changed, 7 insertions(+)\n\ndiff --git a/builtin/pack-refs.c b/builtin/pack-refs.c\nindex f3353564f9..cfbd5c36c7 100644\n--- a/builtin/pack-refs.c\n+++ b/builtin/pack-refs.c\n@@ -1,4 +1,5 @@\n #include \"builtin.h\"\n+#include \"config.h\"\n #include \"parse-options.h\"\n #include \"refs.h\"\n #include \"repository.h\"\n@@ -16,6 +17,7 @@ int cmd_pack_refs(int argc, const char **argv, const char *prefix)\n \t\tOPT_BIT(0, \"prune\", &flags, N_(\"prune loose refs (default)\"), PACK_REFS_PRUNE),\n \t\tOPT_END(),\n \t};\n+\tgit_config(git_default_config, NULL);\n \tif (parse_options(argc, argv, prefix, opts, pack_refs_usage, 0))\n \t\tusage_with_options(pack_refs_usage, opts);\n \treturn refs_pack_refs(get_main_ref_store(the_repository), flags);\ndiff --git a/builtin/show-ref.c b/builtin/show-ref.c\nindex 6a706c02a6..6456da70cc 100644\n--- a/builtin/show-ref.c\n+++ b/builtin/show-ref.c\n@@ -1,5 +1,6 @@\n #include \"builtin.h\"\n #include \"cache.h\"\n+#include \"config.h\"\n #include \"refs.h\"\n #include \"object-store.h\"\n #include \"object.h\"\n@@ -182,6 +183,8 @@ static const struct option show_ref_options[] = {\n \n int cmd_show_ref(int argc, const char **argv, const char *prefix)\n {\n+\tgit_config(git_default_config, NULL);\n+\n \targc = parse_options(argc, argv, prefix, show_ref_options,\n \t\t\t show_ref_usage, 0);\n \ndiff --git a/upload-pack.c b/upload-pack.c\nindex d098ef5982..159f751ea4 100644\n--- a/upload-pack.c\n+++ b/upload-pack.c\n@@ -1064,6 +1064,8 @@ static int upload_pack_config(const char *var, const char *value, void *unused)\n \t\tallow_ref_in_want = git_config_bool(var, value);\n \t} else if (!strcmp(\"uploadpack.allowsidebandall\", var)) {\n \t\tallow_sideband_all = git_config_bool(var, value);\n+\t} else if (!strcmp(\"core.precomposeunicode\", var)) {\n+\t\tprecomposed_unicode = git_config_bool(var, value);\n \t}\n \n \tif (current_config_scope() != CONFIG_SCOPE_REPO) {\n-- \n2.21.0.503.g24b69a0db9\n\n"},{"id":"379388","messageId":"20190726194745.GA8076@sigill.intra.peff.net","threadId":"50977","inReplyTo":"20190423173056.28523-1-newren@gmail.com","subject":"Re: [PATCH] Honor core.precomposeUnicode in more places","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-07-26T19:47:45Z","receivedAt":"2019-07-26T19:47:47Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Apr 23, 2019 at 10:30:56AM -0700, Elijah Newren wrote:\n\n> diff --git a/builtin/pack-refs.c b/builtin/pack-refs.c\n> index f3353564f9..cfbd5c36c7 100644\n> --- a/builtin/pack-refs.c\n> +++ b/builtin/pack-refs.c\n> @@ -1,4 +1,5 @@\n> #include \"builtin.h\"\n> +#include \"config.h\"\n> #include \"parse-options.h\"\n> #include \"refs.h\"\n> #include \"repository.h\"\n> @@ -16,6 +17,7 @@ int cmd_pack_refs(int argc, const char **argv, const char *prefix)\n> \t\tOPT_BIT(0, \"prune\", &flags, N_(\"prune loose refs (default)\"), PACK_REFS_PRUNE),\n> \t\tOPT_END(),\n> \t};\n> +\tgit_config(git_default_config, NULL);\n> \tif (parse_options(argc, argv, prefix, opts, pack_refs_usage, 0))\n> \t\tusage_with_options(pack_refs_usage, opts);\n> \treturn refs_pack_refs(get_main_ref_store(the_repository), flags);\n\nI coincidentally just wrote the same patch (working off a branch of\nv2.21, so I didn't yet have your fix here). Just for the record, this\nfixes other bugs, too!\n\nMine was:\n\ndiff --git a/t/t3210-pack-refs.sh b/t/t3210-pack-refs.sh\nindex 9ea5fa4fd2..07d09ac67a 100755\n--- a/t/t3210-pack-refs.sh\n+++ b/t/t3210-pack-refs.sh\n@@ -253,4 +253,13 @@ test_expect_success SYMLINKS 'pack symlinked packed-refs' '\n \ttest \"$(readlink .git/packed-refs)\" = \"my-deviant-packed-refs\"\n '\n \n+test_expect_success 'pack-refs respects core.useReplaceRefs' '\n+\t# this is broken because the object refers to itself\n+\tcommit=$(git rev-parse HEAD) &&\n+\ttest_when_finished \"git update-ref -d refs/replace/$commit\" &&\n+\tgit update-ref refs/replace/$commit $commit &&\n+\ttest_must_fail git pack-refs --all &&\n+\tgit -c core.useReplaceRefs=false pack-refs --all\n+'\n+\n test_done\n\nIt's probably not worth adding the test independently, but I thought I'd\nthrow it out here for the benefit of the list archive.\n\n-Peff\n"}]}