* [PATCH 0/1] [v2.24.0-rc0 BUG] index.version is not parsed correctly @ 2019-10-23 20:38 Derrick Stolee via GitGitGadget 2019-10-23 20:38 ` [PATCH 1/1] repo-settings: read an int for index.version Derrick Stolee via GitGitGadget 0 siblings, 1 reply; 3+ messages in thread From: Derrick Stolee via GitGitGadget @ 2019-10-23 20:38 UTC (permalink / raw) To: git; +Cc: Derrick Stolee, Junio C Hamano Here is another bug related to changes I made in this release. This time it's not due to a strange side-effect of submodules but rather that I was careless while copy-pasting some boilerplate code. Basically, most of the config options used in repo-settings.c are boolean settings. Except that "index.version" is an int setting! I found this while incorporating v2.24.0-rc0 into our microsoft/git fork and consuming it in VFS for Git. We really care about the index having version 4! The tests I added around how the index version is set were not adequate to cover the case of only "index.version=4". Add a test to prevent this issue from happening again. Thanks, -Stolee Derrick Stolee (1): repo-settings: read an int for index.version repo-settings.c | 2 +- t/t1600-index.sh | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) base-commit: d966095db01190a2196e31195ea6fa0c722aa732 Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-420%2Fderrickstolee%2Frepo-settings-fix-v1 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-420/derrickstolee/repo-settings-fix-v1 Pull-Request: https://github.com/gitgitgadget/git/pull/420 -- gitgitgadget ^ permalink raw reply [flat|nested] 3+ messages in thread
* [PATCH 1/1] repo-settings: read an int for index.version 2019-10-23 20:38 [PATCH 0/1] [v2.24.0-rc0 BUG] index.version is not parsed correctly Derrick Stolee via GitGitGadget @ 2019-10-23 20:38 ` Derrick Stolee via GitGitGadget 2019-10-24 11:04 ` Philip Oakley 0 siblings, 1 reply; 3+ messages in thread From: Derrick Stolee via GitGitGadget @ 2019-10-23 20:38 UTC (permalink / raw) To: git; +Cc: Derrick Stolee, Junio C Hamano, Derrick Stolee From: Derrick Stolee <dstolee@microsoft.com> Several config options were combined into a repo_settings struct in ds/feature-macros, including a move of the "index.version" config setting in 7211b9e (repo-settings: consolidate some config settings, 2019-08-13). Unfortunately, that file looked like a lot of boilerplate and what is clearly a factor of copy-paste overload, the config setting is parsed with repo_config_ge_bool() instead of repo_config_get_int(). This means that a setting "index.version=4" would not register correctly and would revert to the default version of 3. I caught this while incorporating v2.24.0-rc0 into the VFS for Git codebase, where we really care that the index is in version 4. This was not caught by the codebase because the version checks placed in t1600-index.sh did not test the "basic" scenario enough. Here, we modify the test to include these normal settings to not be overridden by features.manyFiles or GIT_INDEX_VERSION. While the "default" version is 3, this is demoted to version 2 in do_write_index() when not necessary. Signed-off-by: Derrick Stolee <dstolee@microsoft.com> --- repo-settings.c | 2 +- t/t1600-index.sh | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/repo-settings.c b/repo-settings.c index 05546db98e..a703e407a3 100644 --- a/repo-settings.c +++ b/repo-settings.c @@ -22,7 +22,7 @@ void prepare_repo_settings(struct repository *r) UPDATE_DEFAULT_BOOL(r->settings.core_commit_graph, 1); UPDATE_DEFAULT_BOOL(r->settings.gc_write_commit_graph, 1); - if (!repo_config_get_bool(r, "index.version", &value)) + if (!repo_config_get_int(r, "index.version", &value)) r->settings.index_version = value; if (!repo_config_get_maybe_bool(r, "core.untrackedcache", &value)) { if (value == 0) diff --git a/t/t1600-index.sh b/t/t1600-index.sh index c77721b580..b7c31aa86a 100755 --- a/t/t1600-index.sh +++ b/t/t1600-index.sh @@ -87,6 +87,10 @@ test_index_version () { } test_expect_success 'index version config precedence' ' + test_index_version 0 false 0 2 && + test_index_version 2 false 0 2 && + test_index_version 3 false 0 2 && + test_index_version 4 false 0 4 && test_index_version 2 false 4 4 && test_index_version 2 true 0 2 && test_index_version 0 true 0 4 && -- gitgitgadget ^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH 1/1] repo-settings: read an int for index.version 2019-10-23 20:38 ` [PATCH 1/1] repo-settings: read an int for index.version Derrick Stolee via GitGitGadget @ 2019-10-24 11:04 ` Philip Oakley 0 siblings, 0 replies; 3+ messages in thread From: Philip Oakley @ 2019-10-24 11:04 UTC (permalink / raw) To: Derrick Stolee via GitGitGadget, git; +Cc: Derrick Stolee, Junio C Hamano spelling nit. On 23/10/2019 21:38, Derrick Stolee via GitGitGadget wrote: > From: Derrick Stolee<dstolee@microsoft.com> > > Several config options were combined into a repo_settings struct in > ds/feature-macros, including a move of the "index.version" config > setting in 7211b9e (repo-settings: consolidate some config settings, > 2019-08-13). > > Unfortunately, that file looked like a lot of boilerplate and what is > clearly a factor of copy-paste overload, the config setting is parsed > with repo_config_ge_bool() instead of repo_config_get_int(). This means s/_ge_bool()/_get_bool()/ > that a setting "index.version=4" would not register correctly and would > revert to the default version of 3. > > I caught this while incorporating v2.24.0-rc0 into the VFS for Git > codebase, where we really care that the index is in version 4. > > This was not caught by the codebase because the version checks placed > in t1600-index.sh did not test the "basic" scenario enough. Here, we > modify the test to include these normal settings to not be overridden by > features.manyFiles or GIT_INDEX_VERSION. While the "default" version is > 3, this is demoted to version 2 in do_write_index() when not necessary. > > Signed-off-by: Derrick Stolee<dstolee@microsoft.com> > --- Philip ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2019-10-24 11:04 UTC | newest] Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-10-23 20:38 [PATCH 0/1] [v2.24.0-rc0 BUG] index.version is not parsed correctly Derrick Stolee via GitGitGadget 2019-10-23 20:38 ` [PATCH 1/1] repo-settings: read an int for index.version Derrick Stolee via GitGitGadget 2019-10-24 11:04 ` Philip Oakley
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).