{"thread":{"id":"47637","subject":"git 2.16.0 segfaults on clone of specific repo","startedAt":"2018-01-18T20:55:42Z","lastAt":"2018-01-21T19:55:26Z","messageCount":33,"participants":["Александр Булаев","Randall S. Becker","Eric Sunshine","brian m. carlson","Duy Nguyen","Todd Zullinger","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"336816","messageId":"CALwADsGfB10f5+nOFN-pHCt4z1SkWMcvSHn8KokcyCM0V6K-BA@mail.gmail.com","threadId":"47637","inReplyTo":null,"subject":"git 2.16.0 segfaults on clone of specific repo","fromName":"Александр Булаев","fromEmail":"aleks.bulaev@gmail.com","sentAt":"2018-01-18T20:55:34Z","receivedAt":"2018-01-18T20:55:42Z","isPatch":false,"sender":{"key":"aleks.bulaev@gmail.com","avatar":null},"body":"I found that git 2.16.0 segfaults on clone of vim-colorschemes repo.\nSee the log below.\n\nalexbool@alexbool-osx ~/Documents> lldb -- git clone\nhttps://github.com/flazz/vim-colorschemes.git\n(lldb) target create \"git\"\nCurrent executable set to 'git' (x86_64).\n(lldb) settings set -- target.run-args \"clone\"\n\"https://github.com/flazz/vim-colorschemes.git\"\n(lldb) run\nProcess 25643 launched: '/usr/local/bin/git' (x86_64)\nCloning into 'vim-colorschemes'...\nremote: Counting objects: 1457, done.\nremote: Total 1457 (delta 0), reused 0 (delta 0), pack-reused 1457\nReceiving objects: 100% (1457/1457), 1.43 MiB | 289.00 KiB/s, done.\nResolving deltas: 100% (424/424), done.\nProcess 25643 stopped\n* thread #1, queue = 'com.apple.main-thread', stop reason =\nEXC_BAD_ACCESS (code=1, address=0x48)\n frame #0: 0x00000001000f7c3d git`ce_match_stat_basic + 263\ngit`ce_match_stat_basic:\n-> 0x1000f7c3d <+263>: movq 0x48(%rax), %rsi\n 0x1000f7c41 <+267>: movl $0x14, %edx\n 0x1000f7c46 <+272>: movq %r14, %rdi\n 0x1000f7c49 <+275>: callq 0x1001659fa ; symbol stub\nfor: memcmp\nTarget 0: (git) stopped.\n(lldb) bt\n* thread #1, queue = 'com.apple.main-thread', stop reason =\nEXC_BAD_ACCESS (code=1, address=0x48)\n * frame #0: 0x00000001000f7c3d git`ce_match_stat_basic + 263\n frame #1: 0x00000001000f7ae8 git`ie_match_stat + 108\n frame #2: 0x00000001000bbe07 git`checkout_entry + 266\n frame #3: 0x0000000100144ff9 git`unpack_trees + 2317\n frame #4: 0x0000000100017e13 git`cmd_clone + 5733\n frame #5: 0x0000000100001ea3 git`handle_builtin + 532\n frame #6: 0x0000000100001954 git`cmd_main + 263\n frame #7: 0x0000000100079a28 git`main + 80\n frame #8: 0x00007fff64d76115 libdyld.dylib`start + 1\n(lldb) ^D\nalexbool@alexbool-osx ~/Documents> git --version\ngit version 2.16.0\n\nOS: macOS 10.13.2\ngit installed from homebrew\n"},{"id":"336817","messageId":"009401d3909f$ca040fe0$5e0c2fa0$@nexbridge.com","threadId":"47637","inReplyTo":"CALwADsGfB10f5+nOFN-pHCt4z1SkWMcvSHn8KokcyCM0V6K-BA@mail.gmail.com","subject":"RE: git 2.16.0 segfaults on clone of specific repo","fromName":"Randall S. Becker","fromEmail":"rsbecker@nexbridge.com","sentAt":"2018-01-18T21:03:21Z","receivedAt":"2018-01-18T21:03:34Z","isPatch":false,"sender":{"key":"randall.becker@nexbridge.ca","avatar":"https://avatars.githubusercontent.com/u/28956764?v=4"},"body":"On January 18, 2018 3:56 PM, Aleks wrote:\n> I found that git 2.16.0 segfaults on clone of vim-colorschemes repo.\n\n\nJust tested on NonStop NSE and works fine here. Just an FYI now that we're on 2.16.0.\n\nCheers,\nRandall\n\n-- Brief whoami:\n NonStop developer since approximately NonStop(211288444200000000)\n UNIX developer since approximately 421664400\n-- In my real life, I talk too much.\n\n\n\n"},{"id":"336865","messageId":"CAPig+cTkBEGyoS93GYCtoDgccTF_UixqBkVTExg7Zf0M1J7KRQ@mail.gmail.com","threadId":"47637","inReplyTo":"CALwADsGfB10f5+nOFN-pHCt4z1SkWMcvSHn8KokcyCM0V6K-BA@mail.gmail.com","subject":"Re: git 2.16.0 segfaults on clone of specific repo","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-01-19T00:15:56Z","receivedAt":"2018-01-19T00:16:01Z","isPatch":false,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"[+cc:brian]\n\nOn Thu, Jan 18, 2018 at 3:55 PM, Александр Булаев\n wrote:\n> I found that git 2.16.0 segfaults on clone of vim-colorschemes repo.\n>\n> (lldb) run\n> Process 25643 launched: '/usr/local/bin/git' (x86_64)\n> Cloning into 'vim-colorschemes'...\n> remote: Counting objects: 1457, done.\n> remote: Total 1457 (delta 0), reused 0 (delta 0), pack-reused 1457\n> Receiving objects: 100% (1457/1457), 1.43 MiB | 289.00 KiB/s, done.\n> Resolving deltas: 100% (424/424), done.\n> Process 25643 stopped\n> * thread #1, queue = 'com.apple.main-thread', stop reason =\n> EXC_BAD_ACCESS (code=1, address=0x48)\n\nI can confirm that this crashes on MacOS; it does not crash on Linux or BSD.\n\ngit-bisect places blame on eb0ccfd7f5 (Switch empty tree and blob\nlookups to use hash abstraction, 2017-11-12).\n"},{"id":"336873","messageId":"20180119024738.GA222163@genre.crustytoothpaste.net","threadId":"47637","inReplyTo":"CAPig+cTkBEGyoS93GYCtoDgccTF_UixqBkVTExg7Zf0M1J7KRQ@mail.gmail.com","subject":"Re: git 2.16.0 segfaults on clone of specific repo","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2018-01-19T02:47:39Z","receivedAt":"2018-01-19T02:47:53Z","isPatch":false,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On Thu, Jan 18, 2018 at 07:15:56PM -0500, Eric Sunshine wrote:\n> [+cc:brian]\n> \n> On Thu, Jan 18, 2018 at 3:55 PM, Александр Булаев\n> wrote:\n> > I found that git 2.16.0 segfaults on clone of vim-colorschemes repo.\n> >\n> > (lldb) run\n> > Process 25643 launched: '/usr/local/bin/git' (x86_64)\n> > Cloning into 'vim-colorschemes'...\n> > remote: Counting objects: 1457, done.\n> > remote: Total 1457 (delta 0), reused 0 (delta 0), pack-reused 1457\n> > Receiving objects: 100% (1457/1457), 1.43 MiB | 289.00 KiB/s, done.\n> > Resolving deltas: 100% (424/424), done.\n> > Process 25643 stopped\n> > * thread #1, queue = 'com.apple.main-thread', stop reason =\n> > EXC_BAD_ACCESS (code=1, address=0x48)\n> \n> I can confirm that this crashes on MacOS; it does not crash on Linux or BSD.\n> \n> git-bisect places blame on eb0ccfd7f5 (Switch empty tree and blob\n> lookups to use hash abstraction, 2017-11-12).\n\nI unfortunately don't have a macOS system to test with, and I've\ncompiled with both gcc and clang on my Debian system and, as you\nmentioned, it doesn't fail there.\n\nI have a guess about what the problem might be. Can you try this patch\nand see if it fixes things?\n\n-- >8 --\nFrom 10b690241619a452634b31fbc5ccd054a4f6e5ec Mon Sep 17 00:00:00 2001\nFrom: \"brian m. carlson\" \nDate: Sun, 14 Jan 2018 18:26:29 +0000\nSubject: [PATCH] repository: pre-initialize hash algo pointer\n\nThere are various git subcommands (among them, clone) which don't set up\nthe repository but end up needing to have information about the hash\nalgorithm in use. In the future, we can add a command line option for\nthis or read it from the configuration, but until we're ready to expose\nthat functionality to the user, simply initialize the repository\nstructure to use the current hash algorithm, SHA-1.\n\nSigned-off-by: brian m. carlson \n---\n repository.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/repository.c b/repository.c\nindex 998413b8bb..f66fcb1342 100644\n--- a/repository.c\n+++ b/repository.c\n@@ -5,7 +5,7 @@\n \n /* The main repository */\n static struct repository the_repo = {\n-\tNULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, &the_index, NULL, 0, 0\n+\tNULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, &the_index, &hash_algos[GIT_HASH_SHA1], 0, 0\n };\n struct repository *the_repository = &the_repo;\n \n-- \n2.16.0.rc2.280.g09355b536d\n-- >8 --\n-- \nbrian m. carlson / brian with sandals: Houston, Texas, US\nhttps://www.crustytoothpaste.net/~bmc | My opinion only\nOpenPGP: https://keybase.io/bk2204\n"},{"id":"336874","messageId":"CAPig+cSbbzKV0GttLjHCyY3CNcJO0bdP3Mp4pT+3waUTMAQ8kg@mail.gmail.com","threadId":"47637","inReplyTo":"20180119024738.GA222163@genre.crustytoothpaste.net","subject":"Re: git 2.16.0 segfaults on clone of specific repo","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-01-19T03:06:10Z","receivedAt":"2018-01-19T03:06:16Z","isPatch":false,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Thu, Jan 18, 2018 at 9:47 PM, brian m. carlson\n wrote:\n> On Thu, Jan 18, 2018 at 07:15:56PM -0500, Eric Sunshine wrote:\n>> On Thu, Jan 18, 2018 at 3:55 PM, Александр Булаев\n>> wrote:\n>> > I found that git 2.16.0 segfaults on clone of vim-colorschemes repo.\n>>\n>> I can confirm that this crashes on MacOS; it does not crash on Linux or BSD.\n>>\n>> git-bisect places blame on eb0ccfd7f5 (Switch empty tree and blob\n>> lookups to use hash abstraction, 2017-11-12).\n>\n> I unfortunately don't have a macOS system to test with, and I've\n> compiled with both gcc and clang on my Debian system and, as you\n> mentioned, it doesn't fail there.\n>\n> I have a guess about what the problem might be. Can you try this patch\n> and see if it fixes things?\n\nThat does fix the crash. Thanks for the quick diagnosis.\n\nCan the commit message go into more detail as to why this was crashing\n(or your speculation about why)? Perhaps give more detail about what\n'clone' is doing that led to the crash.\n\n> -- >8 --\n> From 10b690241619a452634b31fbc5ccd054a4f6e5ec Mon Sep 17 00:00:00 2001\n> From: \"brian m. carlson\" \n> Date: Sun, 14 Jan 2018 18:26:29 +0000\n> Subject: [PATCH] repository: pre-initialize hash algo pointer\n>\n> There are various git subcommands (among them, clone) which don't set up\n> the repository but end up needing to have information about the hash\n> algorithm in use. In the future, we can add a command line option for\n> this or read it from the configuration, but until we're ready to expose\n> that functionality to the user, simply initialize the repository\n> structure to use the current hash algorithm, SHA-1.\n>\n> Signed-off-by: brian m. carlson \n> ---\n> repository.c | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/repository.c b/repository.c\n> index 998413b8bb..f66fcb1342 100644\n> --- a/repository.c\n> +++ b/repository.c\n> @@ -5,7 +5,7 @@\n>\n> /* The main repository */\n> static struct repository the_repo = {\n> - NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, &the_index, NULL, 0, 0\n> + NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, &the_index, &hash_algos[GIT_HASH_SHA1], 0, 0\n> };\n> struct repository *the_repository = &the_repo;\n>\n> --\n"},{"id":"336875","messageId":"00c701d390d7$2dc80d10$89582730$@nexbridge.com","threadId":"47637","inReplyTo":"CAPig+cSbbzKV0GttLjHCyY3CNcJO0bdP3Mp4pT+3waUTMAQ8kg@mail.gmail.com","subject":"RE: git 2.16.0 segfaults on clone of specific repo","fromName":"Randall S. Becker","fromEmail":"rsbecker@nexbridge.com","sentAt":"2018-01-19T03:39:52Z","receivedAt":"2018-01-19T03:40:11Z","isPatch":false,"sender":{"key":"randall.becker@nexbridge.ca","avatar":"https://avatars.githubusercontent.com/u/28956764?v=4"},"body":"On January 18, 2018 10:06 PM, Eric Sunshine wrote:\n> On Thu, Jan 18, 2018 at 9:47 PM, brian m. carlson\n> wrote:\n> > On Thu, Jan 18, 2018 at 07:15:56PM -0500, Eric Sunshine wrote:\n> >> On Thu, Jan 18, 2018 at 3:55 PM, Александр Булаев\n> >> wrote:\n> >> > I found that git 2.16.0 segfaults on clone of vim-colorschemes repo.\n> >>\n> >> I can confirm that this crashes on MacOS; it does not crash on Linux or\n> BSD.\n> >>\n> >> git-bisect places blame on eb0ccfd7f5 (Switch empty tree and blob\n> >> lookups to use hash abstraction, 2017-11-12).\n> >\n> > I unfortunately don't have a macOS system to test with, and I've\n> > compiled with both gcc and clang on my Debian system and, as you\n> > mentioned, it doesn't fail there.\n> >\n> > I have a guess about what the problem might be. Can you try this\n> > patch and see if it fixes things?\n> \n> That does fix the crash. Thanks for the quick diagnosis.\n> \n> Can the commit message go into more detail as to why this was crashing (or\n> your speculation about why)? Perhaps give more detail about what 'clone' is\n> doing that led to the crash.\n\nI'm curious as to why this worked on my platform, given how it tends to get annoyed with NULL in the wrong place.\n\n> \n> > -- >8 --\n> > From 10b690241619a452634b31fbc5ccd054a4f6e5ec Mon Sep 17 00:00:00\n> 2001\n> > From: \"brian m. carlson\" \n> > Date: Sun, 14 Jan 2018 18:26:29 +0000\n> > Subject: [PATCH] repository: pre-initialize hash algo pointer\n> >\n> > There are various git subcommands (among them, clone) which don't set\n> > up the repository but end up needing to have information about the\n> > hash algorithm in use. In the future, we can add a command line\n> > option for this or read it from the configuration, but until we're\n> > ready to expose that functionality to the user, simply initialize the\n> > repository structure to use the current hash algorithm, SHA-1.\n> >\n> > Signed-off-by: brian m. carlson \n> > ---\n> > repository.c | 2 +-\n> > 1 file changed, 1 insertion(+), 1 deletion(-)\n> >\n> > diff --git a/repository.c b/repository.c index 998413b8bb..f66fcb1342\n> > 100644\n> > --- a/repository.c\n> > +++ b/repository.c\n> > @@ -5,7 +5,7 @@\n> >\n> > /* The main repository */\n> > static struct repository the_repo = {\n> > - NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, &the_index,\n> NULL, 0, 0\n> > + NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL,\n> > + &the_index, &hash_algos[GIT_HASH_SHA1], 0, 0\n> > };\n> > struct repository *the_repository = &the_repo;\n> >\n> > --\n\n"},{"id":"336876","messageId":"20180119034025.GB222163@genre.crustytoothpaste.net","threadId":"47637","inReplyTo":"CAPig+cSbbzKV0GttLjHCyY3CNcJO0bdP3Mp4pT+3waUTMAQ8kg@mail.gmail.com","subject":"Re: git 2.16.0 segfaults on clone of specific repo","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2018-01-19T03:40:25Z","receivedAt":"2018-01-19T03:40:37Z","isPatch":false,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On Thu, Jan 18, 2018 at 10:06:10PM -0500, Eric Sunshine wrote:\n> > I have a guess about what the problem might be. Can you try this patch\n> > and see if it fixes things?\n> \n> That does fix the crash. Thanks for the quick diagnosis.\n> \n> Can the commit message go into more detail as to why this was crashing\n> (or your speculation about why)? Perhaps give more detail about what\n> 'clone' is doing that led to the crash.\n\nSure. I ran into this as I was expanding the hash structure abstraction\ninto my next series. I'll send a follow-up patch with a more\ndescriptive answer.\n\nI'm still extremely puzzled as to why this doesn't fail on Linux. If\nit's failing in this case, it should very, very clearly fail all the\ntime we access an empty blob or an empty tree. I've tried with gcc and\ntwo versions of clang, using -fno-lto, with address sanitizer, with -O0,\nand so on. I'd really like to catch this kind of issue sooner in the\nfuture if I can figure out how to reproduce it.\n-- \nbrian m. carlson / brian with sandals: Houston, Texas, US\nhttps://www.crustytoothpaste.net/~bmc | My opinion only\nOpenPGP: https://keybase.io/bk2204\n"},{"id":"336877","messageId":"20180119041825.727904-1-sandals@crustytoothpaste.net","threadId":"47637","inReplyTo":"CAPig+cSbbzKV0GttLjHCyY3CNcJO0bdP3Mp4pT+3waUTMAQ8kg@mail.gmail.com","subject":"[PATCH] repository: pre-initialize hash algo pointer","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2018-01-19T04:18:25Z","receivedAt":"2018-01-19T04:19:14Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"There are various git subcommands (among them, clone) which don't set up\nthe repository (that is, they lack RUN_SETUP or RUN_SETUP_GENTLY) but\nend up needing to have information about the hash algorithm in use.\nBecause the hash algorithm is part of struct repository and it's only\ninitialized in repository setup, we can end up dereferencing a NULL\npointer in some cases if we call one of these subcommands and look up\nthe empty blob or empty tree values.\n\nIn the future, we can add a command line option for this or read it from\nthe configuration, but until we're ready to expose that functionality to\nthe user, simply initialize the repository structure to use the current\nhash algorithm, SHA-1.\n\nSigned-off-by: brian m. carlson \n---\n repository.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\nI'm still quite mystified as to why this is working on Linux and not\nmacOS, but I can only guess that compilers are just very advanced and\nhave somehow concluded that we would clearly never dereference a NULL\npointer, so they picked the only non-NULL value.\n\nI haven't included a test because I have no way to reproduce the issue.\nThis patch is the first from a series I'm working on where I do expand\nthe use of the hash struct and therefore caused a segfault on clone, so\nI can imagine what's going on without having a way to prove it affects\nthis particular case.\n\nIf someone with access to macOS can provide a test, I'd be very\ngrateful.\n\nMy apologies for the error and inconvenience.\n\ndiff --git a/repository.c b/repository.c\nindex 998413b8bb..f66fcb1342 100644\n--- a/repository.c\n+++ b/repository.c\n@@ -5,7 +5,7 @@\n \n /* The main repository */\n static struct repository the_repo = {\n-\tNULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, &the_index, NULL, 0, 0\n+\tNULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, &the_index, &hash_algos[GIT_HASH_SHA1], 0, 0\n };\n struct repository *the_repository = &the_repo;\n \n"},{"id":"336878","messageId":"CACsJy8BTFm_0sv=roL1OKKW=1DyU3vqD50NKyHg3KQ7G+mAepQ@mail.gmail.com","threadId":"47637","inReplyTo":"20180119034025.GB222163@genre.crustytoothpaste.net","subject":"Re: git 2.16.0 segfaults on clone of specific repo","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2018-01-19T05:31:58Z","receivedAt":"2018-01-19T05:32:34Z","isPatch":false,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Fri, Jan 19, 2018 at 10:40 AM, brian m. carlson\n wrote:\n> On Thu, Jan 18, 2018 at 10:06:10PM -0500, Eric Sunshine wrote:\n>> > I have a guess about what the problem might be. Can you try this patch\n>> > and see if it fixes things?\n>>\n>> That does fix the crash. Thanks for the quick diagnosis.\n>>\n>> Can the commit message go into more detail as to why this was crashing\n>> (or your speculation about why)? Perhaps give more detail about what\n>> 'clone' is doing that led to the crash.\n>\n> Sure. I ran into this as I was expanding the hash structure abstraction\n> into my next series. I'll send a follow-up patch with a more\n> descriptive answer.\n>\n> I'm still extremely puzzled as to why this doesn't fail on Linux. If\n> it's failing in this case, it should very, very clearly fail all the\n> time we access an empty blob or an empty tree. I've tried with gcc and\n> two versions of clang, using -fno-lto, with address sanitizer, with -O0,\n> and so on. I'd really like to catch this kind of issue sooner in the\n> future if I can figure out how to reproduce it.\n\nI think it's file system related, case-insensitive one in particular.\n\nThe call trace posted at the beginning of this thread should never\ntrigger for an initial clone. You only check if an _existing_ entry\nmatches what you are about to checkout when you switch trees. For this\nto happen at clone time, I suppose you have to checkout entry \"A\",\nthen re-checkout \"A\" again. Which can only happen on case-insensitive\nfile systems and a case-sensitive repo where the second \"A\" might\nactually be \"a\".\n\nvim-colorschemes.git has these two entries, which meets one of the\nconditions, I suppose macos meets the other\n\ncolors/darkblue.vim\ncolors/darkBlue.vim\n\nOn Linux, after I hacked all over the place to force ce_match_stat()\nto eventually call index_fd() which in turns must use one of the\nhashing function, I got a crash.\n-- \nDuy\n"},{"id":"336879","messageId":"20180119074001.GA55929@flurp.local","threadId":"47637","inReplyTo":"CACsJy8BTFm_0sv=roL1OKKW=1DyU3vqD50NKyHg3KQ7G+mAepQ@mail.gmail.com","subject":"Re: git 2.16.0 segfaults on clone of specific repo","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-01-19T07:40:02Z","receivedAt":"2018-01-19T07:40:21Z","isPatch":false,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Fri, Jan 19, 2018 at 12:31:58PM +0700, Duy Nguyen wrote:\n> On Fri, Jan 19, 2018 at 10:40 AM, brian m. carlson\n> > I'm still extremely puzzled as to why this doesn't fail on Linux. If\n> > it's failing in this case, it should very, very clearly fail all the\n> > time we access an empty blob or an empty tree.[...]\n> \n> I think it's file system related, case-insensitive one in particular.\n> \n> The call trace posted at the beginning of this thread should never\n> trigger for an initial clone. You only check if an _existing_ entry\n> matches what you are about to checkout when you switch trees. For this\n> to happen at clone time, I suppose you have to checkout entry \"A\",\n> then re-checkout \"A\" again. Which can only happen on case-insensitive\n> file systems and a case-sensitive repo where the second \"A\" might\n> actually be \"a\".\n> [...]\n> On Linux, after I hacked all over the place to force ce_match_stat()\n> to eventually call index_fd() which in turns must use one of the\n> hashing function, I got a crash.\n\nNice detective work. This particular manifestation is caught by the\nfollowing test which fails without brian's patch on MacOS (and\npresumably Windows) and succeeds with it. On Linux and BSD, it will of\ncourse succeed always, so I'm not sure how much practical value it\nhas.\n\n--- >8 ---\nhex2oct() {\n\tperl -ne 'printf \"\\\\%03o\", hex for /../g'\n}\n\ntest_expect_success 'clone on case-insensitive fs' '\n\to=$(git hash-object -w --stdin 8 ---\n\n(hex2oct lifted from t1007/t1450)\n"},{"id":"336880","messageId":"CAPig+cTOw5NsSmLHYcBEidDzNyiidJ0Dw1dF227KWDL9JrASvw@mail.gmail.com","threadId":"47637","inReplyTo":"20180119041825.727904-1-sandals@crustytoothpaste.net","subject":"Re: [PATCH] repository: pre-initialize hash algo pointer","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-01-19T07:54:26Z","receivedAt":"2018-01-19T07:54:33Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Thu, Jan 18, 2018 at 11:18 PM, brian m. carlson\n wrote:\n> There are various git subcommands (among them, clone) which don't set up\n> the repository (that is, they lack RUN_SETUP or RUN_SETUP_GENTLY) but\n> end up needing to have information about the hash algorithm in use.\n> Because the hash algorithm is part of struct repository and it's only\n> initialized in repository setup, we can end up dereferencing a NULL\n> pointer in some cases if we call one of these subcommands and look up\n> the empty blob or empty tree values.\n>\n> In the future, we can add a command line option for this or read it from\n> the configuration, but until we're ready to expose that functionality to\n> the user, simply initialize the repository structure to use the current\n> hash algorithm, SHA-1.\n>\n> Signed-off-by: brian m. carlson \n> ---\n> I'm still quite mystified as to why this is working on Linux and not\n> macOS, but I can only guess that compilers are just very advanced and\n> have somehow concluded that we would clearly never dereference a NULL\n> pointer, so they picked the only non-NULL value.\n\nNow that we know (due to Duy's excellent detective work[1]) that the\ntrigger is files with names differing only in case on case-insensitive\nfilesystems, the commit message can be updated appropriately.\n\n> I haven't included a test because I have no way to reproduce the issue.\n> This patch is the first from a series I'm working on where I do expand\n> the use of the hash struct and therefore caused a segfault on clone, so\n> I can imagine what's going on without having a way to prove it affects\n> this particular case.\n>\n> If someone with access to macOS can provide a test, I'd be very\n> grateful.\n\nDone. Find the test here[2]. It fails without your fix on MacOS and\n(presumably) Windows, and succeeds with the fix.\n\n> My apologies for the error and inconvenience.\n\n[1]: https://public-inbox.org/git/CACsJy8BTFm_0sv=roL1OKKW=1DyU3vqD50NKyHg3KQ7G+mAepQ@mail.gmail.com/\n[2]: https://public-inbox.org/git/20180119074001.GA55929@flurp.local/\n"},{"id":"336881","messageId":"CACsJy8AOPDNTbAWzDHd+Oa5PSc1v2qdfcaq60P7X5wznM4VTug@mail.gmail.com","threadId":"47637","inReplyTo":"20180119074001.GA55929@flurp.local","subject":"Re: git 2.16.0 segfaults on clone of specific repo","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2018-01-19T08:22:06Z","receivedAt":"2018-01-19T08:22:45Z","isPatch":false,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Fri, Jan 19, 2018 at 2:40 PM, Eric Sunshine wrote:\n> On Fri, Jan 19, 2018 at 12:31:58PM +0700, Duy Nguyen wrote:\n>> On Fri, Jan 19, 2018 at 10:40 AM, brian m. carlson\n>> > I'm still extremely puzzled as to why this doesn't fail on Linux. If\n>> > it's failing in this case, it should very, very clearly fail all the\n>> > time we access an empty blob or an empty tree.[...]\n>>\n>> I think it's file system related, case-insensitive one in particular.\n>>\n>> The call trace posted at the beginning of this thread should never\n>> trigger for an initial clone. You only check if an _existing_ entry\n>> matches what you are about to checkout when you switch trees. For this\n>> to happen at clone time, I suppose you have to checkout entry \"A\",\n>> then re-checkout \"A\" again. Which can only happen on case-insensitive\n>> file systems and a case-sensitive repo where the second \"A\" might\n>> actually be \"a\".\n>> [...]\n>> On Linux, after I hacked all over the place to force ce_match_stat()\n>> to eventually call index_fd() which in turns must use one of the\n>> hashing function, I got a crash.\n>\n> Nice detective work.\n\nAnd not stepping back to think for a bit. I realized now that if I\njust mounted a vfat filesystem, I could have verified it much quicker.\nIt does crash on linux with similar stack trace.\n\n(gdb) bt\n#0 0x000000000055809f in is_empty_blob_sha1 (sha1=0x8fa6d4 \"\\236\") at\ncache.h:1055\n#1 0x0000000000558acd in ce_match_stat_basic (ce=0x8fa690,\nst=0x7fffffffcd20) at read-cache.c:272\n#2 0x0000000000558c78 in ie_match_stat (istate=0x8e9fc0 ,\nce=0x8fa690, st=0x7fffffffcd20, options=5) at read-cache.c:342\n#3 0x0000000000501e01 in checkout_entry (ce=0x8fa690,\nstate=0x7fffffffcea0, topath=0x0) at entry.c:424\n#4 0x00000000005c8ea0 in check_updates (o=0x7fffffffd060) at unpack-trees.c:383\n#5 0x00000000005cb2bb in unpack_trees (len=1, t=0x7fffffffd010,\no=0x7fffffffd060) at unpack-trees.c:1382\n#6 0x00000000004214ca in checkout (submodule_progress=1) at builtin/clone.c:750\n#7 0x00000000004229ce in cmd_clone (argc=1, argv=0x7fffffffd840,\nprefix=0x0) at builtin/clone.c:1191\n#8 0x0000000000405846 in run_builtin (p=0x89a908 ,\nargc=2, argv=0x7fffffffd840) at git.c:346\n#9 0x0000000000405af7 in handle_builtin (argc=2, argv=0x7fffffffd840)\nat git.c:554\n#10 0x0000000000405c8f in run_argv (argcp=0x7fffffffd6fc,\nargv=0x7fffffffd6f0) at git.c:606\n#11 0x0000000000405e31 in cmd_main (argc=2, argv=0x7fffffffd840) at git.c:683\n#12 0x00000000004a3231 in main (argc=3, argv=0x7fffffffd838) at common-main.c:43\n\n> This particular manifestation is caught by the\n> following test which fails without brian's patch on MacOS (and\n> presumably Windows) and succeeds with it. On Linux and BSD, it will of\n> course succeed always, so I'm not sure how much practical value it\n> has.\n\nThere's a travis job running \"on windows\" (I don't what it really\nmeans) so it might help actually. This vim-colorschemes repository has\nshown up in git mailing list before, I think. We probably should just\nadd it as part of our regression tests ;-)\n-- \nDuy\n"},{"id":"336882","messageId":"CAPig+cQmCeEHZNkYSRPUTZHyRP-Tkiobx6ghSktYiT9CCBV6FA@mail.gmail.com","threadId":"47637","inReplyTo":"CACsJy8AOPDNTbAWzDHd+Oa5PSc1v2qdfcaq60P7X5wznM4VTug@mail.gmail.com","subject":"Re: git 2.16.0 segfaults on clone of specific repo","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-01-19T08:28:42Z","receivedAt":"2018-01-19T08:28:58Z","isPatch":false,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Fri, Jan 19, 2018 at 3:22 AM, Duy Nguyen wrote:\n> On Fri, Jan 19, 2018 at 2:40 PM, Eric Sunshine wrote:\n>> On Fri, Jan 19, 2018 at 12:31:58PM +0700, Duy Nguyen wrote:\n>>> On Linux, after I hacked all over the place to force ce_match_stat()\n>>> to eventually call index_fd() which in turns must use one of the\n>>> hashing function, I got a crash.\n>>\n>> Nice detective work.\n>\n> And not stepping back to think for a bit. I realized now that if I\n> just mounted a vfat filesystem, I could have verified it much quicker.\n> It does crash on linux with similar stack trace.\n> [...]\n>> This particular manifestation is caught by the\n>> following test which fails without brian's patch on MacOS (and\n>> presumably Windows) and succeeds with it. On Linux and BSD, it will of\n>> course succeed always, so I'm not sure how much practical value it\n>> has.\n>\n> There's a travis job running \"on windows\" (I don't what it really\n> means) so it might help actually. This vim-colorschemes repository has\n> shown up in git mailing list before, I think. We probably should just\n> add it as part of our regression tests ;-)\n\nAnd, Dscho does run the test suite on Windows regularly, so perhaps\nthis test does have some practical value.\n"},{"id":"336900","messageId":"20180119172549.GK1427@zaya.teonanacatl.net","threadId":"47637","inReplyTo":"20180119074001.GA55929@flurp.local","subject":"Re: git 2.16.0 segfaults on clone of specific repo","fromName":"Todd Zullinger","fromEmail":"tmz@pobox.com","sentAt":"2018-01-19T17:25:49Z","receivedAt":"2018-01-19T17:25:59Z","isPatch":false,"sender":{"key":"tmz@pobox.com","avatar":"https://avatars.githubusercontent.com/u/806319?v=4"},"body":"Eric Sunshine wrote:\n> Nice detective work. This particular manifestation is caught by the\n> following test which fails without brian's patch on MacOS (and\n> presumably Windows) and succeeds with it. On Linux and BSD, it will of\n> course succeed always, so I'm not sure how much practical value it\n> has.\n\nThe CASE_INSENSITIVE_FS prereq could be used to avoid\nrunning the test on systems where it won't provide much\nvalue, couldn't it?\n\n> --- >8 ---\n> hex2oct() {\n> \tperl -ne 'printf \"\\\\%03o\", hex for /../g'\n> }\n> \n> test_expect_success 'clone on case-insensitive fs' '\n> \to=$(git hash-object -w --stdin \tt=$(printf \"100644 X\\0${o}100644 x\\0${o}\" |\n> \t\t git hash-object -w -t tree --stdin) &&\n> \tc=$(git commit-tree -m bogus $t) &&\n> \tgit update-ref refs/heads/bogus $c &&\n> \tgit clone -b bogus . bogus\n> '\n> --- >8 ---\n> \n> (hex2oct lifted from t1007/t1450)\n\n-- \nTodd\n~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~\nMoney frees you from doing things you dislike. Since I dislike doing\nnearly everything, money is handy.\n -- Groucho Marx\n\n"},{"id":"336926","messageId":"xmqqr2qlps7r.fsf@gitster.mtv.corp.google.com","threadId":"47637","inReplyTo":"CAPig+cTOw5NsSmLHYcBEidDzNyiidJ0Dw1dF227KWDL9JrASvw@mail.gmail.com","subject":"Re: [PATCH] repository: pre-initialize hash algo pointer","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-01-19T19:24:24Z","receivedAt":"2018-01-19T19:24:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Sunshine writes:\n\n>> I'm still quite mystified as to why this is working on Linux and not\n>> macOS, but I can only guess that compilers are just very advanced and\n>> have somehow concluded that we would clearly never dereference a NULL\n>> pointer, so they picked the only non-NULL value.\n>\n> Now that we know (due to Duy's excellent detective work[1]) that the\n> trigger is files with names differing only in case on case-insensitive\n> filesystems, the commit message can be updated appropriately.\n\nThanks. Let me apply the following and do a 2.16.1, hopefully by\nthe end of day or mid tomorrow at the latest. Test to protect the\nfix can come as a separate follow-up patch.\n\n-- >8 --\nSubject: [PATCH] repository: pre-initialize hash algo pointer\n\nThere are various git subcommands (among them, clone) which don't set up\nthe repository (that is, they lack RUN_SETUP or RUN_SETUP_GENTLY) but\nend up needing to have information about the hash algorithm in use.\nBecause the hash algorithm is part of struct repository and it's only\ninitialized in repository setup, we can end up dereferencing a NULL\npointer in some cases if we call one of these subcommands and look up\nthe empty blob or empty tree values.\n\nA \"git clone\" of a project that has two paths that differ only in\ncase suffers from this if it is run on a case insensitive platform.\nWhen the command attempts to check out one of these two paths after\nchecking out the other one, the checkout codepath needs to see if\nthe version that is already on the filesystem (which should not\nhappen if the FS were case sensitive), and it needs to exercise the\nhashing code.\n\nIn the future, we can add a command line option for this or read it\nfrom the configuration, but until we're ready to expose that\nfunctionality to the user, simply initialize the repository\nstructure to use the current hash algorithm, SHA-1.\n\nSigned-off-by: brian m. carlson \nSigned-off-by: Junio C Hamano \n---\n repository.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/repository.c b/repository.c\nindex 998413b8bb..f66fcb1342 100644\n--- a/repository.c\n+++ b/repository.c\n@@ -5,7 +5,7 @@\n \n /* The main repository */\n static struct repository the_repo = {\n-\tNULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, &the_index, NULL, 0, 0\n+\tNULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, &the_index, &hash_algos[GIT_HASH_SHA1], 0, 0\n };\n struct repository *the_repository = &the_repo;\n \n-- \n2.16.0-204-gc262421c89\n\n"},{"id":"336942","messageId":"CAPig+cRX2HpUPCnuf3aiZO71KmFgBR4JE_=NBA3UuwuRxrXR2Q@mail.gmail.com","threadId":"47637","inReplyTo":"xmqqr2qlps7r.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] repository: pre-initialize hash algo pointer","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-01-19T21:48:47Z","receivedAt":"2018-01-19T21:48:52Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Fri, Jan 19, 2018 at 2:24 PM, Junio C Hamano wrote:\n> Eric Sunshine writes:\n>> Now that we know (due to Duy's excellent detective work[1]) that the\n>> trigger is files with names differing only in case on case-insensitive\n>> filesystems, the commit message can be updated appropriately.\n>\n> Thanks. Let me apply the following and do a 2.16.1, hopefully by\n> the end of day or mid tomorrow at the latest. Test to protect the\n> fix can come as a separate follow-up patch.\n>\n> -- >8 --\n> Subject: [PATCH] repository: pre-initialize hash algo pointer\n> [...]\n> A \"git clone\" of a project that has two paths that differ only in\n> case suffers from this if it is run on a case insensitive platform.\n> When the command attempts to check out one of these two paths after\n> checking out the other one, the checkout codepath needs to see if\n> the version that is already on the filesystem (which should not\n> happen if the FS were case sensitive), and it needs to exercise the\n> hashing code.\n\nThanks, the amended commit message makes the reason for the patch more\nconcrete. There does seem to be a bit of a grammatical issue, however,\nwhich makes it difficult to parse. Namely, \"already on the filesystem\n(...)\" probably was meant to say \"already on the filesystem (...) is\n{something}\".\n"},{"id":"336947","messageId":"xmqqshb1o58s.fsf@gitster.mtv.corp.google.com","threadId":"47637","inReplyTo":"CAPig+cRX2HpUPCnuf3aiZO71KmFgBR4JE_=NBA3UuwuRxrXR2Q@mail.gmail.com","subject":"Re: [PATCH] repository: pre-initialize hash algo pointer","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-01-19T22:25:55Z","receivedAt":"2018-01-19T22:26:05Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Sunshine writes:\n\n> On Fri, Jan 19, 2018 at 2:24 PM, Junio C Hamano wrote:\n>> Eric Sunshine writes:\n>>> Now that we know (due to Duy's excellent detective work[1]) that the\n>>> trigger is files with names differing only in case on case-insensitive\n>>> filesystems, the commit message can be updated appropriately.\n>>\n>> Thanks. Let me apply the following and do a 2.16.1, hopefully by\n>> the end of day or mid tomorrow at the latest. Test to protect the\n>> fix can come as a separate follow-up patch.\n>>\n>> -- >8 --\n>> Subject: [PATCH] repository: pre-initialize hash algo pointer\n>> [...]\n>> A \"git clone\" of a project that has two paths that differ only in\n>> case suffers from this if it is run on a case insensitive platform.\n>> When the command attempts to check out one of these two paths after\n>> checking out the other one, the checkout codepath needs to see if\n>> the version that is already on the filesystem (which should not\n>> happen if the FS were case sensitive), and it needs to exercise the\n>> hashing code.\n>\n> Thanks, the amended commit message makes the reason for the patch more\n> concrete. There does seem to be a bit of a grammatical issue, however,\n> which makes it difficult to parse. Namely, \"already on the filesystem\n> (...)\" probably was meant to say \"already on the filesystem (...) is\n> {something}\".\n\nIndeed it is incomplete sentence. The codepath wants to check if\nthe one on the filesystem is the same as the one that is being\nchecked out and exercises the hashing code at that point.\n\nThansk.\n"},{"id":"336949","messageId":"20180119223130.GC222163@genre.crustytoothpaste.net","threadId":"47637","inReplyTo":"20180119074001.GA55929@flurp.local","subject":"Re: git 2.16.0 segfaults on clone of specific repo","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2018-01-19T22:31:30Z","receivedAt":"2018-01-19T22:31:43Z","isPatch":false,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On Fri, Jan 19, 2018 at 02:40:02AM -0500, Eric Sunshine wrote:\n> Nice detective work. This particular manifestation is caught by the\n> following test which fails without brian's patch on MacOS (and\n> presumably Windows) and succeeds with it. On Linux and BSD, it will of\n> course succeed always, so I'm not sure how much practical value it\n> has.\n> \n> --- >8 ---\n> hex2oct() {\n> \tperl -ne 'printf \"\\\\%03o\", hex for /../g'\n> }\n> \n> test_expect_success 'clone on case-insensitive fs' '\n> \to=$(git hash-object -w --stdin \tt=$(printf \"100644 X\\0${o}100644 x\\0${o}\" |\n> \t\t git hash-object -w -t tree --stdin) &&\n> \tc=$(git commit-tree -m bogus $t) &&\n> \tgit update-ref refs/heads/bogus $c &&\n> \tgit clone -b bogus . bogus\n> '\n> --- >8 ---\n\nI'd argue that it's a worthwhile test to have, since it will fail on\nthose systems where it's going to be a problem. Furthermore, people do\nrun the tests (as does Travis) on case-insensitive file systems during\nthe development cycle, so if we break something in the future, someone\nwill notice while we're still in the development cycle.\n-- \nbrian m. carlson / brian with sandals: Houston, Texas, US\nhttps://www.crustytoothpaste.net/~bmc | My opinion only\nOpenPGP: https://keybase.io/bk2204\n"},{"id":"336955","messageId":"20180119231438.GE222163@genre.crustytoothpaste.net","threadId":"47637","inReplyTo":"xmqqr2qlps7r.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] repository: pre-initialize hash algo pointer","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2018-01-19T23:14:38Z","receivedAt":"2018-01-19T23:14:49Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On Fri, Jan 19, 2018 at 11:24:24AM -0800, Junio C Hamano wrote:\n> Eric Sunshine writes:\n> > Now that we know (due to Duy's excellent detective work[1]) that the\n> > trigger is files with names differing only in case on case-insensitive\n> > filesystems, the commit message can be updated appropriately.\n> \n> Thanks. Let me apply the following and do a 2.16.1, hopefully by\n> the end of day or mid tomorrow at the latest. Test to protect the\n> fix can come as a separate follow-up patch.\n\nThis looks good with Eric's suggested changes.\n\nAgain, my apologies for the breakage and the inconvenience involved.\n-- \nbrian m. carlson / brian with sandals: Houston, Texas, US\nhttps://www.crustytoothpaste.net/~bmc | My opinion only\nOpenPGP: https://keybase.io/bk2204\n"},{"id":"336960","messageId":"CAPig+cQmWqQWQrRQHHn=3hn6UFzJxT=9d5kKnJht_dt8sCgwkQ@mail.gmail.com","threadId":"47637","inReplyTo":"20180119223130.GC222163@genre.crustytoothpaste.net","subject":"Re: git 2.16.0 segfaults on clone of specific repo","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-01-20T00:15:45Z","receivedAt":"2018-01-20T00:17:07Z","isPatch":false,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Fri, Jan 19, 2018 at 5:31 PM, brian m. carlson\n wrote:\n> On Fri, Jan 19, 2018 at 02:40:02AM -0500, Eric Sunshine wrote:\n>> Nice detective work. This particular manifestation is caught by the\n>> following test which fails without brian's patch on MacOS (and\n>> presumably Windows) and succeeds with it. On Linux and BSD, it will of\n>> course succeed always, so I'm not sure how much practical value it\n>> has.\n>\n> I'd argue that it's a worthwhile test to have, since it will fail on\n> those systems where it's going to be a problem. Furthermore, people do\n> run the tests (as does Travis) on case-insensitive file systems during\n> the development cycle, so if we break something in the future, someone\n> will notice while we're still in the development cycle.\n\nOh, I agree. My original question of its practical value was based\nupon my belief that the full test suite is very rarely run on MacOS\n(partly because there are so few Git developers on MacOS and partly\nbecause it runs so slowly on the platform, though not nearly as slowly\nas on Windows). However, as soon as I hit \"Send\", it hit me that the\nproblem would also manifest on Windows, and we know that Dscho runs\nthe tests regularly on Windows, so I changed my mind. I did consider\nTravis but wasn't sure if it was testing on case-insensitive\nfilesystems.\n\nAre you planning on submitting the test as a proper patch as follow-up\nto [1]? I couldn't quite decide in which test script it should reside.\n\n[1]: https://public-inbox.org/git/xmqqr2qlps7r.fsf@gitster.mtv.corp.google.com/\n"},{"id":"336961","messageId":"CAPig+cSRN1zHc=zsO1Y_aQ_eO+sbsd0cq5iZ9hYz3ruK_E-0Dw@mail.gmail.com","threadId":"47637","inReplyTo":"20180119172549.GK1427@zaya.teonanacatl.net","subject":"Re: git 2.16.0 segfaults on clone of specific repo","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-01-20T00:23:18Z","receivedAt":"2018-01-20T00:23:24Z","isPatch":false,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Fri, Jan 19, 2018 at 12:25 PM, Todd Zullinger wrote:\n> Eric Sunshine wrote:\n>> Nice detective work. This particular manifestation is caught by the\n>> following test which fails without brian's patch on MacOS (and\n>> presumably Windows) and succeeds with it. On Linux and BSD, it will of\n>> course succeed always, so I'm not sure how much practical value it\n>> has.\n>\n> The CASE_INSENSITIVE_FS prereq could be used to avoid\n> running the test on systems where it won't provide much\n> value, couldn't it?\n\nAs mentioned in [1], my original questioning of the practical value of\nthe test was tied to the belief that the full test suite is probably\nrarely run on MacOS, and it wasn't until I pressed \"Send\" that it hit\nme that it would manifest on Windows as well, and that Dscho does test\non Windows regularly. Therefore, the test could have value.\n\nAt any rate, the test potentially could catch some sort of future\nregression even on case-sensitive filesystems, therefore, it would be\nbetter not to hide it behind CASE_INSENSITIVE_FS.\n\n[1]: https://public-inbox.org/git/CAPig+cQmWqQWQrRQHHn=3hn6UFzJxT=9d5kKnJht_dt8sCgwkQ@mail.gmail.com/\n"},{"id":"336962","messageId":"20180120002346.GA8965@genre.crustytoothpaste.net","threadId":"47637","inReplyTo":"CAPig+cQmWqQWQrRQHHn=3hn6UFzJxT=9d5kKnJht_dt8sCgwkQ@mail.gmail.com","subject":"Re: git 2.16.0 segfaults on clone of specific repo","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2018-01-20T00:23:46Z","receivedAt":"2018-01-20T00:24:00Z","isPatch":false,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On Fri, Jan 19, 2018 at 07:15:45PM -0500, Eric Sunshine wrote:\n> Oh, I agree. My original question of its practical value was based\n> upon my belief that the full test suite is very rarely run on MacOS\n> (partly because there are so few Git developers on MacOS and partly\n> because it runs so slowly on the platform, though not nearly as slowly\n> as on Windows). However, as soon as I hit \"Send\", it hit me that the\n> problem would also manifest on Windows, and we know that Dscho runs\n> the tests regularly on Windows, so I changed my mind. I did consider\n> Travis but wasn't sure if it was testing on case-insensitive\n> filesystems.\n> \n> Are you planning on submitting the test as a proper patch as follow-up\n> to [1]? I couldn't quite decide in which test script it should reside.\n\nSure, I can. Since you wrote it, may I have permission to add your\nsign-off?\n\nI'll try to set up a temporary FAT device and verify that I can\nreproduce the problem on Linux as well.\n-- \nbrian m. carlson / brian with sandals: Houston, Texas, US\nhttps://www.crustytoothpaste.net/~bmc | My opinion only\nOpenPGP: https://keybase.io/bk2204\n"},{"id":"336963","messageId":"CAPig+cQe5EC79nUk9aCgsS5rfu6cb2oikhZpiUuSSDoBYSfU5w@mail.gmail.com","threadId":"47637","inReplyTo":"20180120002346.GA8965@genre.crustytoothpaste.net","subject":"Re: git 2.16.0 segfaults on clone of specific repo","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-01-20T00:27:28Z","receivedAt":"2018-01-20T00:27:33Z","isPatch":false,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Fri, Jan 19, 2018 at 7:23 PM, brian m. carlson\n wrote:\n> On Fri, Jan 19, 2018 at 07:15:45PM -0500, Eric Sunshine wrote:\n>> Are you planning on submitting the test as a proper patch as follow-up\n>> to [1]? I couldn't quite decide in which test script it should reside.\n>\n> Sure, I can. Since you wrote it, may I have permission to add your\n> sign-off?\n\nYou may...\n\nSigned-off-by: Eric Sunshine \n\n> I'll try to set up a temporary FAT device and verify that I can\n> reproduce the problem on Linux as well.\n"},{"id":"336970","messageId":"xmqqo9lpnhdb.fsf@gitster.mtv.corp.google.com","threadId":"47637","inReplyTo":"xmqqr2qlps7r.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] repository: pre-initialize hash algo pointer","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-01-20T07:01:36Z","receivedAt":"2018-01-20T07:01:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Eric Sunshine writes:\n>\n>>> I'm still quite mystified as to why this is working on Linux and not\n>>> macOS, but I can only guess that compilers are just very advanced and\n>>> have somehow concluded that we would clearly never dereference a NULL\n>>> pointer, so they picked the only non-NULL value.\n>>\n>> Now that we know (due to Duy's excellent detective work[1]) that the\n>> trigger is files with names differing only in case on case-insensitive\n>> filesystems, the commit message can be updated appropriately.\n>\n> Thanks. Let me apply the following and do a 2.16.1, hopefully by\n> the end of day or mid tomorrow at the latest. Test to protect the\n> fix can come as a separate follow-up patch.\n\nI actually changed my mind and decided not to rush 2.16.1 with just\nthis change. After all, even though it is better not to crash in\nsuch a problematic case, a \"successful\" clone of such a project on\nsuch a filesystem cannot be sanely used *anyway*, so in that sense\nthe \"fix\" would stop \"clone\" from segfaulting but the resulting\nrepository would still be \"wrong\" (e.g. \"git status\" immediately\nafter clone would likely to say that the working tree is already\ndirty and missing one of the two paths, or something silly like\nthat).\n\n"},{"id":"336976","messageId":"CAPig+cR08-EsBGkrGckG5zTN3iPeLL4cj1g5TykicQJe7sK-pQ@mail.gmail.com","threadId":"47637","inReplyTo":"xmqqo9lpnhdb.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] repository: pre-initialize hash algo pointer","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-01-20T09:38:12Z","receivedAt":"2018-01-20T09:38:21Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sat, Jan 20, 2018 at 2:01 AM, Junio C Hamano wrote:\n> Junio C Hamano writes:\n>> Eric Sunshine writes:\n>>> Now that we know (due to Duy's excellent detective work[1]) that the\n>>> trigger is files with names differing only in case on case-insensitive\n>>> filesystems, the commit message can be updated appropriately.\n>>\n>> Thanks. Let me apply the following and do a 2.16.1, hopefully by\n>> the end of day or mid tomorrow at the latest. Test to protect the\n>> fix can come as a separate follow-up patch.\n>\n> I actually changed my mind and decided not to rush 2.16.1 with just\n> this change. After all, even though it is better not to crash in\n> such a problematic case, a \"successful\" clone of such a project on\n> such a filesystem cannot be sanely used *anyway*, so in that sense\n> the \"fix\" would stop \"clone\" from segfaulting but the resulting\n> repository would still be \"wrong\" (e.g. \"git status\" immediately\n> after clone would likely to say that the working tree is already\n> dirty and missing one of the two paths, or something silly like\n> that).\n\nThe counter-argument is that filenames differing only in case do not\nnecessarily render a project unusable on case-insensitive filesystems.\nFor instance, if the problematic files exist only in a project's test\nsuite or in some platform-specific resource directory, the project\nitself may still be perfectly usable for a person cloning a project\nmerely to build and use it (not develop it). However, \"clone\" crashing\n_does_ render the project unusable for the same person.\n\nThe crash[1] when cloning 'tcell' is an example of a project which is\nstill 100% usable on case-insensitive filesystems despite\ncase-conflicting filenames. Each of the conflicting file pairs [2] has\nidentical content, so everything works as intended.\n\nCase-conflicting filenames also do not necessarily make it impossible\nto do development work on a project. I have, myself, on several\noccasions cloned such projects on MacOS to make improvements and track\ndown and/or fix bugs in parts of the projects not impacted by the\ncase-conflicting filenames.\n\nFootnotes:\n\n[1]: https://public-inbox.org/git/20180119180855.GA98561@smm.local/\n\n[2]: case-conflicting files in tcell's bundled terminfo database:\n terminfo/database/32/2621A\n terminfo/database/32/2621a\n terminfo/database/68/hp2621A\n terminfo/database/68/hp2621a\n terminfo/database/68/hp70092A\n terminfo/database/68/hp70092a\n"},{"id":"336991","messageId":"20180120203324.112522-1-sandals@crustytoothpaste.net","threadId":"47637","inReplyTo":"20180119041825.727904-1-sandals@crustytoothpaste.net","subject":"[PATCH] t: add clone test for files differing only in case","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2018-01-20T20:33:24Z","receivedAt":"2018-01-20T20:33:41Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"We recently introduced a regression in cloning repositories onto\ncase-insensitive file systems where the repository contains multiple\nfiles differing only in case. In such a case, we would segfault. This\nsegfault has already been fixed (repository: pre-initialize hash algo\npointer), but it's not the first time similar problems have arisen.\nIntroduce a test to catch this case and ensure the behavior does not\nregress.\n\nSigned-off-by: Eric Sunshine \nSigned-off-by: brian m. carlson \n---\n t/t5601-clone.sh | 13 +++++++++++++\n 1 file changed, 13 insertions(+)\n\nI've verified that the test does fail without the patch on a vfat file\nsystem. However, many other tests also fail on a vfat file system on\nLinux, so unfortunately that doesn't look like a viable testing strategy\ngoing forward.\n\nI didn't include an object ID for the commit referenced simply because I\ndidn't see one yet and I didn't want to insert a local one that wouldn't\nwork for anyone else.\n\ndiff --git a/t/t5601-clone.sh b/t/t5601-clone.sh\nindex 0f895478f0..53b2dda9d2 100755\n--- a/t/t5601-clone.sh\n+++ b/t/t5601-clone.sh\n@@ -611,4 +611,17 @@ test_expect_success 'GIT_TRACE_PACKFILE produces a usable pack' '\n \tgit -C replay.git index-pack -v --stdin wrote:\n> We recently introduced a regression in cloning repositories onto\n> case-insensitive file systems where the repository contains multiple\n> files differing only in case. In such a case, we would segfault. This\n> segfault has already been fixed (repository: pre-initialize hash algo\n> pointer), but it's not the first time similar problems have arisen.\n> Introduce a test to catch this case and ensure the behavior does not\n> regress.\n\nI guess you'd probably want a \"From: Eric Sunshine \"\nheader before this paragraph.\n\nThe patch itself looks correct... ;-)\n\n> Signed-off-by: Eric Sunshine \n> Signed-off-by: brian m. carlson \n> ---\n> diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh\n> index 0f895478f0..53b2dda9d2 100755\n> --- a/t/t5601-clone.sh\n> +++ b/t/t5601-clone.sh\n> @@ -611,4 +611,17 @@ test_expect_success 'GIT_TRACE_PACKFILE produces a usable pack' '\n> git -C replay.git index-pack -v --stdin '\n>\n> +hex2oct() {\n> + perl -ne 'printf \"\\\\%03o\", hex for /../g'\n> +}\n> +\n> +test_expect_success 'clone on case-insensitive fs' '\n> + o=$(git hash-object -w --stdin + t=$(printf \"100644 X\\0${o}100644 x\\0${o}\" |\n> + git hash-object -w -t tree --stdin) &&\n> + c=$(git commit-tree -m bogus $t) &&\n> + git update-ref refs/heads/bogus $c &&\n> + git clone -b bogus . bogus\n> +'\n> +\n> test_done\n"},{"id":"337019","messageId":"xmqqa7x7oecq.fsf@gitster.mtv.corp.google.com","threadId":"47637","inReplyTo":"20180120203324.112522-1-sandals@crustytoothpaste.net","subject":"Re: [PATCH] t: add clone test for files differing only in case","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-01-21T07:33:41Z","receivedAt":"2018-01-21T07:33:49Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"brian m. carlson\" writes:\n\n> diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh\n> index 0f895478f0..53b2dda9d2 100755\n> --- a/t/t5601-clone.sh\n> +++ b/t/t5601-clone.sh\n> @@ -611,4 +611,17 @@ test_expect_success 'GIT_TRACE_PACKFILE produces a usable pack' '\n> \tgit -C replay.git index-pack -v --stdin '\n> \n> +hex2oct() {\n> +\tperl -ne 'printf \"\\\\%03o\", hex for /../g'\n> +}\n> +\n> +test_expect_success 'clone on case-insensitive fs' '\n> +\to=$(git hash-object -w --stdin +\tt=$(printf \"100644 X\\0${o}100644 x\\0${o}\" |\n> +\t\tgit hash-object -w -t tree --stdin) &&\n> +\tc=$(git commit-tree -m bogus $t) &&\n> +\tgit update-ref refs/heads/bogus $c &&\n> +\tgit clone -b bogus . bogus\n> +'\n> +\n> test_done\n\nHmm, I seem to be seeing a failure from this thing:\n\n expecting success:\n o=$(git hash-object -w --stdin wrote:\n> \"brian m. carlson\" writes:\n>> +test_expect_success 'clone on case-insensitive fs' '\n>> + o=$(git hash-object -w --stdin > + t=$(printf \"100644 X\\0${o}100644 x\\0${o}\" |\n>> + git hash-object -w -t tree --stdin) &&\n>> + c=$(git commit-tree -m bogus $t) &&\n>> + git update-ref refs/heads/bogus $c &&\n>> + git clone -b bogus . bogus\n>> +'\n>> +\n>> test_done\n>\n> Hmm, I seem to be seeing a failure from this thing:\n>\n> expecting success:\n> o=$(git hash-object -w --stdin t=$(printf \"100644 X\\0${o}100644 x\\0${o}\" |\n> git hash-object -w -t tree --stdin) &&\n> c=$(git commit-tree -m bogus $t) &&\n> git update-ref refs/heads/bogus $c &&\n> git clone -b bogus . bogus\n>\n> fatal: repository '.' does not exist\n>\n> even on a case sensitive platform.\n\nYep. In pretty much any other test script, this would work (it was\ndeveloped in a stand-alone script), but t5601 (which nukes .git as its\nfirst action) isn't the most friendly place.\n"},{"id":"337021","messageId":"20180121080728.GA20770@flurp.local","threadId":"47637","inReplyTo":"CAPig+cTW+vO7FWYViUrO-aV8pPL=KKhGf7Wkgh_51cbrxKZPEQ@mail.gmail.com","subject":"Re: [PATCH] t: add clone test for files differing only in case","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-01-21T08:07:28Z","receivedAt":"2018-01-21T08:07:42Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sun, Jan 21, 2018 at 02:46:15AM -0500, Eric Sunshine wrote:\n> On Sun, Jan 21, 2018 at 2:33 AM, Junio C Hamano wrote:\n> > \"brian m. carlson\" writes:\n> >> +test_expect_success 'clone on case-insensitive fs' '\n> >> + o=$(git hash-object -w --stdin >> + t=$(printf \"100644 X\\0${o}100644 x\\0${o}\" |\n> >> + git hash-object -w -t tree --stdin) &&\n> >> + c=$(git commit-tree -m bogus $t) &&\n> >> + git update-ref refs/heads/bogus $c &&\n> >> + git clone -b bogus . bogus\n> >> +'\n> >\n> > Hmm, I seem to be seeing a failure from this thing:\n> > fatal: repository '.' does not exist\n> > even on a case sensitive platform.\n> \n> Yep. In pretty much any other test script, this would work (it was\n> developed in a stand-alone script), but t5601 (which nukes .git as its\n> first action) isn't the most friendly place.\n\nHere's a re-roll which fixes that problem (and has a slightly\nre-written commit message.\n\n--- >8 ---\nFrom: Eric Sunshine \nSubject: [PATCH] t5601-clone: test case-conflicting files on case-insensitive\n filesystem\n\nA recently introduced regression caused a segfault at clone time on\ncase-insensitive filesystems when filenames differing only in case are\npresent. This bug has already been fixed (repository: pre-initialize\nhash algo pointer, 2018-01-18), but it's not the first time similar\nproblems have arisen. Therefore, introduce a test to catch this case and\nprotect against future regressions.\n\nSigned-off-by: Eric Sunshine \nSigned-off-by: brian m. carlson \nSigned-off-by: Eric Sunshine \n---\n t/t5601-clone.sh | 17 +++++++++++++++++\n 1 file changed, 17 insertions(+)\n\ndiff --git a/t/t5601-clone.sh b/t/t5601-clone.sh\nindex 0f895478f0..2d1490f631 100755\n--- a/t/t5601-clone.sh\n+++ b/t/t5601-clone.sh\n@@ -611,4 +611,21 @@ test_expect_success 'GIT_TRACE_PACKFILE produces a usable pack' '\n \tgit -C replay.git index-pack -v --stdin wrote:\n> We recently introduced a regression in cloning repositories onto\n> case-insensitive file systems where the repository contains multiple\n> files differing only in case. In such a case, we would segfault. This\n> segfault has already been fixed (repository: pre-initialize hash algo\n> pointer), but it's not the first time similar problems have arisen.\n> Introduce a test to catch this case and ensure the behavior does not\n> regress.\n>\n> Signed-off-by: Eric Sunshine \n> Signed-off-by: brian m. carlson \n> ---\n> t/t5601-clone.sh | 13 +++++++++++++\n> 1 file changed, 13 insertions(+)\n>\n> I've verified that the test does fail without the patch on a vfat file\n> system. However, many other tests also fail on a vfat file system on\n> Linux, so unfortunately that doesn't look like a viable testing strategy\n> going forward.\n\nvfat is not very UNIXy. I suspect we need a bunch of tweaks that\ncygwin/mingw uses to make the test suite pass. There are other\ncase-insensitive filesystems on linux though. JFS (with \"mkfs.jfs -O\"\nto make it case-insensitive) seems to pass the test suite and crash\ngit without your fix so it's a good candidate if someone wants to test\nthis behavior on linux in future. HFS+ fails some unicode test in\nt0050 and I didn't check further.\n\n> +test_expect_success 'clone on case-insensitive fs' '\n\nWe have CASE_INSENSITIVE_FS prereq. Should we use it here? I know it\ndoes not harm running this test on case-sensitive filesystem, but the\nprereq could be useful for grepping.\n\n> + o=$(git hash-object -w --stdin + t=$(printf \"100644 X\\0${o}100644 x\\0${o}\" |\n> + git hash-object -w -t tree --stdin) &&\n> + c=$(git commit-tree -m bogus $t) &&\n> + git update-ref refs/heads/bogus $c &&\n> + git clone -b bogus . bogus\n> +'\n> +\n> test_done\n-- \nDuy\n"},{"id":"337033","messageId":"CAPig+cT-oj2N978Qk3oQ_7wMOccAA80ws62UdFq3PvWkD6xKOg@mail.gmail.com","threadId":"47637","inReplyTo":"CACsJy8CpfVb546caxP-HGjk9uXa805uayF0A7oMzh+fzEFS_Qw@mail.gmail.com","subject":"Re: [PATCH] t: add clone test for files differing only in case","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-01-21T18:47:09Z","receivedAt":"2018-01-21T18:47:15Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sun, Jan 21, 2018 at 6:50 AM, Duy Nguyen wrote:\n> On Sun, Jan 21, 2018 at 3:33 AM, brian m. carlson\n>> +test_expect_success 'clone on case-insensitive fs' '\n>\n> We have CASE_INSENSITIVE_FS prereq. Should we use it here? I know it\n> does not harm running this test on case-sensitive filesystem, but the\n> prereq could be useful for grepping.\n\nI'd rather not hide it behind the CASE_INSENSITIVE_FS[1] prerequisite\nsince the test potentially could catch some sort of future regression\neven on case-sensitive filesystems.\n\n[1]: Todd Zullinger suggested the same:\nhttps://public-inbox.org/git/CAPig+cSRN1zHc=zsO1Y_aQ_eO+sbsd0cq5iZ9hYz3ruK_E-0Dw@mail.gmail.com/\n"},{"id":"337035","messageId":"20180121195511.GE8965@genre.crustytoothpaste.net","threadId":"47637","inReplyTo":"20180121080728.GA20770@flurp.local","subject":"Re: [PATCH] t: add clone test for files differing only in case","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2018-01-21T19:55:11Z","receivedAt":"2018-01-21T19:55:26Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On Sun, Jan 21, 2018 at 03:07:28AM -0500, Eric Sunshine wrote:\n> On Sun, Jan 21, 2018 at 02:46:15AM -0500, Eric Sunshine wrote:\n> > Yep. In pretty much any other test script, this would work (it was\n> > developed in a stand-alone script), but t5601 (which nukes .git as its\n> > first action) isn't the most friendly place.\n> \n> Here's a re-roll which fixes that problem (and has a slightly\n> re-written commit message.\n\nYes, this looks good. I did indeed intend to forge your From line on\nthe message and actually test that it was fixed, but only tested it on\nthe vfat partition, not my ext4 partition, where, of course, I got\ndistracted by the various other failures. Sigh.\n\nThanks for fixing this up.\n-- \nbrian m. carlson / brian with sandals: Houston, Texas, US\nhttps://www.crustytoothpaste.net/~bmc | My opinion only\nOpenPGP: https://keybase.io/bk2204\n"}]}