transfer.unpackLimit: fetch/receive.unpackLimit takes precedence
The transfer.unpackLimit configuration variable is documented to be used only as a fallback value when the more operation-specific fetch.unpackLimit and receive.unpackLimit variables are not set, but the implementation had the precedence reversed. Apparently this was broken since the transfer.unpackLimit was introduced in e28714c5 (Consolidate {receive,fetch}.unpackLimit, 2007-01-24). Often when documentation and code have diverged for so long, we prefer to change the documentation instead, to avoid disrupting users. But doing so would make these weirdly unlike most other "specific overrides general" config options. And the fact that the bug has existed for so long without anyone noticing implies to me that nobody really tries to mix and match them much. Signed-off-by: Taylor Santiago <taylorsantiago@google.com> [jc: rewrote the log message, added tests, covered receive-pack as well] Helped-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Junio C Hamano committed
Aug 22, 2023 at 18:30 UTC
f3d33f8cfeac8a2a00b4f87eec48dbca1d3080e0
4 files changed
+84
-9
builtin/receive-pack.c
+3
-3
@@ -2524,10 +2524,10 @@ int cmd_receive_pack(int argc, const char **argv, const char *prefix)
2524
if (cert_nonce_seed)
2525
push_cert_nonce = prepare_push_cert_nonce(service_dir, time(NULL));
2526
2527
- if (0 <= transfer_unpack_limit)
2528
- unpack_limit = transfer_unpack_limit;
2529
- else if (0 <= receive_unpack_limit)
2527
+ if (0 <= receive_unpack_limit)
2528
unpack_limit = receive_unpack_limit;
2529
+ else if (0 <= transfer_unpack_limit)
2530
+ unpack_limit = transfer_unpack_limit;
2531
2532
switch (determine_protocol_version_server()) {
2533
case protocol_v2:
fetch-pack.c
+3
-3
@@ -1911,10 +1911,10 @@ static void fetch_pack_setup(void)
1911
if (did_setup)
1912
return;
1913
fetch_pack_config();
1914
- if (0 <= transfer_unpack_limit)
1915
- unpack_limit = transfer_unpack_limit;
1916
- else if (0 <= fetch_unpack_limit)
1914
+ if (0 <= fetch_unpack_limit)
1915
unpack_limit = fetch_unpack_limit;
1916
+ else if (0 <= transfer_unpack_limit)
1917
+ unpack_limit = transfer_unpack_limit;
1918
did_setup = 1;
1919
}
1920
t/t5510-fetch.sh
+46
@@ -1127,6 +1127,52 @@ do
1127
'
1128
done
1129
1130
+test_expect_success 'prepare source branch' '
1131
+ echo one >onebranch &&
1132
+ git checkout --orphan onebranch &&
1133
+ git rm --cached -r . &&
1134
+ git add onebranch &&
1135
+ git commit -m onebranch &&
1136
+ git rev-list --objects onebranch -- >actual &&
1137
+ # 3 objects should be created, at least ...
1138
+ test 3 -le $(wc -l <actual)
1139
+'
1140
+
1141
+validate_store_type () {
1142
+ git -C dest count-objects -v >actual &&
1143
+ case "$store_type" in
1144
+ packed)
1145
+ grep "^count: 0$" actual ;;
1146
+ loose)
1147
+ grep "^packs: 0$" actual ;;
1148
+ esac || {
1149
+ echo "store_type is $store_type"
1150
+ cat actual
1151
+ false
1152
+ }
1153
+}
1154
+
1155
+test_unpack_limit () {
1156
+ store_type=$1
1157
+
1158
+ case "$store_type" in
1159
+ packed) fetch_limit=1 transfer_limit=10000 ;;
1160
+ loose) fetch_limit=10000 transfer_limit=1 ;;
1161
+ esac
1162
+
1163
+ test_expect_success "fetch trumps transfer limit" '
1164
+ rm -fr dest &&
1165
+ git --bare init dest &&
1166
+ git -C dest config fetch.unpacklimit $fetch_limit &&
1167
+ git -C dest config transfer.unpacklimit $transfer_limit &&
1168
+ git -C dest fetch .. onebranch &&
1169
+ validate_store_type
1170
+ '
1171
+}
1172
+
1173
+test_unpack_limit packed
1174
+test_unpack_limit loose
1175
+
1176
setup_negotiation_tip () {
1177
SERVER="$1"
1178
URL="$2"
t/t5546-receive-limits.sh
+32
-3
@@ -9,10 +9,26 @@ TEST_PASSES_SANITIZE_LEAK=true
9
# When the limit is 1, `git receive-pack` will call `git index-pack`.
10
# When the limit is 10000, `git receive-pack` will call `git unpack-objects`.
11
12
+validate_store_type () {
13
+ git -C dest count-objects -v >actual &&
14
+ case "$store_type" in
15
+ index)
16
+ grep "^count: 0$" actual ;;
17
+ unpack)
18
+ grep "^packs: 0$" actual ;;
19
+ esac || {
20
+ echo "store_type is $store_type"
21
+ cat actual
22
+ false;
23
+ }
24
+}
25
+
26
test_pack_input_limit () {
13
- case "$1" in
14
- index) unpack_limit=1 ;;
15
- unpack) unpack_limit=10000 ;;
27
+ store_type=$1
28
+
29
+ case "$store_type" in
30
+ index) unpack_limit=1 other_limit=10000 ;;
31
+ unpack) unpack_limit=10000 other_limit=1 ;;
32
esac
33
34
test_expect_success 'prepare destination repository' '
@@ -43,6 +59,19 @@ test_pack_input_limit () {
59
git --git-dir=dest config receive.maxInputSize 0 &&
60
git push dest HEAD
61
'
62
+
63
+ test_expect_success 'prepare destination repository (once more)' '
64
+ rm -fr dest &&
65
+ git --bare init dest
66
+ '
67
+
68
+ test_expect_success 'receive trumps transfer' '
69
+ git --git-dir=dest config receive.unpacklimit "$unpack_limit" &&
70
+ git --git-dir=dest config transfer.unpacklimit "$other_limit" &&
71
+ git push dest HEAD &&
72
+ validate_store_type
73
+ '
74
+
75
}
76
77
test_expect_success "create known-size (1024 bytes) commit" '