[PATCH] fast-import: checkpoint: dump branches/tags/marks even if object_count==0
- From
Eric Rannaud <e@nanocritical.com>
- Date
- Sep 27, 2017, 19:46 UTC
- Message-ID
- <cccb06b75df3cad9f013d5a9ab0371f0a2d9c2ce.1506541322.git.e@nanocritical.com>
- In-Reply-To
- <xmqqefqs7qx8.fsf@gitster.mtv.corp.google.com>
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> --- fast-import.c | 6 +-- t/t9300-fast-import.sh | 129 +++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 132 insertions(+), 3 deletions(-)
Use named pipes instead of the polling approach. Also incorporate your other comments.
diff --git a/fast-import.c b/fast-import.c index 35bf671f12c4..d5e4cf0bad41 100644 --- a/fast-import.c +++ b/fast-import.c @@ -3189,10 +3189,10 @@ static void checkpoint(void) checkpoint_requested = 0; if (object_count) { cycle_packfile(); - dump_branches(); - dump_tags(); - dump_marks(); } + dump_branches(); + dump_tags(); + dump_marks(); } static void parse_checkpoint(void) diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh index 67b8c50a5ab4..9aa3470d895b 100755 --- a/t/t9300-fast-import.sh +++ b/t/t9300-fast-import.sh @@ -3120,4 +3120,133 @@ test_expect_success 'U: validate root delete result' ' compare_diff_raw expect actual ' +### +### series V (checkpoint) +### + +# To make sure you're observing the side effects of checkpoint *before* +# fast-import terminates (and thus writes out its state), check that the +# fast-import process is still running using background_import_still_running +# *after* evaluating the test conditions. +background_import_until_checkpoint () { + options=$1 + input_file=$2 + + mkfifo V.input + exec 8<>V.input + rm V.input + + mkfifo V.output + exec 9<>V.output + rm V.output + + cat $input_file >&8 + git fast-import $options <&8 >&9 & + echo $! >V.pid + test_when_finished "kill $(cat V.pid) || true" + + error=0 + if read output <&9 + then + if ! test "$output" = "progress checkpoint" + then + echo >&2 "no progress checkpoint received: $output" + error=1 + fi + else + echo >&2 "failed to read fast-import output" + error=1 + fi + + exec 8>&- + exec 9>&- + + if test $error -eq 1 + then + exit 1 + fi +} + +background_import_still_running () { + if ! kill -0 "$(cat V.pid)" + then + echo >&2 "background fast-import terminated too early" + exit 1 + fi +} + +test_expect_success 'V: checkpoint updates refs after reset' ' + cat >input <<-\INPUT_END && + reset refs/heads/V + from refs/heads/U + + checkpoint + progress checkpoint + INPUT_END + + background_import_until_checkpoint "" input && + test "$(git rev-parse --verify V)" = "$(git rev-parse --verify U)" && + background_import_still_running +' + +test_expect_success 'V: checkpoint updates refs and marks after commit' ' + cat >input <<-INPUT_END && + commit refs/heads/V + mark :1 + committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE + data 0 + from refs/heads/U + + checkpoint + progress checkpoint + INPUT_END + + background_import_until_checkpoint "--export-marks=marks.actual" input && + + echo ":1 $(git rev-parse --verify V)" >marks.expected && + + test "$(git rev-parse --verify V^)" = "$(git rev-parse --verify U)" && + test_cmp marks.expected marks.actual && + background_import_still_running +' + +# Re-create the exact same commit, but on a different branch: no new object is +# created in the database, but the refs and marks still need to be updated. +test_expect_success 'V: checkpoint updates refs and marks after commit (no new objects)' ' + cat >input <<-INPUT_END && + commit refs/heads/V2 + mark :2 + committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE + data 0 + from refs/heads/U + + checkpoint + progress checkpoint + INPUT_END + + background_import_until_checkpoint "--export-marks=marks.actual" input && + + echo ":2 $(git rev-parse --verify V2)" >marks.expected && + + test "$(git rev-parse --verify V2)" = "$(git rev-parse --verify V)" && + test_cmp marks.expected marks.actual && + background_import_still_running +' + +test_expect_success 'V: checkpoint updates tags after tag' ' + cat >input <<-INPUT_END && + tag Vtag + from refs/heads/V + tagger $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE + data 0 + + checkpoint + progress checkpoint + INPUT_END + + background_import_until_checkpoint "" input && + git show-ref -d Vtag && + background_import_still_running +' + test_done
-- 2.14.1