{"thread":{"id":"52104","subject":"[PATCH 0/1] [v2.24.0-rc0 BUG] index.version is not parsed correctly","startedAt":"2019-10-23T20:39:01Z","lastAt":"2019-10-24T11:04:08Z","messageCount":3,"participants":["Derrick Stolee via GitGitGadget","Philip Oakley"],"isPatch":true,"patchVersion":1,"patchTotal":1},"messages":[{"id":"384720","messageId":"pull.420.git.1571863137.gitgitgadget@gmail.com","threadId":"52104","inReplyTo":null,"subject":"[PATCH 0/1] [v2.24.0-rc0 BUG] index.version is not parsed correctly","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-10-23T20:38:56Z","receivedAt":"2019-10-23T20:39:01Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"Here is another bug related to changes I made in this release. This time\nit's not due to a strange side-effect of submodules but rather that I was\ncareless while copy-pasting some boilerplate code.\n\nBasically, most of the config options used in repo-settings.c are boolean\nsettings. Except that \"index.version\" is an int setting!\n\nI found this while incorporating v2.24.0-rc0 into our microsoft/git fork and\nconsuming it in VFS for Git. We really care about the index having version\n4!\n\nThe tests I added around how the index version is set were not adequate to\ncover the case of only \"index.version=4\". Add a test to prevent this issue\nfrom happening again.\n\nThanks, -Stolee\n\nDerrick Stolee (1):\n repo-settings: read an int for index.version\n\n repo-settings.c | 2 +-\n t/t1600-index.sh | 4 ++++\n 2 files changed, 5 insertions(+), 1 deletion(-)\n\n\nbase-commit: d966095db01190a2196e31195ea6fa0c722aa732\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-420%2Fderrickstolee%2Frepo-settings-fix-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-420/derrickstolee/repo-settings-fix-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/420\n-- \ngitgitgadget\n"},{"id":"384721","messageId":"095b43f606a352527c9e0551adbd8a6af2061176.1571863137.git.gitgitgadget@gmail.com","threadId":"52104","inReplyTo":"pull.420.git.1571863137.gitgitgadget@gmail.com","subject":"[PATCH 1/1] repo-settings: read an int for index.version","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-10-23T20:38:57Z","receivedAt":"2019-10-23T20:39:02Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nSeveral config options were combined into a repo_settings struct in\nds/feature-macros, including a move of the \"index.version\" config\nsetting in 7211b9e (repo-settings: consolidate some config settings,\n2019-08-13).\n\nUnfortunately, that file looked like a lot of boilerplate and what is\nclearly a factor of copy-paste overload, the config setting is parsed\nwith repo_config_ge_bool() instead of repo_config_get_int(). This means\nthat a setting \"index.version=4\" would not register correctly and would\nrevert to the default version of 3.\n\nI caught this while incorporating v2.24.0-rc0 into the VFS for Git\ncodebase, where we really care that the index is in version 4.\n\nThis was not caught by the codebase because the version checks placed\nin t1600-index.sh did not test the \"basic\" scenario enough. Here, we\nmodify the test to include these normal settings to not be overridden by\nfeatures.manyFiles or GIT_INDEX_VERSION. While the \"default\" version is\n3, this is demoted to version 2 in do_write_index() when not necessary.\n\nSigned-off-by: Derrick Stolee \n---\n repo-settings.c | 2 +-\n t/t1600-index.sh | 4 ++++\n 2 files changed, 5 insertions(+), 1 deletion(-)\n\ndiff --git a/repo-settings.c b/repo-settings.c\nindex 05546db98e..a703e407a3 100644\n--- a/repo-settings.c\n+++ b/repo-settings.c\n@@ -22,7 +22,7 @@ void prepare_repo_settings(struct repository *r)\n \tUPDATE_DEFAULT_BOOL(r->settings.core_commit_graph, 1);\n \tUPDATE_DEFAULT_BOOL(r->settings.gc_write_commit_graph, 1);\n \n-\tif (!repo_config_get_bool(r, \"index.version\", &value))\n+\tif (!repo_config_get_int(r, \"index.version\", &value))\n \t\tr->settings.index_version = value;\n \tif (!repo_config_get_maybe_bool(r, \"core.untrackedcache\", &value)) {\n \t\tif (value == 0)\ndiff --git a/t/t1600-index.sh b/t/t1600-index.sh\nindex c77721b580..b7c31aa86a 100755\n--- a/t/t1600-index.sh\n+++ b/t/t1600-index.sh\n@@ -87,6 +87,10 @@ test_index_version () {\n }\n \n test_expect_success 'index version config precedence' '\n+\ttest_index_version 0 false 0 2 &&\n+\ttest_index_version 2 false 0 2 &&\n+\ttest_index_version 3 false 0 2 &&\n+\ttest_index_version 4 false 0 4 &&\n \ttest_index_version 2 false 4 4 &&\n \ttest_index_version 2 true 0 2 &&\n \ttest_index_version 0 true 0 4 &&\n-- \ngitgitgadget\n"},{"id":"384760","messageId":"e32c2694-e86c-0b93-a680-36b367f4258e@iee.email","threadId":"52104","inReplyTo":"095b43f606a352527c9e0551adbd8a6af2061176.1571863137.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 1/1] repo-settings: read an int for index.version","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.email","sentAt":"2019-10-24T11:04:02Z","receivedAt":"2019-10-24T11:04:08Z","isPatch":true,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"spelling nit.\n\nOn 23/10/2019 21:38, Derrick Stolee via GitGitGadget wrote:\n> From: Derrick Stolee\n>\n> Several config options were combined into a repo_settings struct in\n> ds/feature-macros, including a move of the \"index.version\" config\n> setting in 7211b9e (repo-settings: consolidate some config settings,\n> 2019-08-13).\n>\n> Unfortunately, that file looked like a lot of boilerplate and what is\n> clearly a factor of copy-paste overload, the config setting is parsed\n> with repo_config_ge_bool() instead of repo_config_get_int(). This means\n\ns/_ge_bool()/_get_bool()/\n> that a setting \"index.version=4\" would not register correctly and would\n> revert to the default version of 3.\n>\n> I caught this while incorporating v2.24.0-rc0 into the VFS for Git\n> codebase, where we really care that the index is in version 4.\n>\n> This was not caught by the codebase because the version checks placed\n> in t1600-index.sh did not test the \"basic\" scenario enough. Here, we\n> modify the test to include these normal settings to not be overridden by\n> features.manyFiles or GIT_INDEX_VERSION. While the \"default\" version is\n> 3, this is demoted to version 2 in do_write_index() when not necessary.\n>\n> Signed-off-by: Derrick Stolee\n> ---\nPhilip\n"}]}