{"thread":{"id":"46232","subject":"[PATCH 1/3] read-cache: use shared perms when writing shared index","startedAt":"2017-06-22T19:02:27Z","lastAt":"2018-11-17T12:47:36Z","messageCount":22,"participants":["Christian Couder","Junio C Hamano","Ramsay Jones","Ævar Arnfjörð Bjarmason","Duy Nguyen","SZEDER Gábor"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"322964","messageId":"20170622190158.13265-1-chriscool@tuxfamily.org","threadId":"46232","inReplyTo":null,"subject":"[PATCH 1/3] read-cache: use shared perms when writing shared index","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2017-06-22T19:01:56Z","receivedAt":"2017-06-22T19:02:27Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"Since f6ecc62dbf (write_shared_index(): use tempfile module, 2015-08-10)\nwrite_shared_index() has been using mks_tempfile() to create the\ntemporary file that will become the shared index.\n\nBut even before that, it looks like the functions used to create this\nfile didn't call adjust_shared_perm(), which means that the shared\nindex file has always been created with 600 permissions regardless\nof the shared permission settings.\n\nThis means that on repositories created with `git init --shared=all`\nand using the split index feature one gets an error like:\n\nfatal: .git/sharedindex.a52f910b489bc462f187ab572ba0086f7b5157de: index file open failed: Permission denied\n\nwhen another user performs any operation that reads the shared index.\n\nLet's fix that by using create_tempfile() instead of mks_tempfile()\nto create the shared index file.\n\nSigned-off-by: Christian Couder <chriscool@tuxfamily.org>\n---\n read-cache.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/read-cache.c b/read-cache.c\nindex bc156a133e..eb71e93aa4 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -2414,7 +2414,7 @@ static int write_shared_index(struct index_state *istate,\n \tstruct split_index *si = istate->split_index;\n \tint fd, ret;\n \n-\tfd = mks_tempfile(&temporary_sharedindex, git_path(\"sharedindex_XXXXXX\"));\n+\tfd = create_tempfile(&temporary_sharedindex, git_path(\"sharedindex_XXXXXX\"));\n \tif (fd < 0) {\n \t\thashclr(si->base_sha1);\n \t\treturn do_write_locked_index(istate, lock, flags);\n-- \n2.13.1.516.g05ec6e13aa\n\n"},{"id":"322965","messageId":"20170622190158.13265-2-chriscool@tuxfamily.org","threadId":"46232","inReplyTo":"20170622190158.13265-1-chriscool@tuxfamily.org","subject":"[PATCH 2/3] t1301: move movebits() to test-lib-functions.sh","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2017-06-22T19:01:57Z","receivedAt":"2017-06-22T19:02:29Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"As the movebits() function can be useful outside t1301,\nlet's move it into test-lib-functions.sh, and while at\nit let's rename it test_movebits().\n\nSigned-off-by: Christian Couder <chriscool@tuxfamily.org>\n---\n t/t1301-shared-repo.sh  | 18 +++++++-----------\n t/test-lib-functions.sh |  5 +++++\n 2 files changed, 12 insertions(+), 11 deletions(-)\n\ndiff --git a/t/t1301-shared-repo.sh b/t/t1301-shared-repo.sh\nindex 1312004f8c..dfece751b5 100755\n--- a/t/t1301-shared-repo.sh\n+++ b/t/t1301-shared-repo.sh\n@@ -19,10 +19,6 @@ test_expect_success 'shared = 0400 (faulty permission u-w)' '\n \t)\n '\n \n-modebits () {\n-\tls -l \"$1\" | sed -e 's|^\\(..........\\).*|\\1|'\n-}\n-\n for u in 002 022\n do\n \ttest_expect_success POSIXPERM \"shared=1 does not clear bits preset by umask $u\" '\n@@ -88,7 +84,7 @@ do\n \n \t\trm -f .git/info/refs &&\n \t\tgit update-server-info &&\n-\t\tactual=\"$(modebits .git/info/refs)\" &&\n+\t\tactual=\"$(test_modebits .git/info/refs)\" &&\n \t\tverbose test \"x$actual\" = \"x-$y\"\n \n \t'\n@@ -98,7 +94,7 @@ do\n \n \t\trm -f .git/info/refs &&\n \t\tgit update-server-info &&\n-\t\tactual=\"$(modebits .git/info/refs)\" &&\n+\t\tactual=\"$(test_modebits .git/info/refs)\" &&\n \t\tverbose test \"x$actual\" = \"x-$x\"\n \n \t'\n@@ -111,7 +107,7 @@ test_expect_success POSIXPERM 'info/refs respects umask in unshared repo' '\n \tumask 002 &&\n \tgit update-server-info &&\n \techo \"-rw-rw-r--\" >expect &&\n-\tmodebits .git/info/refs >actual &&\n+\ttest_modebits .git/info/refs >actual &&\n \ttest_cmp expect actual\n '\n \n@@ -177,7 +173,7 @@ test_expect_success POSIXPERM 'remote init does not use config from cwd' '\n \tumask 0022 &&\n \tgit init --bare child.git &&\n \techo \"-rw-r--r--\" >expect &&\n-\tmodebits child.git/config >actual &&\n+\ttest_modebits child.git/config >actual &&\n \ttest_cmp expect actual\n '\n \n@@ -187,7 +183,7 @@ test_expect_success POSIXPERM 're-init respects core.sharedrepository (local)' '\n \techo whatever >templates/foo &&\n \tgit init --template=templates &&\n \techo \"-rw-rw-rw-\" >expect &&\n-\tmodebits .git/foo >actual &&\n+\ttest_modebits .git/foo >actual &&\n \ttest_cmp expect actual\n '\n \n@@ -198,7 +194,7 @@ test_expect_success POSIXPERM 're-init respects core.sharedrepository (remote)'\n \ttest_path_is_missing child.git/foo &&\n \tgit init --bare --template=../templates child.git &&\n \techo \"-rw-rw-rw-\" >expect &&\n-\tmodebits child.git/foo >actual &&\n+\ttest_modebits child.git/foo >actual &&\n \ttest_cmp expect actual\n '\n \n@@ -209,7 +205,7 @@ test_expect_success POSIXPERM 'template can set core.sharedrepository' '\n \tcp .git/config templates/config &&\n \tgit init --bare --template=../templates child.git &&\n \techo \"-rw-rw-rw-\" >expect &&\n-\tmodebits child.git/HEAD >actual &&\n+\ttest_modebits child.git/HEAD >actual &&\n \ttest_cmp expect actual\n '\n \ndiff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh\nindex 5ee124332a..db622c3555 100644\n--- a/t/test-lib-functions.sh\n+++ b/t/test-lib-functions.sh\n@@ -216,6 +216,11 @@ test_chmod () {\n \tgit update-index --add \"--chmod=$@\"\n }\n \n+# Get the modebits from a file.\n+test_modebits () {\n+\tls -l \"$1\" | sed -e 's|^\\(..........\\).*|\\1|'\n+}\n+\n # Unset a configuration variable, but don't fail if it doesn't exist.\n test_unconfig () {\n \tconfig_dir=\n-- \n2.13.1.516.g05ec6e13aa\n\n"},{"id":"322966","messageId":"20170622190158.13265-3-chriscool@tuxfamily.org","threadId":"46232","inReplyTo":"20170622190158.13265-1-chriscool@tuxfamily.org","subject":"[PATCH 3/3] t1700: make sure split-index respects core.sharedrepository","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2017-06-22T19:01:58Z","receivedAt":"2017-06-22T19:02:39Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"Add a test to check that both the split-index file and the\nshared-index file are created using the right permissions\nwhen core.sharedrepository is set.\n\nSigned-off-by: Christian Couder <chriscool@tuxfamily.org>\n---\n t/t1700-split-index.sh | 12 ++++++++++++\n 1 file changed, 12 insertions(+)\n\ndiff --git a/t/t1700-split-index.sh b/t/t1700-split-index.sh\nindex af3ec0da5a..a52b92e82b 100755\n--- a/t/t1700-split-index.sh\n+++ b/t/t1700-split-index.sh\n@@ -370,4 +370,16 @@ test_expect_success 'check splitIndex.sharedIndexExpire set to \"never\" and \"now\"\n \ttest $(ls .git/sharedindex.* | wc -l) -le 2\n '\n \n+test_expect_success POSIXPERM 'split index respects core.sharedrepository' '\n+\tgit config core.sharedrepository 0666 &&\n+\t: >seventeen &&\n+\tgit update-index --add seventeen &&\n+\techo \"-rw-rw-rw-\" >expect &&\n+\ttest_modebits .git/index >actual &&\n+\ttest_cmp expect actual &&\n+\tnewest_shared_index=$(ls -t .git/sharedindex.* | head -1) &&\n+\ttest_modebits \"$newest_shared_index\" >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \n2.13.1.516.g05ec6e13aa\n\n"},{"id":"322975","messageId":"xmqq8tkjkdgl.fsf@gitster.mtv.corp.google.com","threadId":"46232","inReplyTo":"20170622190158.13265-1-chriscool@tuxfamily.org","subject":"Re: [PATCH 1/3] read-cache: use shared perms when writing shared index","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-06-22T19:51:54Z","receivedAt":"2017-06-22T19:52:01Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Christian Couder <christian.couder@gmail.com> writes:\n\n> Since f6ecc62dbf (write_shared_index(): use tempfile module, 2015-08-10)\n> write_shared_index() has been using mks_tempfile() to create the\n> temporary file that will become the shared index.\n>\n> But even before that, it looks like the functions used to create this\n> file didn't call adjust_shared_perm(), which means that the shared\n> index file has always been created with 600 permissions regardless\n> of the shared permission settings.\n>\n> This means that on repositories created with `git init --shared=all`\n> and using the split index feature one gets an error like:\n>\n> fatal: .git/sharedindex.a52f910b489bc462f187ab572ba0086f7b5157de: index file open failed: Permission denied\n>\n> when another user performs any operation that reads the shared index.\n\nAssuming that a \"shared\" repository setting should allow uses by\ndifferent users, the above analysis makes sense to me.\n\nBut the conclusion does not.\n\n> Let's fix that by using create_tempfile() instead of mks_tempfile()\n> to create the shared index file.\n>\n> ...\n> -\tfd = mks_tempfile(&temporary_sharedindex, git_path(\"sharedindex_XXXXXX\"));\n> +\tfd = create_tempfile(&temporary_sharedindex, git_path(\"sharedindex_XXXXXX\"));\n\nSo we used to create a temporary file that made sure its name is\nunique but now we create sharedindex_XXXXXX with 6 X's literally \nat the end?\n\nDoesn't mks_tempfile() family include a variant where you can give\ncustom mode?  Better yet, perhaps you can call adjust_shared_perm()\non the path _after_ seeing that mks_tempfile() succeeds (you can ask\nget_tempfile_path() which path to adjust, I presume)?\n\n>  \tif (fd < 0) {\n>  \t\thashclr(si->base_sha1);\n>  \t\treturn do_write_locked_index(istate, lock, flags);\n"},{"id":"322976","messageId":"xmqq4lv7kdfa.fsf@gitster.mtv.corp.google.com","threadId":"46232","inReplyTo":"20170622190158.13265-2-chriscool@tuxfamily.org","subject":"Re: [PATCH 2/3] t1301: move movebits() to test-lib-functions.sh","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-06-22T19:52:41Z","receivedAt":"2017-06-22T19:52:47Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Christian Couder <christian.couder@gmail.com> writes:\n\n> As the movebits() function can be useful outside t1301,\n> let's move it into test-lib-functions.sh, and while at\n> it let's rename it test_movebits().\n\nGood thinking, especially on the renaming.\n"},{"id":"322978","messageId":"xmqqzicziyss.fsf@gitster.mtv.corp.google.com","threadId":"46232","inReplyTo":"20170622190158.13265-3-chriscool@tuxfamily.org","subject":"Re: [PATCH 3/3] t1700: make sure split-index respects core.sharedrepository","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-06-22T19:53:55Z","receivedAt":"2017-06-22T19:54:01Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Christian Couder <christian.couder@gmail.com> writes:\n\n> Add a test to check that both the split-index file and the\n> shared-index file are created using the right permissions\n> when core.sharedrepository is set.\n>\n> Signed-off-by: Christian Couder <chriscool@tuxfamily.org>\n> ---\n>  t/t1700-split-index.sh | 12 ++++++++++++\n>  1 file changed, 12 insertions(+)\n>\n> diff --git a/t/t1700-split-index.sh b/t/t1700-split-index.sh\n> index af3ec0da5a..a52b92e82b 100755\n> --- a/t/t1700-split-index.sh\n> +++ b/t/t1700-split-index.sh\n> @@ -370,4 +370,16 @@ test_expect_success 'check splitIndex.sharedIndexExpire set to \"never\" and \"now\"\n>  \ttest $(ls .git/sharedindex.* | wc -l) -le 2\n>  '\n>  \n> +test_expect_success POSIXPERM 'split index respects core.sharedrepository' '\n> +\tgit config core.sharedrepository 0666 &&\n> +\t: >seventeen &&\n> +\tgit update-index --add seventeen &&\n> +\techo \"-rw-rw-rw-\" >expect &&\n> +\ttest_modebits .git/index >actual &&\n> +\ttest_cmp expect actual &&\n> +\tnewest_shared_index=$(ls -t .git/sharedindex.* | head -1) &&\n\nHmph.  Don't you want to make sure all of them, not just the latest\none, have the expected mode bits?\n\n> +\ttest_modebits \"$newest_shared_index\" >actual &&\n> +\ttest_cmp expect actual\n> +'\n> +\n>  test_done\n"},{"id":"322980","messageId":"CAP8UFD2QwN_8Mn3eFT_FRRbXNfnQFwt53nofh8AUzR0Zhx5EgA@mail.gmail.com","threadId":"46232","inReplyTo":"xmqqzicziyss.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 3/3] t1700: make sure split-index respects core.sharedrepository","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2017-06-22T20:19:46Z","receivedAt":"2017-06-22T20:19:51Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Thu, Jun 22, 2017 at 9:53 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Christian Couder <christian.couder@gmail.com> writes:\n>\n>> Add a test to check that both the split-index file and the\n>> shared-index file are created using the right permissions\n>> when core.sharedrepository is set.\n>>\n>> Signed-off-by: Christian Couder <chriscool@tuxfamily.org>\n>> ---\n>>  t/t1700-split-index.sh | 12 ++++++++++++\n>>  1 file changed, 12 insertions(+)\n>>\n>> diff --git a/t/t1700-split-index.sh b/t/t1700-split-index.sh\n>> index af3ec0da5a..a52b92e82b 100755\n>> --- a/t/t1700-split-index.sh\n>> +++ b/t/t1700-split-index.sh\n>> @@ -370,4 +370,16 @@ test_expect_success 'check splitIndex.sharedIndexExpire set to \"never\" and \"now\"\n>>       test $(ls .git/sharedindex.* | wc -l) -le 2\n>>  '\n>>\n>> +test_expect_success POSIXPERM 'split index respects core.sharedrepository' '\n>> +     git config core.sharedrepository 0666 &&\n>> +     : >seventeen &&\n>> +     git update-index --add seventeen &&\n>> +     echo \"-rw-rw-rw-\" >expect &&\n>> +     test_modebits .git/index >actual &&\n>> +     test_cmp expect actual &&\n>> +     newest_shared_index=$(ls -t .git/sharedindex.* | head -1) &&\n>\n> Hmph.  Don't you want to make sure all of them, not just the latest\n> one, have the expected mode bits?\n\nWe use \"git config core.sharedrepository 0666\" at the beginning of\nthis test, so it will only apply to the shared index files that are\ncreated after that.\n\nDo you suggest that we test before setting core.sharedrepository that\nthe existing shared index files all have the default permissions?\n"},{"id":"322983","messageId":"xmqqh8z7ixcc.fsf@gitster.mtv.corp.google.com","threadId":"46232","inReplyTo":"CAP8UFD2QwN_8Mn3eFT_FRRbXNfnQFwt53nofh8AUzR0Zhx5EgA@mail.gmail.com","subject":"Re: [PATCH 3/3] t1700: make sure split-index respects core.sharedrepository","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-06-22T20:25:23Z","receivedAt":"2017-06-22T20:25:40Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Christian Couder <christian.couder@gmail.com> writes:\n\n> We use \"git config core.sharedrepository 0666\" at the beginning of\n> this test, so it will only apply to the shared index files that are\n> created after that.\n>\n> Do you suggest that we test before setting core.sharedrepository that\n> the existing shared index files all have the default permissions?\n\nI think it would be sensible to see at least two values appear.\nOtherwise we cannot tell if the right value is coming by accident\n(because it was the default) or by design (because the configuration\nis correctly read).\n\n"},{"id":"322995","messageId":"CAP8UFD1dsC6iY-RJBTJ0utF_EG1+_KNHjqvN=MF9qjo2jDipSA@mail.gmail.com","threadId":"46232","inReplyTo":"xmqqh8z7ixcc.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 3/3] t1700: make sure split-index respects core.sharedrepository","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2017-06-22T21:07:21Z","receivedAt":"2017-06-22T21:07:27Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Thu, Jun 22, 2017 at 10:25 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Christian Couder <christian.couder@gmail.com> writes:\n>\n>> We use \"git config core.sharedrepository 0666\" at the beginning of\n>> this test, so it will only apply to the shared index files that are\n>> created after that.\n>>\n>> Do you suggest that we test before setting core.sharedrepository that\n>> the existing shared index files all have the default permissions?\n>\n> I think it would be sensible to see at least two values appear.\n> Otherwise we cannot tell if the right value is coming by accident\n> (because it was the default) or by design (because the configuration\n> is correctly read).\n\nOk, I think I will use something like this then:\n\nwhile read -r mode modebits filename; do\n    test_expect_success POSIXPERM \"split index respects\ncore.sharedrepository $mode\" '\n        git config core.sharedrepository \"$mode\" &&\n        : >\"$filename\" &&\n        git update-index --add \"$filename\" &&\n        echo \"$modebits\" >expect &&\n        test_modebits .git/index >actual &&\n        test_cmp expect actual &&\n        newest_shared_index=$(ls -t .git/sharedindex.* | head -1) &&\n        test_modebits \"$newest_shared_index\" >actual &&\n        test_cmp expect actual\n    '\ndone <<\\EOF\n0666 -rw-rw-rw- seventeen\n0642 -rw-r---w- eightteen\nEOF\n"},{"id":"323015","messageId":"455cbce5-97eb-2fdc-6e38-b4feb4daaa6e@ramsayjones.plus.com","threadId":"46232","inReplyTo":"xmqq4lv7kdfa.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 2/3] t1301: move movebits() to test-lib-functions.sh","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsayjones.plus.com","sentAt":"2017-06-22T23:09:59Z","receivedAt":"2017-06-22T23:10:08Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"\n\nOn 22/06/17 20:52, Junio C Hamano wrote:\n> Christian Couder <christian.couder@gmail.com> writes:\n> \n>> As the movebits() function can be useful outside t1301,\n>> let's move it into test-lib-functions.sh, and while at\n>> it let's rename it test_movebits().\n> \n> Good thinking, especially on the renaming.\n\nErr, except for the commit message! :-D\n\nBoth the commit message subject and the commit message body\nrefer to _move_bits() rather than _mode_bits() etc.\n(So, three instances of s/move/mode/).\n\nATB,\nRamsay Jones\n\n\n"},{"id":"323059","messageId":"xmqqk243fexu.fsf@gitster.mtv.corp.google.com","threadId":"46232","inReplyTo":"455cbce5-97eb-2fdc-6e38-b4feb4daaa6e@ramsayjones.plus.com","subject":"Re: [PATCH 2/3] t1301: move movebits() to test-lib-functions.sh","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-06-23T05:31:09Z","receivedAt":"2017-06-23T05:31:16Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ramsay Jones <ramsay@ramsayjones.plus.com> writes:\n\n> On 22/06/17 20:52, Junio C Hamano wrote:\n>> Christian Couder <christian.couder@gmail.com> writes:\n>> \n>>> As the movebits() function can be useful outside t1301,\n>>> let's move it into test-lib-functions.sh, and while at\n>>> it let's rename it test_movebits().\n>> \n>> Good thinking, especially on the renaming.\n>\n> Err, except for the commit message! :-D\n>\n> Both the commit message subject and the commit message body\n> refer to _move_bits() rather than _mode_bits() etc.\n> (So, three instances of s/move/mode/).\n\nWow, I shouldn't give praising reviews too easily ;-)\n\nGood eyes, thanks.\n"},{"id":"323098","messageId":"CAP8UFD0HTpAN04YU=zqUYudXdNWEq19grVgO1WLAN9SxpN0xug@mail.gmail.com","threadId":"46232","inReplyTo":"455cbce5-97eb-2fdc-6e38-b4feb4daaa6e@ramsayjones.plus.com","subject":"Re: [PATCH 2/3] t1301: move movebits() to test-lib-functions.sh","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2017-06-23T15:12:32Z","receivedAt":"2017-06-23T15:12:39Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Fri, Jun 23, 2017 at 1:09 AM, Ramsay Jones\n<ramsay@ramsayjones.plus.com> wrote:\n>\n>\n> On 22/06/17 20:52, Junio C Hamano wrote:\n>> Christian Couder <christian.couder@gmail.com> writes:\n>>\n>>> As the movebits() function can be useful outside t1301,\n>>> let's move it into test-lib-functions.sh, and while at\n>>> it let's rename it test_movebits().\n>>\n>> Good thinking, especially on the renaming.\n>\n> Err, except for the commit message! :-D\n>\n> Both the commit message subject and the commit message body\n> refer to _move_bits() rather than _mode_bits() etc.\n> (So, three instances of s/move/mode/).\n\nYeah, sorry about that. This is fixed in the version I will send\nreally soon now.\n"},{"id":"323100","messageId":"CAP8UFD0OrPW+CUiXHBCXjumG=VOkpjnQFe09=UT9W0ULXE=X=g@mail.gmail.com","threadId":"46232","inReplyTo":"xmqq8tkjkdgl.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 1/3] read-cache: use shared perms when writing shared index","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2017-06-23T15:14:24Z","receivedAt":"2017-06-23T15:14:30Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Thu, Jun 22, 2017 at 9:51 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Christian Couder <christian.couder@gmail.com> writes:\n\n>> Let's fix that by using create_tempfile() instead of mks_tempfile()\n>> to create the shared index file.\n>>\n>> ...\n>> -     fd = mks_tempfile(&temporary_sharedindex, git_path(\"sharedindex_XXXXXX\"));\n>> +     fd = create_tempfile(&temporary_sharedindex, git_path(\"sharedindex_XXXXXX\"));\n>\n> So we used to create a temporary file that made sure its name is\n> unique but now we create sharedindex_XXXXXX with 6 X's literally\n> at the end?\n>\n> Doesn't mks_tempfile() family include a variant where you can give\n> custom mode?  Better yet, perhaps you can call adjust_shared_perm()\n> on the path _after_ seeing that mks_tempfile() succeeds (you can ask\n> get_tempfile_path() which path to adjust, I presume)?\n\nYeah, adjust_shared_perm() is called after mks_tempfile() succeeds, in\nthe next version.\n"},{"id":"363167","messageId":"874lcl2e9t.fsf@evledraar.gmail.com","threadId":"46232","inReplyTo":"xmqq8tkjkdgl.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 1/3] read-cache: use shared perms when writing shared index","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-11-13T15:22:38Z","receivedAt":"2018-11-13T15:22:44Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Thu, 22 Jun 2017 12:51:54 -0700 Junio wrote:\n\n> > Let's fix that by using create_tempfile() instead of mks_tempfile()\n> > to create the shared index file.\n> >\n> > ...\n> > -     fd = mks_tempfile(&temporary_sharedindex, git_path(\"sharedindex_XXXXXX\"));\n> > +     fd = create_tempfile(&temporary_sharedindex, git_path(\"sharedindex_XXXXXX\"));\n>\n> So we used to create a temporary file that made sure its name is\n> unique but now we create sharedindex_XXXXXX with 6 X's literally\n> at the end?\n\nI'm looking at some of this again. Yeah that was a bug in Christian's\ncode, but interestingly if you just create literal sharedindex_XXXXXX\nfiles (don't replace the X's) the whole test suite passes under\nGIT_TEST_SPLIT_INDEX=true\n\nThat seems like a major blindspot, i.e. we don't seem to have tests that\nstress test the case of >1 split index.\n"},{"id":"363170","messageId":"20181113153235.25402-1-avarab@gmail.com","threadId":"46232","inReplyTo":"874lcl2e9t.fsf@evledraar.gmail.com","subject":"[RFC/PATCH] read-cache: write all indexes with the same permissions","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-11-13T15:32:35Z","receivedAt":"2018-11-13T15:32:51Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Change the code that writes out the shared index to use\ncreate_tempfile() instead of mks_tempfile();\n\nThe create_tempfile() function is used to write out the main\n.git/index (via .git/index.lock) using lock_file(). The\ncreate_tempfile() function respects the umask, whereas the\nmks_tempfile() function will create files with 0600 permissions.\n\nA bug related to this was spotted, fixed and tested for in\ndf801f3f9f (\"read-cache: use shared perms when writing shared index\",\n2017-06-25) and 3ee83f48e5 (\"t1700: make sure split-index respects\ncore.sharedrepository\", 2017-06-25).\n\nHowever, as noted in those commits we'd still create the file as 0600,\nand would just re-chmod it depending on the setting of\ncore.sharedRepository. So without core.splitIndex a system with\ne.g. the umask set to group writeability would work, but not with\ncore.splitIndex set.\n\nLet's instead make the two consistent by using create_tempfile(). This\nallows us to remove the code added in df801f3f9f (subsequently\nmodified in 59f9d2dd60 (\"read-cache.c: move tempfile creation/cleanup\nout of write_shared_index\", 2018-01-14)) as redundant. The\ncreate_tempfile() function itself calls adjust_shared_perm().\n\nNow we're not leaking the implementation detail that we're using a\nmkstemp()-like API for something that's not really a mkstemp()\nuse-case. See c18b80a0e8 (\"update-index: new options to enable/disable\nsplit index mode\", 2014-06-13) for the initial implementation which\nused mkstemp() without a wrapper.\n\nOne thing I was paranoid about when making this change was not\nintroducing a race condition where with\ne.g. core.sharedRepository=0600 we'd do something different for\n\"index\" v.s. \"sharedindex.*\", as the former has a *.lock file, not the\nlatter.\n\nBut I'm confident that we're exposing no such edge-case. With a user\numask of e.g. 0022 and core.sharedRepository=0600 we initially create\nboth \"index' and \"sharedindex.*\" files that are globally readable, but\nre-chmod them while they're still empty.\n\nIdeally we'd split up the adjust_shared_perm() function to one that\ncan give us the mode we want so we could just call open() instead of\nopen() followed by chmod(), but that's an unrelated cleanup. We\nalready have that minor issue with the \"index\" file #leftoverbits.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n\nI won't have time to finish this today, as noted in\nhttps://public-inbox.org/git/874lcl2e9t.fsf@evledraar.gmail.com/\nthere's a pretty major bug here in that we're now writing out literal\nsharedindex_XXXXXX files.\n\nObviously that needs to be fixed, and the fix is trivial, I can use\nanother one of the mks_*() functions with the same mode we use to\ncreate the index.\n\nBut we really ought to have tests for the bug this patch introduces,\nand as noted in the E-Mail linked above we don't.\n\nSo hopefully Duy or someone with more knowledge of the split index\nwill chime in to say what's missing there...\n\n read-cache.c           |  7 +------\n t/t1700-split-index.sh | 20 ++++++++++++++++++++\n 2 files changed, 21 insertions(+), 6 deletions(-)\n\ndiff --git a/read-cache.c b/read-cache.c\nindex f3a848d61c..7135537554 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -3074,11 +3074,6 @@ static int write_shared_index(struct index_state *istate,\n \tret = do_write_index(si->base, *temp, 1);\n \tif (ret)\n \t\treturn ret;\n-\tret = adjust_shared_perm(get_tempfile_path(*temp));\n-\tif (ret) {\n-\t\terror(\"cannot fix permission bits on %s\", get_tempfile_path(*temp));\n-\t\treturn ret;\n-\t}\n \tret = rename_tempfile(temp,\n \t\t\t      git_path(\"sharedindex.%s\", oid_to_hex(&si->base->oid)));\n \tif (!ret) {\n@@ -3159,7 +3154,7 @@ int write_locked_index(struct index_state *istate, struct lock_file *lock,\n \t\tstruct tempfile *temp;\n \t\tint saved_errno;\n \n-\t\ttemp = mks_tempfile(git_path(\"sharedindex_XXXXXX\"));\n+\t\ttemp = create_tempfile(git_path(\"sharedindex_XXXXXX\"));\n \t\tif (!temp) {\n \t\t\toidclr(&si->base_oid);\n \t\t\tret = do_write_locked_index(istate, lock, flags);\ndiff --git a/t/t1700-split-index.sh b/t/t1700-split-index.sh\nindex 2ac47aa0e4..fa1d3d468b 100755\n--- a/t/t1700-split-index.sh\n+++ b/t/t1700-split-index.sh\n@@ -381,6 +381,26 @@ test_expect_success 'check splitIndex.sharedIndexExpire set to \"never\" and \"now\"\n \ttest $(ls .git/sharedindex.* | wc -l) -le 2\n '\n \n+test_expect_success POSIXPERM 'same mode for index & split index' '\n+\tgit init same-mode &&\n+\t(\n+\t\tcd same-mode &&\n+\t\ttest_commit A &&\n+\t\ttest_modebits .git/index >index_mode &&\n+\t\ttest_must_fail git config core.sharedRepository &&\n+\t\tgit -c core.splitIndex=true status &&\n+\t\tshared=$(ls .git/sharedindex.*) &&\n+\t\tcase \"$shared\" in\n+\t\t*\" \"*)\n+\t\t\t# we have more than one???\n+\t\t\tfalse ;;\n+\t\t*)\n+\t\t\ttest_modebits \"$shared\" >split_index_mode &&\n+\t\t\ttest_cmp index_mode split_index_mode ;;\n+\t\tesac\n+\t)\n+'\n+\n while read -r mode modebits\n do\n \ttest_expect_success POSIXPERM \"split index respects core.sharedrepository $mode\" '\n-- \n2.19.1.1182.g4ecb1133ce\n\n"},{"id":"363176","messageId":"CACsJy8CaAC0UP+VxYU7zbdQc6DtKYa-FzOnbpNf+_P2L3zfUvQ@mail.gmail.com","threadId":"46232","inReplyTo":"20181113153235.25402-1-avarab@gmail.com","subject":"Re: [RFC/PATCH] read-cache: write all indexes with the same permissions","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2018-11-13T16:55:11Z","receivedAt":"2018-11-13T16:55:40Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Tue, Nov 13, 2018 at 4:32 PM Ævar Arnfjörð Bjarmason\n<avarab@gmail.com> wrote:\n> I won't have time to finish this today, as noted in\n> https://public-inbox.org/git/874lcl2e9t.fsf@evledraar.gmail.com/\n> there's a pretty major bug here in that we're now writing out literal\n> sharedindex_XXXXXX files.\n\nIt's not the end of the world because create_tempfile opens with\nO_EXCL so if two processes try to create it at the same time, one will\nfail, but no corruption or such.\n\n> Obviously that needs to be fixed, and the fix is trivial, I can use\n> another one of the mks_*() functions with the same mode we use to\n> create the index.\n>\n> But we really ought to have tests for the bug this patch introduces,\n> and as noted in the E-Mail linked above we don't.\n>\n> So hopefully Duy or someone with more knowledge of the split index\n> will chime in to say what's missing there...\n\nI don't have any bright idea how to catch the literal _XXXXX file.\nIt's a temporary file and will not last long enough for us to verify\nunless we intercept open() calls with LD_PRELOAD.\n-- \nDuy\n"},{"id":"363179","messageId":"8736s43mps.fsf@evledraar.gmail.com","threadId":"46232","inReplyTo":"CACsJy8CaAC0UP+VxYU7zbdQc6DtKYa-FzOnbpNf+_P2L3zfUvQ@mail.gmail.com","subject":"Re: [RFC/PATCH] read-cache: write all indexes with the same permissions","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-11-13T17:34:55Z","receivedAt":"2018-11-13T17:35:01Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Tue, Nov 13 2018, Duy Nguyen wrote:\n\n> On Tue, Nov 13, 2018 at 4:32 PM Ævar Arnfjörð Bjarmason\n> <avarab@gmail.com> wrote:\n>> I won't have time to finish this today, as noted in\n>> https://public-inbox.org/git/874lcl2e9t.fsf@evledraar.gmail.com/\n>> there's a pretty major bug here in that we're now writing out literal\n>> sharedindex_XXXXXX files.\n>\n> It's not the end of the world because create_tempfile opens with\n> O_EXCL so if two processes try to create it at the same time, one will\n> fail, but no corruption or such.\n\nRight, no race there.\n\n>> Obviously that needs to be fixed, and the fix is trivial, I can use\n>> another one of the mks_*() functions with the same mode we use to\n>> create the index.\n>>\n>> But we really ought to have tests for the bug this patch introduces,\n>> and as noted in the E-Mail linked above we don't.\n>>\n>> So hopefully Duy or someone with more knowledge of the split index\n>> will chime in to say what's missing there...\n>\n> I don't have any bright idea how to catch the literal _XXXXX file.\n> It's a temporary file and will not last long enough for us to verify\n> unless we intercept open() calls with LD_PRELOAD.\n\nSorry for being unclear. I don't mean how can we catch this specific\nbug, that would be uninteresting and hard to test for.\n\nI'm asking whether the bug in this patch isn't revealing an existing\nissue with us not having any tests for N number of sharedindex.*\nfiles. I.e. we have >1 of them, merge them and prune them, don't we?\n"},{"id":"363519","messageId":"CAP8UFD1Nd657Afgk6s+uNXMW=26Fg-6Jnv6ngB1biUX2mGBkSw@mail.gmail.com","threadId":"46232","inReplyTo":"8736s43mps.fsf@evledraar.gmail.com","subject":"Re: [RFC/PATCH] read-cache: write all indexes with the same permissions","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2018-11-16T17:41:43Z","receivedAt":"2018-11-16T17:41:58Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Tue, Nov 13, 2018 at 6:34 PM Ævar Arnfjörð Bjarmason\n<avarab@gmail.com> wrote:\n>\n> On Tue, Nov 13 2018, Duy Nguyen wrote:\n>\n> > On Tue, Nov 13, 2018 at 4:32 PM Ævar Arnfjörð Bjarmason\n> > <avarab@gmail.com> wrote:\n> > I don't have any bright idea how to catch the literal _XXXXX file.\n> > It's a temporary file and will not last long enough for us to verify\n> > unless we intercept open() calls with LD_PRELOAD.\n>\n> Sorry for being unclear. I don't mean how can we catch this specific\n> bug, that would be uninteresting and hard to test for.\n>\n> I'm asking whether the bug in this patch isn't revealing an existing\n> issue with us not having any tests for N number of sharedindex.*\n> files. I.e. we have >1 of them, merge them and prune them, don't we?\n\nI think we shouldn't have many of them. Usually we should have just\none, though it's possible that switching the shared index files\nfeature on and off several times or using temporary index files could\ncreate more than one.\n\nAnd there is clean_shared_index_files() which calls\nshould_delete_shared_index() to make sure they are regularly cleaned\nup.\n\nAnyway it's a different topic and according to what we privately\ndiscussed I just sent\nhttps://public-inbox.org/git/20181116173105.21784-1-chriscool@tuxfamily.org/\nto fix the current issue.\n"},{"id":"363523","messageId":"20181116190716.GO30222@szeder.dev","threadId":"46232","inReplyTo":"CAP8UFD1Nd657Afgk6s+uNXMW=26Fg-6Jnv6ngB1biUX2mGBkSw@mail.gmail.com","subject":"Re: [RFC/PATCH] read-cache: write all indexes with the same permissions","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2018-11-16T19:07:16Z","receivedAt":"2018-11-16T19:07:24Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Fri, Nov 16, 2018 at 06:41:43PM +0100, Christian Couder wrote:\n> On Tue, Nov 13, 2018 at 6:34 PM Ævar Arnfjörð Bjarmason\n> <avarab@gmail.com> wrote:\n\n> > I'm asking whether the bug in this patch isn't revealing an existing\n> > issue with us not having any tests for N number of sharedindex.*\n> > files. I.e. we have >1 of them, merge them and prune them, don't we?\n\nWe don't merge shared index files, but write a new one.\n\n> I think we shouldn't have many of them. Usually we should have just\n> one, though it's possible that switching the shared index files\n> feature on and off several times or using temporary index files could\n> create more than one.\n>\n> And there is clean_shared_index_files() which calls\n> should_delete_shared_index() to make sure they are regularly cleaned\n> up.\n\nI would think that it's more common to have more than one shared index\nfiles, because a new shared index is written when the number of\nentries in the split index reaches the threshold given in\n'splitIndex.maxPercentChange'.  The old ones are kept until they\nexpire, and 'splitIndex.sharedIndexExpire' defaults to 2 weeks (and\ncan even be be set to \"never\").\n\nWith the default 20% threshold a new shared index is written rather\nfrequently with our usual small test-repos:\n\n  $ git init\n  $ git update-index --split-index\n  $ ls -1 .git/*index*\n  .git/index\n  .git/sharedindex.4370042739b31cd17a5c5cd6043a77c9a00df113\n  $ echo 1 >file\n  $ git add file\n  $ git commit -q -m 1\n  $ echo 2 >file\n  $ git commit -q -m 2 file\n  $ echo 3 >file\n  $ git commit -q -m 3 file\n  $ ls -1 .git/*index*\n  .git/index\n  .git/sharedindex.4370042739b31cd17a5c5cd6043a77c9a00df113\n  .git/sharedindex.6aedbf71b1a6bdc0018078ec7571e1b21ba4b954\n  .git/sharedindex.b9106e9b82a818a0e4e9148224fc44ea98f488a1\n  .git/sharedindex.bad0b75d270a431b9e961cfc15df6ec935a67be8\n\n> Anyway it's a different topic and according to what we privately\n> discussed I just sent\n> https://public-inbox.org/git/20181116173105.21784-1-chriscool@tuxfamily.org/\n> to fix the current issue.\n"},{"id":"363526","messageId":"CACsJy8DS1JyuRFyKhVRnVt+ax40+yyv-OVknfQ82OO9jtMeK8g@mail.gmail.com","threadId":"46232","inReplyTo":"20181116190716.GO30222@szeder.dev","subject":"Re: [RFC/PATCH] read-cache: write all indexes with the same permissions","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2018-11-16T19:19:42Z","receivedAt":"2018-11-16T19:20:12Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Fri, Nov 16, 2018 at 8:07 PM SZEDER Gábor <szeder.dev@gmail.com> wrote:\n>\n> On Fri, Nov 16, 2018 at 06:41:43PM +0100, Christian Couder wrote:\n> > On Tue, Nov 13, 2018 at 6:34 PM Ævar Arnfjörð Bjarmason\n> > <avarab@gmail.com> wrote:\n>\n> > > I'm asking whether the bug in this patch isn't revealing an existing\n> > > issue with us not having any tests for N number of sharedindex.*\n> > > files. I.e. we have >1 of them, merge them and prune them, don't we?\n>\n> We don't merge shared index files, but write a new one.\n\nTrue. They are immutable like git objects.\n\n> With the default 20% threshold a new shared index is written rather\n> frequently with our usual small test-repos:\n\nSide note. Split index is definitely not meant for small repos. But\nmaybe we should have a lower limit (in terms of absolute number of\nentries) that prevent splitting. This splitting seems excessive.\n\n>   $ git init\n>   $ git update-index --split-index\n>   $ ls -1 .git/*index*\n>   .git/index\n>   .git/sharedindex.4370042739b31cd17a5c5cd6043a77c9a00df113\n>   $ echo 1 >file\n>   $ git add file\n>   $ git commit -q -m 1\n>   $ echo 2 >file\n>   $ git commit -q -m 2 file\n>   $ echo 3 >file\n>   $ git commit -q -m 3 file\n>   $ ls -1 .git/*index*\n>   .git/index\n>   .git/sharedindex.4370042739b31cd17a5c5cd6043a77c9a00df113\n>   .git/sharedindex.6aedbf71b1a6bdc0018078ec7571e1b21ba4b954\n>   .git/sharedindex.b9106e9b82a818a0e4e9148224fc44ea98f488a1\n>   .git/sharedindex.bad0b75d270a431b9e961cfc15df6ec935a67be8\n-- \nDuy\n"},{"id":"363542","messageId":"CAP8UFD3sX4YPmyXtxbcL3-TZEyd_fq7MuSZkYeNGvqswuF4Ncw@mail.gmail.com","threadId":"46232","inReplyTo":"CACsJy8DS1JyuRFyKhVRnVt+ax40+yyv-OVknfQ82OO9jtMeK8g@mail.gmail.com","subject":"Re: [RFC/PATCH] read-cache: write all indexes with the same permissions","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2018-11-17T06:52:30Z","receivedAt":"2018-11-17T06:52:45Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Fri, Nov 16, 2018 at 8:20 PM Duy Nguyen <pclouds@gmail.com> wrote:\n>\n> On Fri, Nov 16, 2018 at 8:07 PM SZEDER Gábor <szeder.dev@gmail.com> wrote:\n>\n> > With the default 20% threshold a new shared index is written rather\n> > frequently with our usual small test-repos:\n>\n> Side note. Split index is definitely not meant for small repos.\n\nI very much agree with that. It makes sense to use them only for big\nrepos and big repos usually don't pass a 20% threshold very often.\n\n> But\n> maybe we should have a lower limit (in terms of absolute number of\n> entries) that prevent splitting. This splitting seems excessive.\n\nI would agree if split index was the default mode or if our goal was\nto eventually make it the default mode.\n\nOr it could be a new \"mixed\" mode for core.splitIndex (which might\neventually become the default mode) to have no split-index as long as\nthe repo stays under a lower limit and to automatically use\nsplit-index when the repo gets over the limit.\n"},{"id":"363553","messageId":"20181117123854.GQ30222@szeder.dev","threadId":"46232","inReplyTo":"CAP8UFD3sX4YPmyXtxbcL3-TZEyd_fq7MuSZkYeNGvqswuF4Ncw@mail.gmail.com","subject":"Re: [RFC/PATCH] read-cache: write all indexes with the same permissions","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2018-11-17T12:38:54Z","receivedAt":"2018-11-17T12:47:36Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Sat, Nov 17, 2018 at 07:52:30AM +0100, Christian Couder wrote:\n> On Fri, Nov 16, 2018 at 8:20 PM Duy Nguyen <pclouds@gmail.com> wrote:\n> >\n> > On Fri, Nov 16, 2018 at 8:07 PM SZEDER Gábor <szeder.dev@gmail.com> wrote:\n> >\n> > > With the default 20% threshold a new shared index is written rather\n> > > frequently with our usual small test-repos:\n> >\n> > Side note. Split index is definitely not meant for small repos.\n> \n> I very much agree with that. It makes sense to use them only for big\n> repos and big repos usually don't pass a 20% threshold very often.\n\nBut our test suite does use very small repositories, so perhaps we\nhave been already testing what Ævar wanted to test?  (Though I didn't\nquite understood what that was; and we likely don't do so explicitly,\nbut only by chance with GIT_TEST_SPLIT_INDEX=1.)\n\n> > But\n> > maybe we should have a lower limit (in terms of absolute number of\n> > entries) that prevent splitting. This splitting seems excessive.\n> \n> I would agree if split index was the default mode or if our goal was\n> to eventually make it the default mode.\n\nSame here.  If you don't need split index, i.e. don't have huge repos,\nthen don't enable it in the first place.  And if it is enabled in a\nsmall repo, then the extra effort to write a new shared index is\nnegligible, and the space wasted for those small files doesn't really\nmatter (though arguably the output from a 'ls .git' would be\nsurprising...  which, at the same time, would be a good motivating\nfactor to turn the feature off).\n\n"}]}