From: "Eric Rannaud" <e@nanocritical.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org, jeremy.serror@gmail.com,
"Shawn O . Pearce" <spearce@spearce.org>,
Ramsay Jones <ramsay@ramsayjones.plus.com>,
Eric Rannaud <e@nanocritical.com>
Subject: [PATCH 1/1] fast-import: checkpoint: dump branches/tags/marks even if object_count==0
Date: Wed, 27 Sep 2017 22:07:41 -0700 [thread overview]
Message-ID: <5b6001e7db3c57fdd9e6354ed4294e2192524061.1506574951.git.e@nanocritical.com> (raw)
In-Reply-To: <CAH_=xoZfuJDE515zZbry1DdGV+YeySy+5T9QOBefO-MMTSvXSA@mail.gmail.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 | 126 +++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 129 insertions(+), 3 deletions(-)
Updated to include Junio's latest remarks.
Also adding the necessary PIPE prereq, as pointed out by Ramsay Jones.
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..b8d394548520 100755
--- a/t/t9300-fast-import.sh
+++ b/t/t9300-fast-import.sh
@@ -3120,4 +3120,130 @@ test_expect_success 'U: validate root delete result' '
compare_diff_raw expect actual
'
+###
+### series V (checkpoint)
+###
+
+# The commands in input_file should not produce any output on the file
+# descriptor set with --cat-blob-fd (or stdout if unspecified).
+#
+# 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_then_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
+
+ git fast-import $options <&8 >&9 &
+ echo $! >V.pid
+ # We don't mind if fast-import has already died by the time the test
+ # ends.
+ test_when_finished "exec 8>&-; exec 9>&-; kill $(cat V.pid) || true"
+
+ cat "$input_file" >&8
+ echo "checkpoint" >&8
+ echo "progress checkpoint" >&8
+
+ 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
+
+ if test $error -eq 1
+ then
+ false
+ fi
+}
+
+background_import_still_running () {
+ if ! kill -0 "$(cat V.pid)"
+ then
+ echo >&2 "background fast-import terminated too early"
+ false
+ fi
+}
+
+test_expect_success PIPE 'V: checkpoint updates refs after reset' '
+ cat >input <<-\INPUT_END &&
+ reset refs/heads/V
+ from refs/heads/U
+
+ INPUT_END
+
+ background_import_then_checkpoint "" input &&
+ test "$(git rev-parse --verify V)" = "$(git rev-parse --verify U)" &&
+ background_import_still_running
+'
+
+test_expect_success PIPE '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
+
+ INPUT_END
+
+ background_import_then_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 PIPE '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
+
+ INPUT_END
+
+ background_import_then_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 PIPE '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
+
+ INPUT_END
+
+ background_import_then_checkpoint "" input &&
+ git show-ref -d Vtag &&
+ background_import_still_running
+'
+
test_done
--
2.14.1
next prev parent reply other threads:[~2017-09-28 5:07 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-09-26 3:30 [PATCH] fast-import: checkpoint: dump branches/tags/marks even if object_count==0 Eric Rannaud
2017-09-26 4:25 ` Junio C Hamano
2017-09-26 9:53 ` [PATCH 1/1] " Eric Rannaud
2017-09-27 3:37 ` Junio C Hamano
2017-09-27 19:46 ` [PATCH] " Eric Rannaud
2017-09-27 23:19 ` Ramsay Jones
2017-09-28 3:48 ` Junio C Hamano
2017-09-28 4:56 ` Eric Rannaud
2017-09-28 5:07 ` Eric Rannaud [this message]
2017-09-28 10:35 ` [PATCH 1/1] " Junio C Hamano
2017-09-28 20:30 ` Eric Rannaud
2017-09-28 12:59 ` Adam Dinwoodie
2017-09-28 21:03 ` [PATCH] " Eric Rannaud
2017-09-29 2:44 ` [PATCH 1/1] " Junio C Hamano
2017-09-29 3:09 ` [PATCH] " Eric Rannaud
2017-09-29 3:51 ` Junio C Hamano
2017-09-29 5:40 ` Eric Rannaud
2017-09-29 9:35 ` Junio C Hamano
2017-09-28 6:02 ` Junio C Hamano
2017-09-28 6:44 ` Eric Rannaud
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=5b6001e7db3c57fdd9e6354ed4294e2192524061.1506574951.git.e@nanocritical.com \
--to=e@nanocritical.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=jeremy.serror@gmail.com \
--cc=ramsay@ramsayjones.plus.com \
--cc=spearce@spearce.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
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 read-only IMAP folder(s) and NNTP newsgroup(s).