fast-import: checkpoint: dump branches/tags/marks even if object_count==0
The checkpoint command cycles packfiles if object_count != 0, a sensible test or there would be no pack files to write. Since 820b931012, the command also dumps branches, tags and marks, but still conditionally. However, it is possible for a command stream to modify refs or create marks without creating any new objects. For example, reset a branch (and keep fast-import running): $ git fast-import reset refs/heads/master from refs/heads/master^ checkpoint but refs/heads/master remains unchanged. Other example: a commit command that re-creates an object that already exists in the object database. The man page also states that checkpoint "updates the refs" and that "placing a progress command immediately after a checkpoint will inform the reader when the checkpoint has been completed and it can safely access the refs that fast-import updated". This wasn't always true without this patch. This fix unconditionally calls dump_{branches,tags,marks}() for all checkpoint commands. dump_branches() and dump_tags() are cheap to call in the case of a no-op. Add tests to t9300 that observe the (non-packfiles) effects of checkpoint. Signed-off-by: Eric Rannaud <e@nanocritical.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Eric Rannaud committed
Sep 28, 2017 at 20:09 UTC
30e215a65c9f75e0f2a72c9f695389179bc30f72
2 files changed
+145
-3
fast-import.c
+3
-3
@@ -3188,10 +3188,10 @@ static void checkpoint(void)
3188
checkpoint_requested = 0;
3189
if (object_count) {
3190
cycle_packfile();
3191
- dump_branches();
3192
- dump_tags();
3193
- dump_marks();
3191
}
3192
+ dump_branches();
3193
+ dump_tags();
3194
+ dump_marks();
3195
}
3196
3197
static void parse_checkpoint(void)
t/t9300-fast-import.sh
+142
@@ -3120,4 +3120,146 @@ test_expect_success 'U: validate root delete result' '
3120
compare_diff_raw expect actual
3121
'
3122
3123
+###
3124
+### series V (checkpoint)
3125
+###
3126
+
3127
+# The commands in input_file should not produce any output on the file
3128
+# descriptor set with --cat-blob-fd (or stdout if unspecified).
3129
+#
3130
+# To make sure you're observing the side effects of checkpoint *before*
3131
+# fast-import terminates (and thus writes out its state), check that the
3132
+# fast-import process is still running using background_import_still_running
3133
+# *after* evaluating the test conditions.
3134
+background_import_then_checkpoint () {
3135
+ options=$1
3136
+ input_file=$2
3137
+
3138
+ mkfifo V.input
3139
+ exec 8<>V.input
3140
+ rm V.input
3141
+
3142
+ mkfifo V.output
3143
+ exec 9<>V.output
3144
+ rm V.output
3145
+
3146
+ git fast-import $options <&8 >&9 &
3147
+ echo $! >V.pid
3148
+ # We don't mind if fast-import has already died by the time the test
3149
+ # ends.
3150
+ test_when_finished "exec 8>&-; exec 9>&-; kill $(cat V.pid) || true"
3151
+
3152
+ # Start in the background to ensure we adhere strictly to (blocking)
3153
+ # pipes writing sequence. We want to assume that the write below could
3154
+ # block, e.g. if fast-import blocks writing its own output to &9
3155
+ # because there is no reader on &9 yet.
3156
+ (
3157
+ cat "$input_file"
3158
+ echo "checkpoint"
3159
+ echo "progress checkpoint"
3160
+ ) >&8 &
3161
+
3162
+ error=1 ;# assume the worst
3163
+ while read output <&9
3164
+ do
3165
+ if test "$output" = "progress checkpoint"
3166
+ then
3167
+ error=0
3168
+ break
3169
+ fi
3170
+ # otherwise ignore cruft
3171
+ echo >&2 "cruft: $output"
3172
+ done
3173
+
3174
+ if test $error -eq 1
3175
+ then
3176
+ false
3177
+ fi
3178
+}
3179
+
3180
+background_import_still_running () {
3181
+ if ! kill -0 "$(cat V.pid)"
3182
+ then
3183
+ echo >&2 "background fast-import terminated too early"
3184
+ false
3185
+ fi
3186
+}
3187
+
3188
+test_expect_success PIPE 'V: checkpoint helper does not get stuck with extra output' '
3189
+ cat >input <<-INPUT_END &&
3190
+ progress foo
3191
+ progress bar
3192
+
3193
+ INPUT_END
3194
+
3195
+ background_import_then_checkpoint "" input &&
3196
+ background_import_still_running
3197
+'
3198
+
3199
+test_expect_success PIPE 'V: checkpoint updates refs after reset' '
3200
+ cat >input <<-\INPUT_END &&
3201
+ reset refs/heads/V
3202
+ from refs/heads/U
3203
+
3204
+ INPUT_END
3205
+
3206
+ background_import_then_checkpoint "" input &&
3207
+ test "$(git rev-parse --verify V)" = "$(git rev-parse --verify U)" &&
3208
+ background_import_still_running
3209
+'
3210
+
3211
+test_expect_success PIPE 'V: checkpoint updates refs and marks after commit' '
3212
+ cat >input <<-INPUT_END &&
3213
+ commit refs/heads/V
3214
+ mark :1
3215
+ committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
3216
+ data 0
3217
+ from refs/heads/U
3218
+
3219
+ INPUT_END
3220
+
3221
+ background_import_then_checkpoint "--export-marks=marks.actual" input &&
3222
+
3223
+ echo ":1 $(git rev-parse --verify V)" >marks.expected &&
3224
+
3225
+ test "$(git rev-parse --verify V^)" = "$(git rev-parse --verify U)" &&
3226
+ test_cmp marks.expected marks.actual &&
3227
+ background_import_still_running
3228
+'
3229
+
3230
+# Re-create the exact same commit, but on a different branch: no new object is
3231
+# created in the database, but the refs and marks still need to be updated.
3232
+test_expect_success PIPE 'V: checkpoint updates refs and marks after commit (no new objects)' '
3233
+ cat >input <<-INPUT_END &&
3234
+ commit refs/heads/V2
3235
+ mark :2
3236
+ committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
3237
+ data 0
3238
+ from refs/heads/U
3239
+
3240
+ INPUT_END
3241
+
3242
+ background_import_then_checkpoint "--export-marks=marks.actual" input &&
3243
+
3244
+ echo ":2 $(git rev-parse --verify V2)" >marks.expected &&
3245
+
3246
+ test "$(git rev-parse --verify V2)" = "$(git rev-parse --verify V)" &&
3247
+ test_cmp marks.expected marks.actual &&
3248
+ background_import_still_running
3249
+'
3250
+
3251
+test_expect_success PIPE 'V: checkpoint updates tags after tag' '
3252
+ cat >input <<-INPUT_END &&
3253
+ tag Vtag
3254
+ from refs/heads/V
3255
+ tagger $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
3256
+ data 0
3257
+
3258
+ INPUT_END
3259
+
3260
+ background_import_then_checkpoint "" input &&
3261
+ git show-ref -d Vtag &&
3262
+ background_import_still_running
3263
+'
3264
+
3265
test_done