On 7/12/2018 9:19 AM, Derrick Stolee wrote:
On 7/6/2018 1:39 AM, Eric Sunshine wrote:
On Thu, Jul 5, 2018 at 8:54 PM Derrick Stolee <sto...@gmail.com> wrote:
The core.multiPackIndex config setting controls the multi-pack-
index (MIDX) feature. If false, the setting will disable all reads
from the multi-pack-index file.
Add comparison commands in t5319-multi-pack-index.sh to check
typical Git behavior remains the same as the config setting is turned
on and off. This currently includes 'git rev-list' and 'git log'
commands to trigger several object database reads.
Signed-off-by: Derrick Stolee <dsto...@microsoft.com>
---
diff --git a/cache.h b/cache.h
@@ -814,6 +814,7 @@ extern char *git_replace_ref_base;
+extern int core_multi_pack_index;
diff --git a/config.c b/config.c
@@ -1313,6 +1313,11 @@ static int git_default_core_config(const char
*var, const char *value)
+ if (!strcmp(var, "core.multipackindex")) {
+ core_multi_pack_index = git_config_bool(var, value);
+ return 0;
+ }
This is a rather unusual commit. This new configuration is assigned,
but it's never actually consulted, which means that it's impossible
for it to have any impact on functionality, yet tests are being added
to check whether it _did_ have any impact on functionality. Confusing.
Patch 17 does consult 'core_multi_pack_index', so it's only at that
point that it could have any impact. This situation would be less
confusing if you swapped patches 16 and 17 (and, of course, move the
declaration of 'core_multi_pack_index' to patch 17 with a reasonable
default value).
You're right that this commit is a bit too aware of the future, but I
disagree with the recommendation to change it.
Yes, in this commit there is no possible way that these tests could
fail. The point is that patches 17-23 all change behavior if this
setting is on, and we want to make sure we do not break at any point
along that journey (or in future iterations of the multi-pack-index
feature).
With this in mind, I don't think there is a better commit to place
these tests.
Of course, as I convert this global config variable into an on-demand
check as promised [1] this commit seems even more trivial. I'm going to
squash it with PATCH 17.
[1]
https://public-inbox.org/git/b5733625-29c8-4317-ff44-d27c2fca1...@gmail.com/
