fast-import: only allow cat-blob requests where it makes sense

In commit 777f80d7429b ("fast-import: Allow cat-blob requests at arbitrary points in stream", 2010-11-28), fast-import started allowing cat-blob commands to appear on the start of any line except in the middle of a "data" command. It could be in the middle of various directives that were part of a tag command, or in the middle of checkpoints or progresses (each of which allow an optional second empty newline), or even immediately after the mark command of a blob before the data directive appeared (raising the question of what if it used the mark for the blob that just barely appeared in the stream that we do not yet have the data for). None of these locations make any sense as places to put cat-blob requests. The purpose of this change as stated in that commit message was to [save] frontends from having to loop over everything they want to commit in the next commit and cat-ing the necessary objects in advance. However, that can be achieved by simply allowing cat-blob requests to appear whenever a filemodify directive is allowed. Further, it avoids setting a bad precedent for other commands to follow (e.g. get-mark); a precedent which caused parsing problems in corner cases. Technically, inline filemodify directives add a slight wrinkle in that frontends might want to have cat-blob directives appear after the start of the filemodify and before the data directive contained within it. I think it would have been better to disallow such a case (it would be trivial to use cat-blob before the filemodify instead), but since there is evidence this was used, for backwards compatibility let's support that case too. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Elijah Newren committed Feb 20, 2019 at 14:58 UTC 7ffde293f2e7f0ae455800b138416da601254436
2 files changed +17 -9
Documentation/git-fast-import.txt
+4 -3
@@ -1001,9 +1001,10 @@ Output uses the same format as `git cat-file --batch`:
1001 <contents> LF
1002 ====
1003
1004 -This command can be used anywhere in the stream that comments are
1005 -accepted. In particular, the `cat-blob` command can be used in the
1006 -middle of a commit but not in the middle of a `data` command.
1004 +This command can be used where a `filemodify` directive can appear,
1005 +allowing it to be used in the middle of a commit. For a `filemodify`
1006 +using an inline directive, it can also appear right before the `data`
1007 +directive.
1008
1009 See ``Responses To Commands'' below for details about how to read
1010 this output safely.
fast-import.c
+13 -6
@@ -1786,10 +1786,6 @@ static int read_next_command(void)
1786 parse_get_mark(p);
1787 continue;
1788 }
1789 - if (skip_prefix(command_buf.buf, "cat-blob ", &p)) {
1790 - parse_cat_blob(p);
1791 - continue;
1792 - }
1789 if (command_buf.buf[0] == '#')
1790 continue;
1791 return 0;
@@ -2254,8 +2250,15 @@ static void file_change_m(const char *p, struct branch *b)
2250 strbuf_addstr(&uq, p);
2251 p = uq.buf;
2252 }
2257 - read_next_command();
2258 - parse_and_store_blob(&last_blob, &oid, 0);
2253 + while (read_next_command() != EOF) {
2254 + const char *v;
2255 + if (skip_prefix(command_buf.buf, "cat-blob ", &v))
2256 + parse_cat_blob(v);
2257 + else {
2258 + parse_and_store_blob(&last_blob, &oid, 0);
2259 + break;
2260 + }
2261 + }
2262 } else {
2263 enum object_type expected = S_ISDIR(mode) ?
2264 OBJ_TREE: OBJ_BLOB;
@@ -2627,6 +2630,8 @@ static void parse_new_commit(const char *arg)
2630 file_change_deleteall(b);
2631 else if (skip_prefix(command_buf.buf, "ls ", &v))
2632 parse_ls(v, b);
2633 + else if (skip_prefix(command_buf.buf, "cat-blob ", &v))
2634 + parse_cat_blob(v);
2635 else {
2636 unread_command_buf = 1;
2637 break;
@@ -3311,6 +3316,8 @@ int cmd_main(int argc, const char **argv)
3316 parse_reset_branch(v);
3317 else if (skip_prefix(command_buf.buf, "ls ", &v))
3318 parse_ls(v, NULL);
3319 + else if (skip_prefix(command_buf.buf, "cat-blob ", &v))
3320 + parse_cat_blob(v);
3321 else if (!strcmp("checkpoint", command_buf.buf))
3322 parse_checkpoint();
3323 else if (!strcmp("done", command_buf.buf))