git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH 01/13] fast-import: add the 'done' command

From
Jonathan Nieder <jrnieder@gmail.com>
Date
Feb 13, 2011, 09:42 UTC
Message-ID
<20110213094212.GA25435@elie>
In-Reply-To
<1283053540-27042-2-git-send-email-srabbelier@gmail.com>
Hi Sverre et al,
Sverre Rabbelier wrote:
> Currently the only way to end an import stream is to close it, which
> is not desirable when the stream that's being used is shared.

Here's a variation on the same theme, with notes indicating what remains to be fixed. Maybe it can save someone some time.

-- 8< --
From: Sverre Rabbelier <srabbelier@gmail.com>
Date: Sat, 28 Aug 2010 22:45:28 -0500
Subject: fast-import: introduce 'done' command

Add a 'done' command that causes fast-import to stop reading from the stream and exit.

If the new --done command line flag was passed on the command line (or a "feature done" declaration included at the start of the stream), make the 'done' command mandatory. So "git fast-import --done"'s input format will be prefix-free, making errors easier to detect when they show up as early termination at some convenient time of the upstream of a pipe writing to fast-import.

Another possible application of the 'done' command would to be allow a fast-import stream that is only a small part of a larger encapsulating stream to be easily parsed, leaving the file offset after the "done\n" so the other application can pick up from there. This patch does not teach fast-import to do that --- fast-import still uses buffered input (stdio).

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
 Documentation/git-fast-import.txt |   25 ++++++++++++++++++++++
 fast-import.c                     |   14 ++++++++++++
 t/t9300-fast-import.sh            |   42 +++++++++++++++++++++++++++++++++++++
 3 files changed, 81 insertions(+), 0 deletions(-)
diff --git a/Documentation/git-fast-import.txt b/Documentation/git-fast-import.txt
index c3a2766..d0efdf8 100644
--- a/Documentation/git-fast-import.txt
+++ b/Documentation/git-fast-import.txt
@@ -101,6 +101,12 @@ OPTIONS
 	when the `cat-blob` command is encountered in the stream.
 	The default behaviour is to write to `stdout`.
 
+--done::
+	Require a `done` command at the end of the stream.
+	This option might be useful for detecting errors that
+	cause the frontend to terminate before it has started to
+	write a stream.
+
 --export-pack-edges=<file>::
 	After creating a packfile, print a line of data to
 	<file> listing the filename of the packfile and the last
@@ -329,6 +335,11 @@ and control the current import process.  More detailed discussion
 	standard output.  This command is optional and is not needed
 	to perform an import.
 
+`done`::
+	Marks the end of the stream. This command is optional
+	unless the `done` feature was requested using the
+	`--done` command line option or `feature done` command.
+
 `cat-blob`::
 	Causes fast-import to print a blob in 'cat-file --batch'
 	format to the file descriptor set with `--cat-blob-fd` or
@@ -958,6 +969,11 @@ notes::
 	Versions of fast-import not supporting notes will exit
 	with a message indicating so.
 
+done::
+	Error out if the stream ends without a 'done' command.
+	Without this feature, errors causing the frontend to end
+	abruptly at a convenient point in the stream can go
+	undetected.
 
 `option`
 ~~~~~~~~
@@ -987,6 +1003,15 @@ not be passed as option:
 * cat-blob-fd
 * force
 
+`done`
+~~~~~~
+If the `done` feature is not in use, treated as if EOF was read.
+This can be used to tell fast-import to finish early.
+
+If the `--done` command line option or `feature done` command is
+in use, the `done` command is mandatory and marks the end of the
+stream.
+
 Crash Reports
 -------------
 If fast-import is supplied invalid input it will terminate with a
diff --git a/fast-import.c b/fast-import.c
index 3886a1b..cbcf61f 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -365,6 +365,7 @@ static unsigned int cmd_save = 100;
 static uintmax_t next_mark;
 static struct strbuf new_data = STRBUF_INIT;
 static int seen_data_command;
+static int require_explicit_termination;
 
 /* Signal handling */
 static volatile sig_atomic_t checkpoint_requested;
@@ -2999,6 +3000,8 @@ static int parse_one_feature(const char *feature, int from_stream)
 		relative_marks_paths = 1;
 	} else if (!prefixcmp(feature, "no-relative-marks")) {
 		relative_marks_paths = 0;
+	} else if (!strcmp(feature, "done")) {
+		require_explicit_termination = 1;
 	} else if (!prefixcmp(feature, "force")) {
 		force_update = 1;
 	} else if (!strcmp(feature, "notes")) {
@@ -3150,6 +3153,8 @@ int main(int argc, const char **argv)
 			parse_reset_branch();
 		else if (!strcmp("checkpoint", command_buf.buf))
 			parse_checkpoint();
+		else if (!strcmp("done", command_buf.buf))
+			break;
 		else if (!prefixcmp(command_buf.buf, "progress "))
 			parse_progress();
 		else if (!prefixcmp(command_buf.buf, "feature "))
@@ -3169,6 +3174,15 @@ int main(int argc, const char **argv)
 	if (!seen_data_command)
 		parse_argv();
 
+	/*
+	 * NEEDSWORK: we should report input errors before
+	 * errno has a chance to be clobbered.
+	 */
+	if (ferror(stdin))
+		die("error reading input");
+	if (require_explicit_termination && feof(stdin))
+		die("stream ends early");
+
 	end_packfile();
 
 	dump_branches();
diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh
index 52ac0e5..a366ee2 100755
--- a/t/t9300-fast-import.sh
+++ b/t/t9300-fast-import.sh
@@ -2121,6 +2121,48 @@ test_expect_success 'R: quiet option results in no stats being output' '
     test_cmp empty output
 '
 
+test_expect_success 'R: feature done means terminating "done" is mandatory' '
+	echo feature done | test_must_fail git fast-import &&
+	test_must_fail git fast-import --done </dev/null
+'
+
+test_expect_success 'R: terminating "done" with trailing gibberish is ok' '
+	git fast-import <<-\EOF &&
+	feature done
+	done
+	trailing gibberish
+	EOF
+	git fast-import <<-\EOF
+	done
+	more trailing gibberish
+	EOF
+'
+
+test_expect_success 'R: terminating "done" within commit' '
+	cat >expect <<-\EOF &&
+	OBJID
+	:000000 100644 OBJID OBJID A	hello.c
+	:000000 100644 OBJID OBJID A	hello2.c
+	EOF
+	git fast-import <<-EOF &&
+	commit refs/heads/done-ends
+	committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
+	data <<EOT
+	Commit terminated by "done" command
+	EOT
+	M 100644 inline hello.c
+	data <<EOT
+	Hello, world.
+	EOT
+	C hello.c hello2.c
+	done
+	EOF
+	git rev-list done-ends |
+	git diff-tree -r --stdin --root --always |
+	sed -e "s/$_x40/OBJID/g" >actual &&
+	test_cmp expect actual
+'
+
 cat >input <<EOF
 option git non-existing-option
 EOF

-- 
1.7.4.1
Previous: Jonathan NiederNext: Sverre Rabbelier
Message 12 of 52 in “remote helper improvements”
  1. 00/13 remote helper improvementsSverre Rabbelier, Aug 29, 2010
  2. 01/13 fast-import: add the 'done' commandSverre Rabbelier, Aug 29, 2010
  3. Daniel BarkalowAug 29, 2010
  4. Sverre RabbelierAug 29, 2010
  5. Jonathan NiederAug 29, 2010
  6. Sverre RabbelierAug 29, 2010
  7. Jonathan NiederAug 29, 2010
  8. Sverre RabbelierAug 30, 2010
  9. Jonathan NiederAug 30, 2010
  10. Sverre RabbelierAug 30, 2010
  11. Jonathan NiederAug 30, 2010
  12. Jonathan NiederFeb 13, 2011
  13. 02/13 fast-export: support done featureSverre Rabbelier, Aug 29, 2010
  14. Daniel BarkalowAug 29, 2010
  15. Sverre RabbelierAug 29, 2010
  16. Tay Ray ChuanAug 29, 2010
  17. Sverre RabbelierAug 30, 2010
  18. 03/13 transport-helper: factor out push_update_refs_statusSverre Rabbelier, Aug 29, 2010
  19. Jonathan NiederAug 29, 2010
  20. Sverre RabbelierAug 29, 2010
  21. 04/13 transport-helper: check status code of finish_commandSverre Rabbelier, Aug 29, 2010
  22. Jonathan NiederAug 29, 2010
  23. 05/13 transport-helper: use the new done feature to properly do importsSverre Rabbelier, Aug 29, 2010
  24. Jonathan NiederAug 29, 2010
  25. Sverre RabbelierAug 30, 2010
  26. 06/13 transport-helper: update ref status after push with exportSverre Rabbelier, Aug 29, 2010
  27. Jonathan NiederAug 29, 2010
  28. Sverre RabbelierAug 30, 2010
  29. 07/13 transport-helper: change import semanticsSverre Rabbelier, Aug 29, 2010
  30. Daniel BarkalowAug 29, 2010
  31. Sverre RabbelierAug 29, 2010
  32. 08/13 transport-helper: export should disconnect tooSverre Rabbelier, Aug 29, 2010
  33. Daniel BarkalowAug 29, 2010
  34. Sverre RabbelierAug 29, 2010
  35. 09/13 transport-helper: Use capname for gitdir capability tooSverre Rabbelier, Aug 29, 2010
  36. Jonathan NiederAug 30, 2010
  37. 10/13 transport-helper: implement marks location as capabilitySverre Rabbelier, Aug 29, 2010
  38. Daniel BarkalowAug 29, 2010
  39. Sverre RabbelierAug 29, 2010
  40. Jonathan NiederAug 30, 2010
  41. Sverre RabbelierAug 30, 2010
  42. 11/13 remote-curl: accept empty line as terminatorSverre Rabbelier, Aug 29, 2010
  43. Jonathan NiederAug 30, 2010
  44. Sverre RabbelierAug 30, 2010
  45. 12/13 git-remote-testgit: only push for non-local repositoriesSverre Rabbelier, Aug 29, 2010
  46. Jonathan NiederAug 30, 2010
  47. Sverre RabbelierAug 30, 2010
  48. Jonathan NiederAug 30, 2010
  49. 13/13 git-remote-testgit: fix error handlingSverre Rabbelier, Aug 29, 2010
  50. Jonathan NiederAug 30, 2010
  51. Sverre RabbelierAug 30, 2010
  52. Sverre RabbelierAug 30, 2010

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.