{"thread":{"id":"25454","subject":"[PATCH 5/5] svn-fe: Use the cat-blob command to apply deltas","startedAt":"2010-10-15T12:54:11Z","lastAt":"2011-01-16T02:16:05Z","messageCount":34,"participants":["David Barr","Ramkumar Ramachandra","Jonathan Nieder","Sverre Rabbelier","Junio C Hamano","Thomas Rast"],"isPatch":true,"patchVersion":1,"patchTotal":5},"messages":[{"id":"153582","messageId":"1287147256-9457-1-git-send-email-david.barr@cordelta.com","threadId":"25454","inReplyTo":null,"subject":"[PATCHv2] Add support for subversion dump format v3","fromName":"David Barr","fromEmail":"david.barr@cordelta.com","sentAt":"2010-10-15T12:54:11Z","receivedAt":"2010-10-15T12:54:11Z","isPatch":false,"sender":{"key":"david.barr@cordelta.com","avatar":"https://avatars.githubusercontent.com/u/220594?v=4"},"body":"This series follows Jonathan Nieder's svn diff applier series.\n\nPatch 1 adds the required infrastructure to fast-import.\nThis features the addition of the cat-blob command to\nfast-import. This allows access to blobs written to the\nthe current pack prior to a checkpoint and is critical to\nretrieving full-texts to drive the diff applier.\n\nPatch 2 adds the basic parsing necessary to process the v3 format.\n\nPatch 3 adds logic around decoding prop deltas.\n\nPatch 5 integrates svn-fe with svn-da to decode text deltas.\nIt is based on a large patch authored by Jonathan and inspired by Ram.\nIt has been heavily trimmed to reduce code bloat and enabled me to\ndetermine the logic for the previous patches.\n\nA bit shout-out to Jonathan Nieder and Ramkumar Ramachandra for\ntheir help in bring this series into existence.\n\nI have tried to incorporate all the feedback on the list and over\nat #git-devel. I hope to impress.\n\n--\nDavid Barr.\n"},{"id":"153581","messageId":"1287147256-9457-2-git-send-email-david.barr@cordelta.com","threadId":"25454","inReplyTo":"1287147256-9457-1-git-send-email-david.barr@cordelta.com","subject":"[PATCH 1/5] fast-import: Let importers retrieve blobs","fromName":"David Barr","fromEmail":"david.barr@cordelta.com","sentAt":"2010-10-15T12:54:12Z","receivedAt":"2010-10-15T12:54:12Z","isPatch":true,"sender":{"key":"david.barr@cordelta.com","avatar":"https://avatars.githubusercontent.com/u/220594?v=4"},"body":"As the description of the \"progress\" option in git-fast-import.1\nhints, there is no convenient way to immediately access the blobs\nwritten to a new repository through fast-import.  Until a checkpoint\nhas been started and finishes writing the pack index, any new blobs\nwill not be accessible using standard git tools.\n\nSo introduce another way: a \"cat-blob\" command introduced in the\ncommand stream requests for fast-import to print a blob to stdout\nor a file descriptor specified by the argument --cat-blob-fd.\n\nThe output uses the same format as \"git cat-file --batch\".\n\nCc: Shawn O. Pearce <spearce@spearce.org>\nCc: Ramkumar Ramachandra <artagnon@gmail.com>\nHelped-by: Sverre Rabbelier <srabbelier@gmail.com>\nBased-on-patch-by: Jonathan Nieder <jrnieder@gmail.com>\nSigned-off-by: David Barr <david.barr@cordelta.com>\n---\n Documentation/git-fast-import.txt |   34 ++++++++++++++\n fast-import.c                     |   92 +++++++++++++++++++++++++++++++++++++\n 2 files changed, 126 insertions(+), 0 deletions(-)\n\ndiff --git a/Documentation/git-fast-import.txt b/Documentation/git-fast-import.txt\nindex 966ba4f..42a100b 100644\n--- a/Documentation/git-fast-import.txt\n+++ b/Documentation/git-fast-import.txt\n@@ -92,6 +92,17 @@ OPTIONS\n \t--(no-)-relative-marks= with the --(import|export)-marks=\n \toptions.\n \n+--cat-blob-fd=<fd>::\n+\tSpecify the file descriptor that will be written to\n+\twhen the `cat-blob` command is encountered in the stream.\n+\tThe default behaviour is to write to `stdout`.\n++\n+The described objects are not necessarily accessible\n+using standard git plumbing tools until a little while\n+after the next checkpoint.  To request access to the\n+blobs before then, use `cat-blob` lines in the command\n+stream.\n+\n --export-pack-edges=<file>::\n \tAfter creating a packfile, print a line of data to\n \t<file> listing the filename of the packfile and the last\n@@ -320,6 +331,11 @@ and control the current import process.  More detailed discussion\n \tstandard output.  This command is optional and is not needed\n \tto perform an import.\n \n+`cat-blob`::\n+\tCauses fast-import to print a blob in 'cat-file --batch'\n+\tformat to the file descriptor set with `--cat-blob-fd` or\n+\t`stdout` if unspecified.\n+\n `feature`::\n \tRequire that fast-import supports the specified feature, or\n \tabort if it does not.\n@@ -876,6 +892,23 @@ Placing a `progress` command immediately after a `checkpoint` will\n inform the reader when the `checkpoint` has been completed and it\n can safely access the refs that fast-import updated.\n \n+`cat-blob`\n+~~~~~\n+Causes fast-import to print a blob to a file descriptor previously\n+arranged with the `--cat-blob-fd` argument.  The command otherwise\n+has no impact on the current import; its main purpose is to\n+retrieve blobs that may be in fast-import's memory but not\n+accessible from the target repository a little quicker than by the\n+method suggested by the description of the `progress` option.\n+\n+....\n+\t'cat-blob' SP <dataref> LF\n+....\n+\n+The `<dataref>` can be either a mark reference (`:<idnum>`)\n+set previously, or a full 40-byte SHA-1 of any Git blob,\n+preexisting or ready to be written.\n+\n `feature`\n ~~~~~~~~~\n Require that fast-import supports the specified feature, or abort if\n@@ -896,6 +929,7 @@ The following features are currently supported:\n * date-format\n * import-marks\n * export-marks\n+* cat-blob\n * relative-marks\n * no-relative-marks\n * force\ndiff --git a/fast-import.c b/fast-import.c\nindex 2317b0f..ea3e529 100644\n--- a/fast-import.c\n+++ b/fast-import.c\n@@ -55,6 +55,8 @@ Format of STDIN stream:\n     ('from' sp committish lf)?\n     lf?;\n \n+  cat_blob ::= 'cat-blob' sp (hexsha1 | idnum) lf;\n+\n   checkpoint ::= 'checkpoint' lf\n     lf?;\n \n@@ -361,6 +363,9 @@ static uintmax_t next_mark;\n static struct strbuf new_data = STRBUF_INIT;\n static int seen_data_command;\n \n+/* Where to write output of cat-blob commands */\n+static int cat_blob_fd = 1;\n+\n static void parse_argv(void);\n \n static void write_branch_report(FILE *rpt, struct branch *b)\n@@ -2680,6 +2685,77 @@ static void parse_reset_branch(void)\n \t\tunread_command_buf = 1;\n }\n \n+static void cat_blob_write(const char *buf, unsigned long size)\n+{\n+\tif (write_in_full(cat_blob_fd, buf, size) != size)\n+\t\tdie_errno(\"Write to frontend failed\");\n+}\n+\n+static void cat_blob(struct object_entry *oe, unsigned char sha1[20])\n+{\n+\tstruct strbuf line = STRBUF_INIT;\n+\tunsigned long size;\n+\tenum object_type type = 0;\n+\tchar *buf;\n+\n+\tif (oe && oe->pack_id != MAX_PACK_ID) {\n+\t\ttype = oe->type;\n+\t\tbuf = gfi_unpack_entry(oe, &size);\n+\t} else {\n+\t\tbuf = read_sha1_file(sha1, &type, &size);\n+\t}\n+\tif (!buf)\n+\t\tdie(\"Can't read object %s\", sha1_to_hex(sha1));\n+\n+\t/*\n+\t * Output based on batch_one_object() from cat-file.c.\n+\t */\n+\tif (type <= 0) {\n+\t\tstrbuf_reset(&line);\n+\t\tstrbuf_addf(&line, \"%s missing\\n\", sha1_to_hex(sha1));\n+\t\tcat_blob_write(line.buf, line.len);\n+\t\treturn;\n+\t} else if (type != OBJ_BLOB) {\n+\t\tdie(\"Object %s is a %s but a blob was expected.\",\n+\t\t    sha1_to_hex(sha1), typename(type));\n+\t}\n+\tstrbuf_reset(&line);\n+\tstrbuf_addf(&line, \"%s %s %lu\\n\", sha1_to_hex(sha1),\n+\t\t\t\t\t\ttypename(type), size);\n+\tcat_blob_write(line.buf, line.len);\n+\tcat_blob_write(buf, size);\n+\tcat_blob_write(\"\\n\", 1);\n+\tfree(buf);\n+}\n+\n+\n+static void parse_cat_blob(void)\n+{\n+\tconst char *p;\n+\tstruct object_entry *oe = oe;\n+\tunsigned char sha1[20];\n+\n+\t/* cat SP <object> */\n+\tp = command_buf.buf + strlen(\"cat-blob \");\n+\tif (*p == ':') {\n+\t\tchar *x;\n+\t\toe = find_mark(strtoumax(p + 1, &x, 10));\n+\t\tif (x == p + 1)\n+\t\t\tdie(\"Invalid mark: %s\", command_buf.buf);\n+\t\tif (!oe)\n+\t\t\tdie(\"Unknown mark: %s\", command_buf.buf);\n+\t\tp = x;\n+\t\thashcpy(sha1, oe->idx.sha1);\n+\t} else {\n+\t\tif (get_sha1_hex(p, sha1))\n+\t\t\tdie(\"Invalid SHA1: %s\", command_buf.buf);\n+\t\tp += 40;\n+\t\toe = find_object(sha1);\n+\t}\n+\n+\tcat_blob(oe, sha1);\n+}\n+\n static void parse_checkpoint(void)\n {\n \tif (object_count) {\n@@ -2755,6 +2831,14 @@ static void option_export_marks(const char *marks)\n \tsafe_create_leading_directories_const(export_marks_file);\n }\n \n+static void option_cat_blob_fd(const char *fd)\n+{\n+\tunsigned long n = strtoul(fd, NULL, 0);\n+\tif (n > (unsigned long) INT_MAX)\n+\t\tdie(\"--cat-blob-fd cannot exceed %d\", INT_MAX);\n+\tcat_blob_fd = (int) n;\n+}\n+\n static void option_export_pack_edges(const char *edges)\n {\n \tif (pack_edges)\n@@ -2808,6 +2892,8 @@ static int parse_one_feature(const char *feature, int from_stream)\n \t\toption_import_marks(feature + 13, from_stream);\n \t} else if (!prefixcmp(feature, \"export-marks=\")) {\n \t\toption_export_marks(feature + 13);\n+\t} else if (!prefixcmp(feature, \"cat-blob\")) {\n+\t\t/* Don't die - this feature is supported */\n \t} else if (!prefixcmp(feature, \"relative-marks\")) {\n \t\trelative_marks_paths = 1;\n \t} else if (!prefixcmp(feature, \"no-relative-marks\")) {\n@@ -2896,6 +2982,10 @@ static void parse_argv(void)\n \t\tif (*a != '-' || !strcmp(a, \"--\"))\n \t\t\tbreak;\n \n+\t\tif (!prefixcmp(a + 2, \"cat-blob-fd=\")) {\n+\t\t\toption_cat_blob_fd(a + 2 + 12);\n+\t\t}\n+\n \t\tif (parse_one_option(a + 2))\n \t\t\tcontinue;\n \n@@ -2953,6 +3043,8 @@ int main(int argc, const char **argv)\n \t\t\tparse_new_tag();\n \t\telse if (!prefixcmp(command_buf.buf, \"reset \"))\n \t\t\tparse_reset_branch();\n+\t\telse if (!prefixcmp(command_buf.buf, \"cat-blob \"))\n+\t\t\tparse_cat_blob();\n \t\telse if (!strcmp(\"checkpoint\", command_buf.buf))\n \t\t\tparse_checkpoint();\n \t\telse if (!prefixcmp(command_buf.buf, \"progress \"))\n-- \n1.7.3.32.g634ef\n"},{"id":"153579","messageId":"1287147256-9457-3-git-send-email-david.barr@cordelta.com","threadId":"25454","inReplyTo":"1287147256-9457-1-git-send-email-david.barr@cordelta.com","subject":"[PATCH 2/5] vcs-svn: Extend svndump to parse version 3 format","fromName":"David Barr","fromEmail":"david.barr@cordelta.com","sentAt":"2010-10-15T12:54:13Z","receivedAt":"2010-10-15T12:54:13Z","isPatch":true,"sender":{"key":"david.barr@cordelta.com","avatar":"https://avatars.githubusercontent.com/u/220594?v=4"},"body":"Add 1 new dump header, SVN-fs-dump-format-version.\nAdd 6 new node headers:\n* Text-delta: true|false\n* Prop-delta: true|false\n* Text-delta-base-md5: <32 hex digits>\n* Text-delta-base-sha1: <40 hex digits>\n* Text-copy-source-sha1: <40 hex digits>\n* Text-content-sha1: <40 hex digits>\n\nThis change simply populates the context.\nFurther changes will be needed to handle text and prop deltas.\n\nSigned-off-by: David Barr <david.barr@cordelta.com>\n---\n vcs-svn/svndump.c |   58 ++++++++++++++++++++++++++++++++++++++++++++++++----\n 1 files changed, 53 insertions(+), 5 deletions(-)\n\ndiff --git a/vcs-svn/svndump.c b/vcs-svn/svndump.c\nindex 3bba0fe..458053e 100644\n--- a/vcs-svn/svndump.c\n+++ b/vcs-svn/svndump.c\n@@ -27,6 +27,9 @@\n #define LENGTH_UNKNOWN (~0)\n #define DATE_RFC2822_LEN 31\n \n+#define MD5_HEX_LENGTH 32\n+#define SHA1_HEX_LENGTH 40\n+\n /* Create memory pool for log messages */\n obj_pool_gen(log, char, 4096)\n \n@@ -44,6 +47,11 @@ static char* log_copy(uint32_t length, char *log)\n static struct {\n \tuint32_t action, propLength, textLength, srcRev, srcMode, mark, type;\n \tuint32_t src[REPO_MAX_PATH_DEPTH], dst[REPO_MAX_PATH_DEPTH];\n+\tuint32_t text_delta, prop_delta;\n+\tchar text_delta_base_md5[MD5_HEX_LENGTH + 1];\n+\tchar text_content_sha1[SHA1_HEX_LENGTH + 1];\n+\tchar text_delta_base_sha1[SHA1_HEX_LENGTH + 1];\n+\tchar text_copy_source_sha1[SHA1_HEX_LENGTH + 1];\n } node_ctx;\n \n static struct {\n@@ -53,14 +61,20 @@ static struct {\n } rev_ctx;\n \n static struct {\n-\tuint32_t uuid, url;\n+\tuint32_t version, uuid, url;\n } dump_ctx;\n \n static struct {\n-\tuint32_t svn_log, svn_author, svn_date, svn_executable, svn_special, uuid,\n+\tuint32_t svn_log, svn_author, svn_date, svn_executable, svn_special,\n \t\trevision_number, node_path, node_kind, node_action,\n \t\tnode_copyfrom_path, node_copyfrom_rev, text_content_length,\n-\t\tprop_content_length, content_length;\n+\t\tprop_content_length, content_length,\n+\t\t/* SVN dump version 2 */\n+\t\tuuid, svn_fs_dump_format_version,\n+\t\t/* SVN dump version 3 */\n+\t\ttext_delta, prop_delta, text_content_sha1,\n+\t\ttext_delta_base_md5, text_delta_base_sha1,\n+\t\ttext_copy_source_sha1;\n } keys;\n \n static void reset_node_ctx(char *fname)\n@@ -74,6 +88,12 @@ static void reset_node_ctx(char *fname)\n \tnode_ctx.srcMode = 0;\n \tpool_tok_seq(REPO_MAX_PATH_DEPTH, node_ctx.dst, \"/\", fname);\n \tnode_ctx.mark = 0;\n+\tnode_ctx.text_delta = 0;\n+\tnode_ctx.prop_delta = 0;\n+\t*node_ctx.text_delta_base_md5 = '\\0';\n+\t*node_ctx.text_content_sha1 = '\\0';\n+\t*node_ctx.text_delta_base_sha1 = '\\0';\n+\t*node_ctx.text_copy_source_sha1 = '\\0';\n }\n \n static void reset_rev_ctx(uint32_t revision)\n@@ -87,6 +107,7 @@ static void reset_rev_ctx(uint32_t revision)\n static void reset_dump_ctx(uint32_t url)\n {\n \tdump_ctx.url = url;\n+\tdump_ctx.version = 1;\n \tdump_ctx.uuid = ~0;\n }\n \n@@ -97,7 +118,6 @@ static void init_keys(void)\n \tkeys.svn_date = pool_intern(\"svn:date\");\n \tkeys.svn_executable = pool_intern(\"svn:executable\");\n \tkeys.svn_special = pool_intern(\"svn:special\");\n-\tkeys.uuid = pool_intern(\"UUID\");\n \tkeys.revision_number = pool_intern(\"Revision-number\");\n \tkeys.node_path = pool_intern(\"Node-path\");\n \tkeys.node_kind = pool_intern(\"Node-kind\");\n@@ -107,6 +127,16 @@ static void init_keys(void)\n \tkeys.text_content_length = pool_intern(\"Text-content-length\");\n \tkeys.prop_content_length = pool_intern(\"Prop-content-length\");\n \tkeys.content_length = pool_intern(\"Content-length\");\n+\t/* SVN dump version 2 */\n+\tkeys.svn_fs_dump_format_version = pool_intern(\"SVN-fs-dump-format-version\");\n+\tkeys.uuid = pool_intern(\"UUID\");\n+\t/* SVN dump version 3 */\n+\tkeys.text_delta = pool_intern(\"Text-delta\");\n+\tkeys.prop_delta = pool_intern(\"Prop-delta\");\n+\tkeys.text_delta_base_md5 = pool_intern(\"Text-delta-base-md5\");\n+\tkeys.text_delta_base_sha1 = pool_intern(\"Text-delta-base-sha1\");\n+\tkeys.text_copy_source_sha1 = pool_intern(\"Text-copy-source-sha1\");\n+\tkeys.text_content_sha1 = pool_intern(\"Text-content-sha1\");\n }\n \n static void read_props(void)\n@@ -209,7 +239,9 @@ void svndump_read(const char *url)\n \t\t*val++ = '\\0';\n \t\tkey = pool_intern(t);\n \n-\t\tif (key == keys.uuid) {\n+\t\tif (key == keys.svn_fs_dump_format_version) {\n+\t\t\tdump_ctx.version = atoi(val);\n+\t\t} else if (key == keys.uuid) {\n \t\t\tdump_ctx.uuid = pool_intern(val);\n \t\t} else if (key == keys.revision_number) {\n \t\t\tif (active_ctx == NODE_CTX)\n@@ -251,6 +283,22 @@ void svndump_read(const char *url)\n \t\t\tnode_ctx.textLength = atoi(val);\n \t\t} else if (key == keys.prop_content_length) {\n \t\t\tnode_ctx.propLength = atoi(val);\n+\t\t} else if (key == keys.text_delta) {\n+\t\t\tnode_ctx.text_delta = !strcmp(val, \"true\");\n+\t\t} else if (key == keys.prop_delta) {\n+\t\t\tnode_ctx.prop_delta = !strcmp(val, \"true\");\n+\t\t} else if (key == keys.text_delta_base_md5) {\n+\t\t\tstrncpy(node_ctx.text_delta_base_md5, val,\n+\t\t\t\tMD5_HEX_LENGTH + 1);\n+\t\t} else if (key == keys.text_delta_base_sha1) {\n+\t\t\tstrncpy(node_ctx.text_delta_base_sha1, val,\n+\t\t\t\tSHA1_HEX_LENGTH + 1);\n+\t\t} else if (key == keys.text_copy_source_sha1) {\n+\t\t\tstrncpy(node_ctx.text_copy_source_sha1, val,\n+\t\t\t\tSHA1_HEX_LENGTH + 1);\n+\t\t} else if (key == keys.text_content_sha1) {\n+\t\t\tstrncpy(node_ctx.text_content_sha1, val,\n+\t\t\t\tSHA1_HEX_LENGTH + 1);\n \t\t} else if (key == keys.content_length) {\n \t\t\tlen = atoi(val);\n \t\t\tbuffer_read_line(&input);\n-- \n1.7.3.32.g634ef\n"},{"id":"153580","messageId":"1287147256-9457-4-git-send-email-david.barr@cordelta.com","threadId":"25454","inReplyTo":"1287147256-9457-1-git-send-email-david.barr@cordelta.com","subject":"[PATCH 3/5] vcs-svn: Implement prop-delta handling.","fromName":"David Barr","fromEmail":"david.barr@cordelta.com","sentAt":"2010-10-15T12:54:14Z","receivedAt":"2010-10-15T12:54:14Z","isPatch":true,"sender":{"key":"david.barr@cordelta.com","avatar":"https://avatars.githubusercontent.com/u/220594?v=4"},"body":"By testing against the Apache Software Foundation\nrepository, some simple rules for decoding prop\ndeltas were derived.\n\n'Node-action: replace' implies the empty prop set\nas the base for the delta.\nOtherwise, if a copyfrom source is given that node\nforms the basis for the delta.\nLastly, if the destination path exists in the active\nrevision it forms the basis.\n\nThe same rules ought to apply to text deltas as well.\n\nApply these rules to prop handling.\nAdd a placeholder srcMark parameter to fast_export_blob().\n\nSigned-off-by: David Barr <david.barr@cordelta.com>\n---\n vcs-svn/fast_export.c |    4 +++-\n vcs-svn/fast_export.h |    3 ++-\n vcs-svn/repo_tree.c   |   23 +++++++++++++++++++++++\n vcs-svn/repo_tree.h   |    2 ++\n vcs-svn/svndump.c     |   35 +++++++++++++++++++++++++++++++----\n 5 files changed, 61 insertions(+), 6 deletions(-)\n\ndiff --git a/vcs-svn/fast_export.c b/vcs-svn/fast_export.c\nindex 260cf50..d984aaa 100644\n--- a/vcs-svn/fast_export.c\n+++ b/vcs-svn/fast_export.c\n@@ -63,7 +63,9 @@ void fast_export_commit(uint32_t revision, uint32_t author, char *log,\n \tprintf(\"progress Imported commit %\"PRIu32\".\\n\\n\", revision);\n }\n \n-void fast_export_blob(uint32_t mode, uint32_t mark, uint32_t len, struct line_buffer *input)\n+void fast_export_blob(uint32_t mode, uint32_t mark, uint32_t len,\n+\t\t\tuint32_t delta, uint32_t srcMark, uint32_t srcMode,\n+\t\t\tstruct line_buffer *input)\n {\n \tif (mode == REPO_MODE_LNK) {\n \t\t/* svn symlink blobs start with \"link \" */\ndiff --git a/vcs-svn/fast_export.h b/vcs-svn/fast_export.h\nindex 054e7d5..634d9c6 100644\n--- a/vcs-svn/fast_export.h\n+++ b/vcs-svn/fast_export.h\n@@ -9,6 +9,7 @@ void fast_export_modify(uint32_t depth, uint32_t *path, uint32_t mode,\n void fast_export_commit(uint32_t revision, uint32_t author, char *log,\n \t\t\tuint32_t uuid, uint32_t url, unsigned long timestamp);\n void fast_export_blob(uint32_t mode, uint32_t mark, uint32_t len,\n-\t\t      struct line_buffer *input);\n+\t\t\tuint32_t delta, uint32_t srcMark, uint32_t srcMode,\n+\t\t\tstruct line_buffer *input);\n \n #endif\ndiff --git a/vcs-svn/repo_tree.c b/vcs-svn/repo_tree.c\nindex e94d91d..b616bda 100644\n--- a/vcs-svn/repo_tree.c\n+++ b/vcs-svn/repo_tree.c\n@@ -157,6 +157,29 @@ static void repo_write_dirent(uint32_t *path, uint32_t mode,\n \t\tdent_remove(&dir_pointer(parent_dir_o)->entries, dent);\n }\n \n+uint32_t repo_read_mark(uint32_t revision, uint32_t *path)\n+{\n+\tuint32_t mode = 0, content_offset = 0;\n+\tstruct repo_dirent *src_dent;\n+\tsrc_dent = repo_read_dirent(revision, path);\n+\tif (src_dent != NULL) {\n+\t\tmode = src_dent->mode;\n+\t\tcontent_offset = src_dent->content_offset;\n+\t}\n+\treturn mode && mode != REPO_MODE_DIR ? content_offset : 0;\n+}\n+\n+uint32_t repo_read_mode(uint32_t revision, uint32_t *path)\n+{\n+\tuint32_t mode = 0;\n+\tstruct repo_dirent *src_dent;\n+\tsrc_dent = repo_read_dirent(revision, path);\n+\tif (src_dent != NULL) {\n+\t\tmode = src_dent->mode;\n+\t}\n+\treturn mode;\n+}\n+\n uint32_t repo_copy(uint32_t revision, uint32_t *src, uint32_t *dst)\n {\n \tuint32_t mode = 0, content_offset = 0;\ndiff --git a/vcs-svn/repo_tree.h b/vcs-svn/repo_tree.h\nindex 5476175..bd6a3f7 100644\n--- a/vcs-svn/repo_tree.h\n+++ b/vcs-svn/repo_tree.h\n@@ -12,6 +12,8 @@\n #define REPO_MAX_PATH_DEPTH 1000\n \n uint32_t next_blob_mark(void);\n+uint32_t repo_read_mark(uint32_t revision, uint32_t *path);\n+uint32_t repo_read_mode(uint32_t revision, uint32_t *path);\n uint32_t repo_copy(uint32_t revision, uint32_t *src, uint32_t *dst);\n void repo_add(uint32_t *path, uint32_t mode, uint32_t blob_mark);\n uint32_t repo_replace(uint32_t *path, uint32_t blob_mark);\ndiff --git a/vcs-svn/svndump.c b/vcs-svn/svndump.c\nindex 458053e..3431c22 100644\n--- a/vcs-svn/svndump.c\n+++ b/vcs-svn/svndump.c\n@@ -45,7 +45,7 @@ static char* log_copy(uint32_t length, char *log)\n }\n \n static struct {\n-\tuint32_t action, propLength, textLength, srcRev, srcMode, mark, type;\n+\tuint32_t action, propLength, textLength, srcRev, srcMode, srcMark, mark, type;\n \tuint32_t src[REPO_MAX_PATH_DEPTH], dst[REPO_MAX_PATH_DEPTH];\n \tuint32_t text_delta, prop_delta;\n \tchar text_delta_base_md5[MD5_HEX_LENGTH + 1];\n@@ -86,6 +86,7 @@ static void reset_node_ctx(char *fname)\n \tnode_ctx.src[0] = ~0;\n \tnode_ctx.srcRev = 0;\n \tnode_ctx.srcMode = 0;\n+\tnode_ctx.srcMark = 0;\n \tpool_tok_seq(REPO_MAX_PATH_DEPTH, node_ctx.dst, \"/\", fname);\n \tnode_ctx.mark = 0;\n \tnode_ctx.text_delta = 0;\n@@ -168,17 +169,42 @@ static void read_props(void)\n \t\t\t}\n \t\t\tkey = ~0;\n \t\t\tbuffer_read_line(&input);\n+\t\t} else if (!strncmp(t, \"D \", 2)) {\n+\t\t\tlen = atoi(&t[2]);\n+\t\t\tkey = pool_intern(buffer_read_string(&input, len));\n+\t\t\tbuffer_read_line(&input);\n+\t\t\tif (key == keys.svn_executable) {\n+\t\t\t\tif (node_ctx.type == REPO_MODE_EXE)\n+\t\t\t\t\tnode_ctx.type = REPO_MODE_BLB;\n+\t\t\t} else if (key == keys.svn_special) {\n+\t\t\t\tif (node_ctx.type == REPO_MODE_LNK)\n+\t\t\t\t\tnode_ctx.type = REPO_MODE_BLB;\n+\t\t\t}\n+\t\t\tkey = ~0;\n \t\t}\n \t}\n }\n \n static void handle_node(void)\n {\n+\tif (node_ctx.prop_delta) {\n+\t\tif (node_ctx.srcRev)\n+\t\t\tnode_ctx.srcMode = repo_read_mode(node_ctx.srcRev, node_ctx.src);\n+\t\telse\n+\t\t\tnode_ctx.srcMode = repo_read_mode(rev_ctx.revision, node_ctx.dst);\n+\t\tif (node_ctx.srcMode && node_ctx.action != NODEACT_REPLACE)\n+\t\t\tnode_ctx.type = node_ctx.srcMode;\n+\t}\n+\n \tif (node_ctx.propLength != LENGTH_UNKNOWN && node_ctx.propLength)\n \t\tread_props();\n \n-\tif (node_ctx.srcRev)\n+\tif (node_ctx.srcRev) {\n+\t\tnode_ctx.srcMark = repo_read_mark(node_ctx.srcRev, node_ctx.src);\n \t\tnode_ctx.srcMode = repo_copy(node_ctx.srcRev, node_ctx.src, node_ctx.dst);\n+\t} else {\n+\t\tnode_ctx.srcMark = repo_read_mark(rev_ctx.revision, node_ctx.dst);\n+\t}\n \n \tif (node_ctx.textLength != LENGTH_UNKNOWN &&\n \t    node_ctx.type != REPO_MODE_DIR)\n@@ -209,8 +235,9 @@ static void handle_node(void)\n \t\tnode_ctx.type = node_ctx.srcMode;\n \n \tif (node_ctx.mark)\n-\t\tfast_export_blob(node_ctx.type,\n-\t\t\t\t node_ctx.mark, node_ctx.textLength, &input);\n+\t\tfast_export_blob(node_ctx.type, node_ctx.mark, node_ctx.textLength,\n+\t\t\t\tnode_ctx.text_delta, node_ctx.srcMark, node_ctx.srcMode,\n+\t\t\t\t&input);\n \telse if (node_ctx.textLength != LENGTH_UNKNOWN)\n \t\tbuffer_skip_bytes(&input, node_ctx.textLength);\n }\n-- \n1.7.3.32.g634ef\n"},{"id":"153583","messageId":"1287147256-9457-5-git-send-email-david.barr@cordelta.com","threadId":"25454","inReplyTo":"1287147256-9457-1-git-send-email-david.barr@cordelta.com","subject":"[PATCH 4/5] vcs-svn: Add outfile option to buffer_copy_bytes()","fromName":"David Barr","fromEmail":"david.barr@cordelta.com","sentAt":"2010-10-15T12:54:15Z","receivedAt":"2010-10-15T12:54:15Z","isPatch":true,"sender":{"key":"david.barr@cordelta.com","avatar":"https://avatars.githubusercontent.com/u/220594?v=4"},"body":"Explicitly declare that output is to stdout for existing use.\nAllow users of buffer_copy_bytes() to specify the output file.\n\nSigned-off-by: David Barr <david.barr@cordelta.com>\n---\n test-line-buffer.c    |    2 +-\n vcs-svn/fast_export.c |    2 +-\n vcs-svn/line_buffer.c |    6 +++---\n vcs-svn/line_buffer.h |    2 +-\n 4 files changed, 6 insertions(+), 6 deletions(-)\n\ndiff --git a/test-line-buffer.c b/test-line-buffer.c\nindex f9af892..adc23e8 100644\n--- a/test-line-buffer.c\n+++ b/test-line-buffer.c\n@@ -36,7 +36,7 @@ int main(int argc, char *argv[])\n \t\tbuffer_skip_bytes(&buf, 1);\n \t\tif (!(s = buffer_read_line(&buf)))\n \t\t\tbreak;\n-\t\tbuffer_copy_bytes(&buf, strtouint32(s) + 1);\n+\t\tbuffer_copy_bytes(&buf, stdout, strtouint32(s) + 1);\n \t}\n \tif (buffer_deinit(&buf))\n \t\tdie(\"input error\");\ndiff --git a/vcs-svn/fast_export.c b/vcs-svn/fast_export.c\nindex d984aaa..b017dfb 100644\n--- a/vcs-svn/fast_export.c\n+++ b/vcs-svn/fast_export.c\n@@ -73,6 +73,6 @@ void fast_export_blob(uint32_t mode, uint32_t mark, uint32_t len,\n \t\tlen -= 5;\n \t}\n \tprintf(\"blob\\nmark :%\"PRIu32\"\\ndata %\"PRIu32\"\\n\", mark, len);\n-\tbuffer_copy_bytes(input, len);\n+\tbuffer_copy_bytes(input, stdout, len);\n \tfputc('\\n', stdout);\n }\ndiff --git a/vcs-svn/line_buffer.c b/vcs-svn/line_buffer.c\nindex c54031b..676cb62 100644\n--- a/vcs-svn/line_buffer.c\n+++ b/vcs-svn/line_buffer.c\n@@ -82,7 +82,7 @@ void buffer_read_binary(struct strbuf *sb, uint32_t size,\n \tstrbuf_fread(sb, size, buf->infile);\n }\n \n-void buffer_copy_bytes(struct line_buffer *buf, off_t len)\n+void buffer_copy_bytes(struct line_buffer *buf, FILE *outfile, off_t len)\n {\n \tchar byte_buffer[COPY_BUFFER_LEN];\n \tuint32_t in;\n@@ -90,8 +90,8 @@ void buffer_copy_bytes(struct line_buffer *buf, off_t len)\n \t\tin = len < COPY_BUFFER_LEN ? len : COPY_BUFFER_LEN;\n \t\tin = fread(byte_buffer, 1, in, buf->infile);\n \t\tlen -= in;\n-\t\tfwrite(byte_buffer, 1, in, stdout);\n-\t\tif (ferror(stdout)) {\n+\t\tfwrite(byte_buffer, 1, in, outfile);\n+\t\tif (ferror(outfile)) {\n \t\t\tbuffer_skip_bytes(buf, len);\n \t\t\treturn;\n \t\t}\ndiff --git a/vcs-svn/line_buffer.h b/vcs-svn/line_buffer.h\nindex 2375ee1..23f4931 100644\n--- a/vcs-svn/line_buffer.h\n+++ b/vcs-svn/line_buffer.h\n@@ -20,7 +20,7 @@ char *buffer_read_line(struct line_buffer *buf);\n char *buffer_read_string(struct line_buffer *buf, uint32_t len);\n int buffer_read_char(struct line_buffer *buf);\n void buffer_read_binary(struct strbuf *sb, uint32_t len, struct line_buffer *f);\n-void buffer_copy_bytes(struct line_buffer *buf, off_t len);\n+void buffer_copy_bytes(struct line_buffer *buf, FILE *outfile, off_t len);\n off_t buffer_skip_bytes(struct line_buffer *buf, off_t len);\n void buffer_reset(struct line_buffer *buf);\n \n-- \n1.7.3.32.g634ef\n"},{"id":"153578","messageId":"1287147256-9457-6-git-send-email-david.barr@cordelta.com","threadId":"25454","inReplyTo":"1287147256-9457-1-git-send-email-david.barr@cordelta.com","subject":"[PATCH 5/5] svn-fe: Use the cat-blob command to apply deltas","fromName":"David Barr","fromEmail":"david.barr@cordelta.com","sentAt":"2010-10-15T12:54:16Z","receivedAt":"2010-10-15T12:54:16Z","isPatch":true,"sender":{"key":"david.barr@cordelta.com","avatar":"https://avatars.githubusercontent.com/u/220594?v=4"},"body":"Use the new cat-blob command for fast-import to extract\nblobs so that text-deltas may be applied.\n\nThe backchannel should only need to be configured when\nparsing v3 svn dump streams.\n\nBased-on-patch-by: Ramkumar Ramachandra <artagnon@gmail.com>\nBased-on-patch-by: Jonathan Nieder <jrnieder@gmail.com>\nTested-by: David Barr <david.barr@cordelta.com>\nSigned-off-by: David Barr <david.barr@cordelta.com>\n---\n contrib/svn-fe/svn-fe.txt |    6 +++-\n t/t9010-svn-fe.sh         |    6 ++--\n vcs-svn/fast_export.c     |   86 +++++++++++++++++++++++++++++++++++++++++++-\n 3 files changed, 92 insertions(+), 6 deletions(-)\n\ndiff --git a/contrib/svn-fe/svn-fe.txt b/contrib/svn-fe/svn-fe.txt\nindex 35f84bd..39ffa07 100644\n--- a/contrib/svn-fe/svn-fe.txt\n+++ b/contrib/svn-fe/svn-fe.txt\n@@ -7,7 +7,11 @@ svn-fe - convert an SVN \"dumpfile\" to a fast-import stream\n \n SYNOPSIS\n --------\n-svnadmin dump --incremental REPO | svn-fe [url] | git fast-import\n+[verse]\n+mkfifo backchannel &&\n+svnadmin dump --incremental REPO |\n+\tsvn-fe [url] 3<backchannel |\n+\tgit fast-import --cat-blob-fd=3 3>backchannel\n \n DESCRIPTION\n -----------\ndiff --git a/t/t9010-svn-fe.sh b/t/t9010-svn-fe.sh\nindex de976ed..d750c7a 100755\n--- a/t/t9010-svn-fe.sh\n+++ b/t/t9010-svn-fe.sh\n@@ -34,10 +34,10 @@ test_dump () {\n \t\tsvnadmin load \"$label-svn\" < \"$TEST_DIRECTORY/$dump\" &&\n \t\tsvn_cmd export \"file://$PWD/$label-svn\" \"$label-svnco\" &&\n \t\tgit init \"$label-git\" &&\n-\t\ttest-svn-fe \"$TEST_DIRECTORY/$dump\" >\"$label.fe\" &&\n \t\t(\n-\t\t\tcd \"$label-git\" &&\n-\t\t\tgit fast-import < ../\"$label.fe\"\n+\t\t\tcd \"$label-git\" && mkfifo backchannel && \\\n+\t\t\ttest-svn-fe \"$TEST_DIRECTORY/$dump\" 3< backchannel | \\\n+\t\t\tgit fast-import --cat-blob-fd=3 3> backchannel\n \t\t) &&\n \t\t(\n \t\t\tcd \"$label-svnco\" &&\ndiff --git a/vcs-svn/fast_export.c b/vcs-svn/fast_export.c\nindex b017dfb..812563d 100644\n--- a/vcs-svn/fast_export.c\n+++ b/vcs-svn/fast_export.c\n@@ -8,10 +8,17 @@\n #include \"line_buffer.h\"\n #include \"repo_tree.h\"\n #include \"string_pool.h\"\n+#include \"svndiff.h\"\n \n #define MAX_GITSVN_LINE_LEN 4096\n+#define REPORT_FILENO 3\n+\n+#define SHA1_HEX_LENGTH 40\n \n static uint32_t first_commit_done;\n+static struct line_buffer preimage = LINE_BUFFER_INIT;\n+static struct line_buffer postimage = LINE_BUFFER_INIT;\n+static struct line_buffer backchannel = LINE_BUFFER_INIT;\n \n void fast_export_delete(uint32_t depth, uint32_t *path)\n {\n@@ -63,16 +70,91 @@ void fast_export_commit(uint32_t revision, uint32_t author, char *log,\n \tprintf(\"progress Imported commit %\"PRIu32\".\\n\\n\", revision);\n }\n \n+static int fast_export_save_blob(FILE *out)\n+{\n+\tsize_t len;\n+\tchar *header;\n+\tchar *end;\n+\tchar *tail;\n+\n+\tif (!backchannel.infile)\n+\t\tbackchannel.infile = fdopen(REPORT_FILENO, \"r\");\n+\tif (!backchannel.infile)\n+\t\treturn error(\"Could not open backchannel fd: %d\", REPORT_FILENO);\n+\theader = buffer_read_line(&backchannel);\n+\tif (header == NULL)\n+\t\treturn 1;\n+\tend = strchr(header, '\\0');\n+\tif (end - header > 7 && !strcmp(end - 7, \"missing\"))\n+\t\treturn error(\"cat-blob reports missing blob: %s\", header);\n+\tif (end - header < SHA1_HEX_LENGTH)\n+\t\treturn error(\"cat-blob header too short for SHA1: %s\", header);\n+\tif (strncmp(header + SHA1_HEX_LENGTH, \" blob \", 6))\n+\t\treturn error(\"cat-blob header has wrong object type: %s\", header);\n+\tlen = strtoumax(header + SHA1_HEX_LENGTH + 6, &end, 10);\n+\tif (end == header + SHA1_HEX_LENGTH + 6)\n+\t\treturn error(\"cat-blob header did not contain length: %s\", header);\n+\tif (*end)\n+\t\treturn error(\"cat-blob header contained garbage after length: %s\", header);\n+\tbuffer_copy_bytes(&backchannel, out, len);\n+\ttail = buffer_read_line(&backchannel);\n+\tif (!tail)\n+\t\treturn 1;\n+\tif (*tail)\n+\t\treturn error(\"cat-blob trailing line contained garbage: %s\", tail);\n+\treturn 0;\n+}\n+\n void fast_export_blob(uint32_t mode, uint32_t mark, uint32_t len,\n \t\t\tuint32_t delta, uint32_t srcMark, uint32_t srcMode,\n \t\t\tstruct line_buffer *input)\n {\n+\tlong preimage_len = 0;\n+\n+\tif (delta) {\n+\t\tif (!preimage.infile)\n+\t\t\tpreimage.infile = tmpfile();\n+\t\tif (!preimage.infile)\n+\t\t\tdie(\"Unable to open temp file for blob retrieval\");\n+\t\tif (srcMark) {\n+\t\t\tprintf(\"cat-blob :%\"PRIu32\"\\n\", srcMark);\n+\t\t\tfflush(stdout);\n+\t\t\tif (srcMode == REPO_MODE_LNK)\n+\t\t\t\tfwrite(\"link \", 1, 5, preimage.infile);\n+\t\t\tif (fast_export_save_blob(preimage.infile))\n+\t\t\t\tdie(\"Failed to retrieve blob for delta application\");\n+\t\t}\n+\t\tpreimage_len = ftell(preimage.infile);\n+\t\tfseek(preimage.infile, 0, SEEK_SET);\n+\t\tif (!postimage.infile)\n+\t\t\tpostimage.infile = tmpfile();\n+\t\tif (!postimage.infile)\n+\t\t\tdie(\"Unable to open temp file for blob application\");\n+\t\tsvndiff0_apply(input, len, &preimage, postimage.infile);\n+\t\tlen = ftell(postimage.infile);\n+\t\tfseek(postimage.infile, 0, SEEK_SET);\n+\t}\n+\n \tif (mode == REPO_MODE_LNK) {\n \t\t/* svn symlink blobs start with \"link \" */\n-\t\tbuffer_skip_bytes(input, 5);\n+\t\tif (delta)\n+\t\t\tbuffer_skip_bytes(&postimage, 5);\n+\t\telse\n+\t\t\tbuffer_skip_bytes(input, 5);\n \t\tlen -= 5;\n \t}\n \tprintf(\"blob\\nmark :%\"PRIu32\"\\ndata %\"PRIu32\"\\n\", mark, len);\n-\tbuffer_copy_bytes(input, stdout, len);\n+\tif (!delta)\n+\t\tbuffer_copy_bytes(input, stdout, len);\n+\telse\n+\t\tbuffer_copy_bytes(&postimage, stdout, len);\n \tfputc('\\n', stdout);\n+\n+\tif (preimage.infile) {\n+\t\tfseek(preimage.infile, 0, SEEK_SET);\n+\t}\n+\n+\tif (postimage.infile) {\n+\t\tfseek(postimage.infile, 0, SEEK_SET);\n+\t}\n }\n-- \n1.7.3.32.g634ef\n"},{"id":"153695","messageId":"20101018065657.GE22376@kytes","threadId":"25454","inReplyTo":"1287147256-9457-6-git-send-email-david.barr@cordelta.com","subject":"Re: [PATCH 5/5] svn-fe: Use the cat-blob command to apply deltas","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2010-10-18T06:57:01Z","receivedAt":"2010-10-18T06:57:01Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Hi David,\n\nDavid Barr writes:\n> Use the new cat-blob command for fast-import to extract\n> blobs so that text-deltas may be applied.\n\nI like this straightforward approach, and I like the name 'cat-blob'.\n\n> The backchannel should only need to be configured when\n> parsing v3 svn dump streams.\n\nMaybe get the synopsis to say this as well?\n\n> Based-on-patch-by: Ramkumar Ramachandra <artagnon@gmail.com>\n> Based-on-patch-by: Jonathan Nieder <jrnieder@gmail.com>\n> Tested-by: David Barr <david.barr@cordelta.com>\n> Signed-off-by: David Barr <david.barr@cordelta.com>\n> ---\n>  contrib/svn-fe/svn-fe.txt |    6 +++-\n>  t/t9010-svn-fe.sh         |    6 ++--\n>  vcs-svn/fast_export.c     |   86 +++++++++++++++++++++++++++++++++++++++++++-\n>  3 files changed, 92 insertions(+), 6 deletions(-)\n> \n> diff --git a/contrib/svn-fe/svn-fe.txt b/contrib/svn-fe/svn-fe.txt\n> index 35f84bd..39ffa07 100644\n> --- a/contrib/svn-fe/svn-fe.txt\n> +++ b/contrib/svn-fe/svn-fe.txt\n> @@ -7,7 +7,11 @@ svn-fe - convert an SVN \"dumpfile\" to a fast-import stream\n>  \n>  SYNOPSIS\n>  --------\n> -svnadmin dump --incremental REPO | svn-fe [url] | git fast-import\n> +[verse]\n> +mkfifo backchannel &&\n> +svnadmin dump --incremental REPO |\n> +\tsvn-fe [url] 3<backchannel |\n> +\tgit fast-import --cat-blob-fd=3 3>backchannel\n\nSee above.\n\n>  DESCRIPTION\n>  -----------\n> diff --git a/t/t9010-svn-fe.sh b/t/t9010-svn-fe.sh\n> index de976ed..d750c7a 100755\n> --- a/t/t9010-svn-fe.sh\n> +++ b/t/t9010-svn-fe.sh\n> @@ -34,10 +34,10 @@ test_dump () {\n>  \t\tsvnadmin load \"$label-svn\" < \"$TEST_DIRECTORY/$dump\" &&\n>  \t\tsvn_cmd export \"file://$PWD/$label-svn\" \"$label-svnco\" &&\n>  \t\tgit init \"$label-git\" &&\n> -\t\ttest-svn-fe \"$TEST_DIRECTORY/$dump\" >\"$label.fe\" &&\n>  \t\t(\n> -\t\t\tcd \"$label-git\" &&\n> -\t\t\tgit fast-import < ../\"$label.fe\"\n> +\t\t\tcd \"$label-git\" && mkfifo backchannel && \\\n> +\t\t\ttest-svn-fe \"$TEST_DIRECTORY/$dump\" 3< backchannel | \\\n> +\t\t\tgit fast-import --cat-blob-fd=3 3> backchannel\n>  \t\t) &&\n>  \t\t(\n>  \t\t\tcd \"$label-svnco\" &&\n\nOk.\n\n> diff --git a/vcs-svn/fast_export.c b/vcs-svn/fast_export.c\n> index b017dfb..812563d 100644\n> --- a/vcs-svn/fast_export.c\n> +++ b/vcs-svn/fast_export.c\n> @@ -8,10 +8,17 @@\n>  #include \"line_buffer.h\"\n>  #include \"repo_tree.h\"\n>  #include \"string_pool.h\"\n> +#include \"svndiff.h\"\n>  \n>  #define MAX_GITSVN_LINE_LEN 4096\n> +#define REPORT_FILENO 3\n> +\n> +#define SHA1_HEX_LENGTH 40\n>  \n>  static uint32_t first_commit_done;\n> +static struct line_buffer preimage = LINE_BUFFER_INIT;\n> +static struct line_buffer postimage = LINE_BUFFER_INIT;\n> +static struct line_buffer backchannel = LINE_BUFFER_INIT;\n\nElegant :)\n\n>  void fast_export_delete(uint32_t depth, uint32_t *path)\n>  {\n> @@ -63,16 +70,91 @@ void fast_export_commit(uint32_t revision, uint32_t author, char *log,\n>  \tprintf(\"progress Imported commit %\"PRIu32\".\\n\\n\", revision);\n>  }\n>  \n> +static int fast_export_save_blob(FILE *out)\n> +{\n> +\tsize_t len;\n> +\tchar *header;\n> +\tchar *end;\n> +\tchar *tail;\n> +\n> +\tif (!backchannel.infile)\n> +\t\tbackchannel.infile = fdopen(REPORT_FILENO, \"r\");\n> +\tif (!backchannel.infile)\n> +\t\treturn error(\"Could not open backchannel fd: %d\", REPORT_FILENO);\n\nREPORT_FILENO = 3 is hard-coded. Is this intended? Maybe a\ncommand-line option to specify the fd?\n\n> +\theader = buffer_read_line(&backchannel);\n> +\tif (header == NULL)\n> +\t\treturn 1;\n\nNote to self: This prints the error \"Failed to retrieve blob for delta\napplication\" in the caller.\n\n> +\tend = strchr(header, '\\0');\n> +\tif (end - header > 7 && !strcmp(end - 7, \"missing\"))\n> +\t\treturn error(\"cat-blob reports missing blob: %s\", header);\n> +\tif (end - header < SHA1_HEX_LENGTH)\n> +\t\treturn error(\"cat-blob header too short for SHA1: %s\", header);\n> +\tif (strncmp(header + SHA1_HEX_LENGTH, \" blob \", 6))\n> +\t\treturn error(\"cat-blob header has wrong object type: %s\", header);\n> +\tlen = strtoumax(header + SHA1_HEX_LENGTH + 6, &end, 10);\n> +\tif (end == header + SHA1_HEX_LENGTH + 6)\n> +\t\treturn error(\"cat-blob header did not contain length: %s\", header);\n> +\tif (*end)\n> +\t\treturn error(\"cat-blob header contained garbage after length: %s\", header);\n> +\tbuffer_copy_bytes(&backchannel, out, len);\n> +\ttail = buffer_read_line(&backchannel);\n> +\tif (!tail)\n> +\t\treturn 1;\n\nCould you clarify when exactly will this happen?\n\n> +\tif (*tail)\n> +\t\treturn error(\"cat-blob trailing line contained garbage: %s\", tail);\n> +\treturn 0;\n> +}\n> +\n>  void fast_export_blob(uint32_t mode, uint32_t mark, uint32_t len,\n>  \t\t\tuint32_t delta, uint32_t srcMark, uint32_t srcMode,\n>  \t\t\tstruct line_buffer *input)\n>  {\n\nNote to reviewers: The function looks like this in `master`:\nvoid fast_export_blob(uint32_t mode, uint32_t mark, uint32_t len)\n\nNew parameters intrduced in the svn-fe3 series: srcMark, srcMode,\ndelta, input.\n\n> +\tlong preimage_len = 0;\n> +\n> +\tif (delta) {\n> +\t\tif (!preimage.infile)\n> +\t\t\tpreimage.infile = tmpfile();\n\nDidn't you later decide against this and use one tmpfile instead? In\nthis case, the temporary file will be automatically deleted when\n`preimage.infile` goes out of scope.\n\n> +\t\tif (!preimage.infile)\n> +\t\t\tdie(\"Unable to open temp file for blob retrieval\");\n> +\t\tif (srcMark) {\n> +\t\t\tprintf(\"cat-blob :%\"PRIu32\"\\n\", srcMark);\n> +\t\t\tfflush(stdout);\n> +\t\t\tif (srcMode == REPO_MODE_LNK)\n> +\t\t\t\tfwrite(\"link \", 1, 5, preimage.infile);\n\nSpecial handling for symbolic links. Perhaps you should mention it in\na comment here?\n\n> +\t\t\tif (fast_export_save_blob(preimage.infile))\n> +\t\t\t\tdie(\"Failed to retrieve blob for delta application\");\n> +\t\t}\n> +\t\tpreimage_len = ftell(preimage.infile);\n> +\t\tfseek(preimage.infile, 0, SEEK_SET);\n> +\t\tif (!postimage.infile)\n> +\t\t\tpostimage.infile = tmpfile();\n\nOne tmpfile?\n\n> +\t\tif (!postimage.infile)\n> +\t\t\tdie(\"Unable to open temp file for blob application\");\n> +\t\tsvndiff0_apply(input, len, &preimage, postimage.infile);\n> +\t\tlen = ftell(postimage.infile);\n\nSince you already have a preimage_len, perhaps name this postimage_len\nto avoid confusion?\n\n> +\t\tfseek(postimage.infile, 0, SEEK_SET);\n> +\t}\n> +\n>  \tif (mode == REPO_MODE_LNK) {\n>  \t\t/* svn symlink blobs start with \"link \" */\n> -\t\tbuffer_skip_bytes(input, 5);\n> +\t\tif (delta)\n> +\t\t\tbuffer_skip_bytes(&postimage, 5);\n> +\t\telse\n> +\t\t\tbuffer_skip_bytes(input, 5);\n>  \t\tlen -= 5;\n>  \t}\n>  \tprintf(\"blob\\nmark :%\"PRIu32\"\\ndata %\"PRIu32\"\\n\", mark, len);\n> -\tbuffer_copy_bytes(input, stdout, len);\n> +\tif (!delta)\n> +\t\tbuffer_copy_bytes(input, stdout, len);\n> +\telse\n> +\t\tbuffer_copy_bytes(&postimage, stdout, len);\n>  \tfputc('\\n', stdout);\n\nI should have asked this a long time ago: why the extra newline?\n\n> +\n> +\tif (preimage.infile) {\n> +\t\tfseek(preimage.infile, 0, SEEK_SET);\n> +\t}\n> +\n> +\tif (postimage.infile) {\n> +\t\tfseek(postimage.infile, 0, SEEK_SET);\n> +\t}\n\nStyle nits: The extra braces around the `if` statement are unnecessary.\n\nOverall, pleasant read. Thanks for taking this forward.\n\n-- Ram\n"},{"id":"153697","messageId":"20101018073605.GF22376@kytes","threadId":"25454","inReplyTo":"1287147256-9457-2-git-send-email-david.barr@cordelta.com","subject":"Re: [PATCH 1/5] fast-import: Let importers retrieve blobs","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2010-10-18T07:36:08Z","receivedAt":"2010-10-18T07:36:08Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Hi David,\n\nDavid Barr writes:\n> As the description of the \"progress\" option in git-fast-import.1\n> hints, there is no convenient way to immediately access the blobs\n> written to a new repository through fast-import.  Until a checkpoint\n> has been started and finishes writing the pack index, any new blobs\n> will not be accessible using standard git tools.\n> \n> So introduce another way: a \"cat-blob\" command introduced in the\n> command stream requests for fast-import to print a blob to stdout\n> or a file descriptor specified by the argument --cat-blob-fd.\n> \n> The output uses the same format as \"git cat-file --batch\".\n\nNice :) It looks like we finally have a nice polished version.\nCaution: Most of the review are just notes to self, or style\nnitpicks. Feel free to ignore.\n\n> Cc: Shawn O. Pearce <spearce@spearce.org>\n> Cc: Ramkumar Ramachandra <artagnon@gmail.com>\n> Helped-by: Sverre Rabbelier <srabbelier@gmail.com>\n> Based-on-patch-by: Jonathan Nieder <jrnieder@gmail.com>\n> Signed-off-by: David Barr <david.barr@cordelta.com>\n> ---\n>  Documentation/git-fast-import.txt |   34 ++++++++++++++\n>  fast-import.c                     |   92 +++++++++++++++++++++++++++++++++++++\n>  2 files changed, 126 insertions(+), 0 deletions(-)\n> \n> diff --git a/Documentation/git-fast-import.txt b/Documentation/git-fast-import.txt\n> index 966ba4f..42a100b 100644\n> --- a/Documentation/git-fast-import.txt\n> +++ b/Documentation/git-fast-import.txt\n> @@ -92,6 +92,17 @@ OPTIONS\n>  \t--(no-)-relative-marks= with the --(import|export)-marks=\n>  \toptions.\n>  \n> +--cat-blob-fd=<fd>::\n> +\tSpecify the file descriptor that will be written to\n> +\twhen the `cat-blob` command is encountered in the stream.\n> +\tThe default behaviour is to write to `stdout`.\n> ++\n> +The described objects are not necessarily accessible\n> +using standard git plumbing tools until a little while\n> +after the next checkpoint.  To request access to the\n> +blobs before then, use `cat-blob` lines in the command\n> +stream.\n> +\n>  --export-pack-edges=<file>::\n>  \tAfter creating a packfile, print a line of data to\n>  \t<file> listing the filename of the packfile and the last\n> @@ -320,6 +331,11 @@ and control the current import process.  More detailed discussion\n>  \tstandard output.  This command is optional and is not needed\n>  \tto perform an import.\n>  \n> +`cat-blob`::\n> +\tCauses fast-import to print a blob in 'cat-file --batch'\n> +\tformat to the file descriptor set with `--cat-blob-fd` or\n> +\t`stdout` if unspecified.\n> +\n>  `feature`::\n>  \tRequire that fast-import supports the specified feature, or\n>  \tabort if it does not.\n> @@ -876,6 +892,23 @@ Placing a `progress` command immediately after a `checkpoint` will\n>  inform the reader when the `checkpoint` has been completed and it\n>  can safely access the refs that fast-import updated.\n>  \n> +`cat-blob`\n> +~~~~~\n> +Causes fast-import to print a blob to a file descriptor previously\n> +arranged with the `--cat-blob-fd` argument.  The command otherwise\n> +has no impact on the current import; its main purpose is to\n> +retrieve blobs that may be in fast-import's memory but not\n> +accessible from the target repository a little quicker than by the\n> +method suggested by the description of the `progress` option.\n> +\n> +....\n> +\t'cat-blob' SP <dataref> LF\n> +....\n> +\n> +The `<dataref>` can be either a mark reference (`:<idnum>`)\n> +set previously, or a full 40-byte SHA-1 of any Git blob,\n> +preexisting or ready to be written.\n> +\n>  `feature`\n>  ~~~~~~~~~\n>  Require that fast-import supports the specified feature, or abort if\n> @@ -896,6 +929,7 @@ The following features are currently supported:\n>  * date-format\n>  * import-marks\n>  * export-marks\n> +* cat-blob\n>  * relative-marks\n>  * no-relative-marks\n>  * force\n\nLooks perfect.\n\n> diff --git a/fast-import.c b/fast-import.c\n> index 2317b0f..ea3e529 100644\n> --- a/fast-import.c\n> +++ b/fast-import.c\n> @@ -55,6 +55,8 @@ Format of STDIN stream:\n>      ('from' sp committish lf)?\n>      lf?;\n>  \n> +  cat_blob ::= 'cat-blob' sp (hexsha1 | idnum) lf;\n> +\n>    checkpoint ::= 'checkpoint' lf\n>      lf?;\n>  \n> @@ -361,6 +363,9 @@ static uintmax_t next_mark;\n>  static struct strbuf new_data = STRBUF_INIT;\n>  static int seen_data_command;\n>  \n> +/* Where to write output of cat-blob commands */\n> +static int cat_blob_fd = 1;\n> +\n\nRight. Defaults to stdout, as described in documentation.\n\n>  static void parse_argv(void);\n>  \n>  static void write_branch_report(FILE *rpt, struct branch *b)\n> @@ -2680,6 +2685,77 @@ static void parse_reset_branch(void)\n>  \t\tunread_command_buf = 1;\n>  }\n>  \n> +static void cat_blob_write(const char *buf, unsigned long size)\n> +{\n> +\tif (write_in_full(cat_blob_fd, buf, size) != size)\n> +\t\tdie_errno(\"Write to frontend failed\");\n> +}\n\nNote to self: I didn't notice write_in_full in wrapper.c\nearlier. Returns the actual size written to the fd from the buffer.\n\n> +static void cat_blob(struct object_entry *oe, unsigned char sha1[20])\n> +{\n> +\tstruct strbuf line = STRBUF_INIT;\n> +\tunsigned long size;\n> +\tenum object_type type = 0;\n> +\tchar *buf;\n> +\n> +\tif (oe && oe->pack_id != MAX_PACK_ID) {\n> +\t\ttype = oe->type;\n> +\t\tbuf = gfi_unpack_entry(oe, &size);\n> +\t} else {\n> +\t\tbuf = read_sha1_file(sha1, &type, &size);\n> +\t}\n> +\tif (!buf)\n> +\t\tdie(\"Can't read object %s\", sha1_to_hex(sha1));\n\nOk. Looks similar enough to the code in the other functions.\n\n> +\n> +\t/*\n> +\t * Output based on batch_one_object() from cat-file.c.\n> +\t */\n> +\tif (type <= 0) {\n> +\t\tstrbuf_reset(&line);\n> +\t\tstrbuf_addf(&line, \"%s missing\\n\", sha1_to_hex(sha1));\n> +\t\tcat_blob_write(line.buf, line.len);\n> +\t\treturn;\n> +\t} else if (type != OBJ_BLOB) {\n> +\t\tdie(\"Object %s is a %s but a blob was expected.\",\n> +\t\t    sha1_to_hex(sha1), typename(type));\n> +\t}\n> +\tstrbuf_reset(&line);\n> +\tstrbuf_addf(&line, \"%s %s %lu\\n\", sha1_to_hex(sha1),\n> +\t\t\t\t\t\ttypename(type), size);\n> +\tcat_blob_write(line.buf, line.len);\n> +\tcat_blob_write(buf, size);\n> +\tcat_blob_write(\"\\n\", 1);\n> +\tfree(buf);\n\nCool.\n\n> +}\n> +\n> +\n> +static void parse_cat_blob(void)\n> +{\n> +\tconst char *p;\n> +\tstruct object_entry *oe = oe;\n> +\tunsigned char sha1[20];\n> +\n> +\t/* cat SP <object> */\n> +\tp = command_buf.buf + strlen(\"cat-blob \");\n> +\tif (*p == ':') {\n> +\t\tchar *x;\n> +\t\toe = find_mark(strtoumax(p + 1, &x, 10));\n> +\t\tif (x == p + 1)\n> +\t\t\tdie(\"Invalid mark: %s\", command_buf.buf);\n> +\t\tif (!oe)\n> +\t\t\tdie(\"Unknown mark: %s\", command_buf.buf);\n> +\t\tp = x;\n> +\t\thashcpy(sha1, oe->idx.sha1);\n\nThe mark case.\n\n> +\t} else {\n> +\t\tif (get_sha1_hex(p, sha1))\n> +\t\t\tdie(\"Invalid SHA1: %s\", command_buf.buf);\n> +\t\tp += 40;\n> +\t\toe = find_object(sha1);\n> +\t}\n\nThe SHA1 case.\nLooks similar enough to the code in the other functions.\n\n> +\n> +\tcat_blob(oe, sha1);\n> +}\n> +\n>  static void parse_checkpoint(void)\n>  {\n>  \tif (object_count) {\n> @@ -2755,6 +2831,14 @@ static void option_export_marks(const char *marks)\n>  \tsafe_create_leading_directories_const(export_marks_file);\n>  }\n>  \n> +static void option_cat_blob_fd(const char *fd)\n> +{\n> +\tunsigned long n = strtoul(fd, NULL, 0);\n> +\tif (n > (unsigned long) INT_MAX)\n> +\t\tdie(\"--cat-blob-fd cannot exceed %d\", INT_MAX);\n> +\tcat_blob_fd = (int) n;\n> +}\n> +\n\nYou don't display an appropriate error when n < 0.\n\n>  static void option_export_pack_edges(const char *edges)\n>  {\n>  \tif (pack_edges)\n> @@ -2808,6 +2892,8 @@ static int parse_one_feature(const char *feature, int from_stream)\n>  \t\toption_import_marks(feature + 13, from_stream);\n>  \t} else if (!prefixcmp(feature, \"export-marks=\")) {\n>  \t\toption_export_marks(feature + 13);\n> +\t} else if (!prefixcmp(feature, \"cat-blob\")) {\n> +\t\t/* Don't die - this feature is supported */\n\nStyle nit: There are no statements in this branch. Maybe put a\nsemicolon?\n\n>  \t} else if (!prefixcmp(feature, \"relative-marks\")) {\n>  \t\trelative_marks_paths = 1;\n>  \t} else if (!prefixcmp(feature, \"no-relative-marks\")) {\n> @@ -2896,6 +2982,10 @@ static void parse_argv(void)\n>  \t\tif (*a != '-' || !strcmp(a, \"--\"))\n>  \t\t\tbreak;\n>  \n> +\t\tif (!prefixcmp(a + 2, \"cat-blob-fd=\")) {\n> +\t\t\toption_cat_blob_fd(a + 2 + 12);\n> +\t\t}\n> +\n\nStyle nit: Unnecessary braces around `if` statement.\n\n>  \t\tif (parse_one_option(a + 2))\n>  \t\t\tcontinue;\n>  \n> @@ -2953,6 +3043,8 @@ int main(int argc, const char **argv)\n>  \t\t\tparse_new_tag();\n>  \t\telse if (!prefixcmp(command_buf.buf, \"reset \"))\n>  \t\t\tparse_reset_branch();\n> +\t\telse if (!prefixcmp(command_buf.buf, \"cat-blob \"))\n> +\t\t\tparse_cat_blob();\n>  \t\telse if (!strcmp(\"checkpoint\", command_buf.buf))\n>  \t\t\tparse_checkpoint();\n>  \t\telse if (!prefixcmp(command_buf.buf, \"progress \"))\n> -- \n> 1.7.3.32.g634ef\n> \n\nOk. I'm eager to see this go through to `master`.\nReviewed-by: Ramkumar Ramachandra <artagnon@gmail.com>\n\n-- Ram\n"},{"id":"153698","messageId":"20101018082612.GB3979@burratino","threadId":"25454","inReplyTo":"1287147256-9457-2-git-send-email-david.barr@cordelta.com","subject":"Re: [PATCH 1/5] fast-import: Let importers retrieve blobs","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-10-18T08:26:12Z","receivedAt":"2010-10-18T08:26:12Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"(+cc: Sam)\n\nHi,\n\nDavid Barr wrote:\n\n> So introduce another way: a \"cat-blob\" command introduced in the\n> command stream requests for fast-import to print a blob to stdout\n> or a file descriptor specified by the argument --cat-blob-fd.\n\nYes, please!\n\n> Cc: Shawn O. Pearce <spearce@spearce.org>\n> Cc: Ramkumar Ramachandra <artagnon@gmail.com>\n\nIt turns out these Cc tags are not supposed to be used except in\nsome very weird circumstances.  See [1] if curious.\n\n[...]\n> --- a/Documentation/git-fast-import.txt\n> +++ b/Documentation/git-fast-import.txt\n> @@ -92,6 +92,17 @@ OPTIONS\n>  \t--(no-)-relative-marks= with the --(import|export)-marks=\n>  \toptions.\n>  \n> +--cat-blob-fd=<fd>::\n> +\tSpecify the file descriptor that will be written to\n> +\twhen the `cat-blob` command is encountered in the stream.\n> +\tThe default behaviour is to write to `stdout`.\n\nSounds good.\n\n> ++\n> +The described objects are not necessarily accessible\n> +using standard git plumbing tools until a little while\n> +after the next checkpoint.  To request access to the\n> +blobs before then, use `cat-blob` lines in the command\n> +stream.\n\nThis is stale explanation from --report-fd, I think, to explain\nwhy the commit ids it printed were not very useful.  It would\nbe possible to reword it to describe cat-blob-fd but since the\nfrontend does not have easy access to blob names as it is, I\nthink cat-blob motivates itself on its own.\n\n[...]\n> @@ -876,6 +892,23 @@ Placing a `progress` command immediately after a `checkpoint` will\n>  inform the reader when the `checkpoint` has been completed and it\n>  can safely access the refs that fast-import updated.\n>  \n> +`cat-blob`\n> +~~~~~\n\n   ~~~~~~~~~~\n\n> @@ -896,6 +929,7 @@ The following features are currently supported:\n>  * date-format\n>  * import-marks\n>  * export-marks\n> +* cat-blob\n\nThe explanation says (paraphrased) \"Features work identically to their\noption counterparts, with the exception of import-marks as described\nbelow\".\n\nMaybe ought to be reworded?\n\n date-format::\n export-marks::\n relative-marks::\n no-relative-marks::\n force::\n\tSee the corresponding command-line option.\n\n import-marks::\n\tLike --import-marks, except in two respects.  First, only one\n\t\"feature import-marks\" command is allowed per stream.  Second,\n\tan --import-marks= specified on the command line will override it.\n\n cat-blob::\n\tNo-op to check that the importer supports the cat-blob command.\n\nBy the way, it might be nice to make cat-blob not just check the importer\nbut the environment in which it was invoked, like this:\n\n\texporter says:\n\t\tfeature this\n\t\tfeature that\n\t\tfeature cat-blob\n\t\tfeature another\n\t\t...\n\n\timporter says:\n\t\tfeature cat-blob\n\nThat is, after writing \"feature cat-blob\\n\", an exporter could tell if\nthe backchannel was set up correctly by reading for \"feature cat-blob\\n\"\nfrom the importer.\n\n> --- a/fast-import.c\n> +++ b/fast-import.c\n> @@ -2680,6 +2685,77 @@ static void parse_reset_branch(void)\n>  \t\tunread_command_buf = 1;\n>  }\n>  \n> +static void cat_blob_write(const char *buf, unsigned long size)\n> +{\n> +\tif (write_in_full(cat_blob_fd, buf, size) != size)\n> +\t\tdie_errno(\"Write to frontend failed\");\n> +}\n\nAn odd operation, since if the pipe_buf gets filled then it blocks\nuntil the exporter finds time to read.  Maybe in some future version\nthis would write to a private ring buffer and there would be an\nevent loop or seperate thread to flush it out when the exporter is\nready.\n\nUpshot: I am happy with this as a separate function.\n\n[...]\n> @@ -2808,6 +2892,8 @@ static int parse_one_feature(const char *feature, int from_stream)\n>  \t\toption_import_marks(feature + 13, from_stream);\n>  \t} else if (!prefixcmp(feature, \"export-marks=\")) {\n>  \t\toption_export_marks(feature + 13);\n> +\t} else if (!prefixcmp(feature, \"cat-blob\")) {\n> +\t\t/* Don't die - this feature is supported */\n\nMaybe if (!strcmp(...?\n\n[...]\n> @@ -2896,6 +2982,10 @@ static void parse_argv(void)\n>  \t\tif (*a != '-' || !strcmp(a, \"--\"))\n>  \t\t\tbreak;\n>  \n> +\t\tif (!prefixcmp(a + 2, \"cat-blob-fd=\")) {\n> +\t\t\toption_cat_blob_fd(a + 2 + 12);\n> +\t\t}\n> +\n>  \t\tif (parse_one_option(a + 2))\n>  \t\t\tcontinue;\n\nProbably worth mentioning in the manual, under the option command:\n\n  The following command-line option describes the environment\n  in which fast-import was executed and may not be passed to\n  'option':\n\n   * cat-blob-fd\n\n> @@ -2953,6 +3043,8 @@ int main(int argc, const char **argv)\n>  \t\t\tparse_new_tag();\n>  \t\telse if (!prefixcmp(command_buf.buf, \"reset \"))\n>  \t\t\tparse_reset_branch();\n> +\t\telse if (!prefixcmp(command_buf.buf, \"cat-blob \"))\n> +\t\t\tparse_cat_blob();\n\nThanks.  That was simple. :)\n\nTests?\n\n[1] http://thread.gmane.org/gmane.comp.version-control.git/157711\n"},{"id":"153699","messageId":"20101018085002.GC3979@burratino","threadId":"25454","inReplyTo":"20101018073605.GF22376@kytes","subject":"Re: [PATCH 1/5] fast-import: Let importers retrieve blobs","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-10-18T08:50:02Z","receivedAt":"2010-10-18T08:50:02Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Ramkumar Ramachandra wrote:\n\n> Note to self: I didn't notice write_in_full in wrapper.c\n> earlier. Returns the actual size written to the fd from the buffer.\n\nYeah, write_in_full() is write() without the partial-read semantics\nwhen interrupted by a signal, on Windows, or encountering an empty\npipe.\n\n>> +static void option_cat_blob_fd(const char *fd)\n>> +{\n>> +\tunsigned long n = strtoul(fd, NULL, 0);\n>> +\tif (n > (unsigned long) INT_MAX)\n>> +\t\tdie(\"--cat-blob-fd cannot exceed %d\", INT_MAX);\n>> +\tcat_blob_fd = (int) n;\n>> +}\n>> +\n>\n> You don't display an appropriate error when n < 0.\n\nHow can n be < 0?  strtoul returns an unsigned long.\n\nBut more to the point, yes, this does not return an appropriate\nerror when \"--cat-blob-fd=\" is not followed by an unsigned\ninteger.  At least it's consistent with --depth=nonsense et al.\n\nRough patch below (needs tests).\n\n> Ok. I'm eager to see this go through to `master`.\n> Reviewed-by: Ramkumar Ramachandra <artagnon@gmail.com>\n\nThanks.\n\nSigned-off-by: Jonathan Nieder <jrnieder@gmail.com>\n---\ndiff --git a/fast-import.c b/fast-import.c\nindex eb6860d..20023c1 100644\n--- a/fast-import.c\n+++ b/fast-import.c\n@@ -2822,16 +2822,25 @@ static void option_date_format(const char *fmt)\n \t\tdie(\"unknown --date-format argument %s\", fmt);\n }\n \n+static unsigned long ulong_arg(const char *arg)\n+{\n+\tchar *endptr;\n+\tunsigned long rv = strtoul(arg, &endptr, 0);\n+\tif (endptr == arg || *endptr)\n+\t\tdie(\"%s: argument must be an unsigned integer\", arg);\n+\treturn rv;\n+}\n+\n static void option_depth(const char *depth)\n {\n-\tmax_depth = strtoul(depth, NULL, 0);\n+\tmax_depth = ulong_arg(depth);\n \tif (max_depth > MAX_DEPTH)\n \t\tdie(\"--depth cannot exceed %u\", MAX_DEPTH);\n }\n \n static void option_active_branches(const char *branches)\n {\n-\tmax_active_branches = strtoul(branches, NULL, 0);\n+\tmax_active_branches = ulong_arg(branches);\n }\n \n static void option_export_marks(const char *marks)\n@@ -2842,7 +2851,7 @@ static void option_export_marks(const char *marks)\n \n static void option_cat_blob_fd(const char *fd)\n {\n-\tunsigned long n = strtoul(fd, NULL, 0);\n+\tunsigned long n = ulong_arg(fd);\n \tif (n > (unsigned long) INT_MAX)\n \t\tdie(\"--cat-blob-fd cannot exceed %d\", INT_MAX);\n \tcat_blob_fd = (int) n;\n"},{"id":"153701","messageId":"20101018085930.GA5425@burratino","threadId":"25454","inReplyTo":"1287147256-9457-5-git-send-email-david.barr@cordelta.com","subject":"Re: [PATCH 4/5] vcs-svn: Add outfile option to buffer_copy_bytes()","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-10-18T08:59:30Z","receivedAt":"2010-10-18T08:59:30Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"David Barr wrote:\n\n> Explicitly declare that output is to stdout for existing use.\n> Allow users of buffer_copy_bytes() to specify the output file.\n\nProbably worth mentioning the motivation, which is presumably\nthat svn-fe will be streaming the preimage for files expressed as\ndeltas from the cat-file-fd to a temporary file.\n\n> Signed-off-by: David Barr <david.barr@cordelta.com>\n\nReviewed-by: Jonathan Nieder <jrnieder@gmail.com>\n\nThanks.  (It's a good API change.)\n"},{"id":"153704","messageId":"20101018092418.GB5425@burratino","threadId":"25454","inReplyTo":"20101018065657.GE22376@kytes","subject":"Re: [PATCH 5/5] svn-fe: Use the cat-blob command to apply deltas","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-10-18T09:24:18Z","receivedAt":"2010-10-18T09:24:18Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi Ram,\n\nGlad to see you are feeling a little better.\n\nRamkumar Ramachandra wrote:\n> David Barr writes:\n\n>> +\tif (!backchannel.infile)\n>> +\t\tbackchannel.infile = fdopen(REPORT_FILENO, \"r\");\n>> +\tif (!backchannel.infile)\n>> +\t\treturn error(\"Could not open backchannel fd: %d\", REPORT_FILENO);\n>\n> REPORT_FILENO = 3 is hard-coded. Is this intended? Maybe a\n> command-line option to specify the fd?\n\nfast-import gets the --cat-file-fd parameter to choose between stdout,\nstdin-as-socket, stderr, or another fd (not necessarily 3 because it\nmight have to compete with other similar features some day).\n\nFor svn-fe, it is just like another stdin.  stdin is always fd 0,\nso...\n\nFor callers other than svn-fe, it would be especially useful to\nmake it configurable, yes.\n\n>> +\ttail = buffer_read_line(&backchannel);\n>> +\tif (!tail)\n>> +\t\treturn 1;\n>\n> Could you clarify when exactly will this happen?\n\nbuffer_read_line() returns NULL on error and when data is exhausted\nwithout the trailing newline appearing.  The input here is supposed to\nbe just a single newline (trimmed to an empty string).\n\n>> +\tlong preimage_len = 0;\n>> +\n>> +\tif (delta) {\n>> +\t\tif (!preimage.infile)\n>> +\t\t\tpreimage.infile = tmpfile();\n>\n> Didn't you later decide against this and use one tmpfile instead?\n\nThis is a single tempfile (because static).  Or am I missing\nsomething?\n\n>> +\t\tif (!preimage.infile)\n>> +\t\t\tdie(\"Unable to open temp file for blob retrieval\");\n>> +\t\tif (srcMark) {\n>> +\t\t\tprintf(\"cat-blob :%\"PRIu32\"\\n\", srcMark);\n>> +\t\t\tfflush(stdout);\n>> +\t\t\tif (srcMode == REPO_MODE_LNK)\n>> +\t\t\t\tfwrite(\"link \", 1, 5, preimage.infile);\n>\n> Special handling for symbolic links. Perhaps you should mention it in\n> a comment here?\n\nOr better yet, a comment in the commit message. :)\n\n>> +\t\t\tif (fast_export_save_blob(preimage.infile))\n>> +\t\t\t\tdie(\"Failed to retrieve blob for delta application\");\n>> +\t\t}\n>> +\t\tpreimage_len = ftell(preimage.infile);\n>> +\t\tfseek(preimage.infile, 0, SEEK_SET);\n>> +\t\tif (!postimage.infile)\n>> +\t\t\tpostimage.infile = tmpfile();\n>\n> One tmpfile?\n\nDo you mean letting the preimage and postimage share a file?\n\n[...]\n>>  \tprintf(\"blob\\nmark :%\"PRIu32\"\\ndata %\"PRIu32\"\\n\", mark, len);\n>> -\tbuffer_copy_bytes(input, stdout, len);\n>> +\tif (!delta)\n>> +\t\tbuffer_copy_bytes(input, stdout, len);\n>> +\telse\n>> +\t\tbuffer_copy_bytes(&postimage, stdout, len);\n>>  \tfputc('\\n', stdout);\n>\n> I should have asked this a long time ago: why the extra newline?\n\n>From the fast-import manual:\n\n\tThe LF after <raw> is optional (it used to be required)\n\tbut recommended. Always including it makes debugging a\n\tfast-import stream easier as the next command always\n\tstarts in column 0 of the next line, even if <raw> did\n\tnot end with an LF.\n\n> Overall, pleasant read. Thanks for taking this forward.\n\nSeconded.  Thanks, both.\n"},{"id":"153706","messageId":"20101018095408.GA5641@burratino","threadId":"25454","inReplyTo":"1287147256-9457-1-git-send-email-david.barr@cordelta.com","subject":"Re: [PATCHv2] Add support for subversion dump format v3","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-10-18T09:54:08Z","receivedAt":"2010-10-18T09:54:08Z","isPatch":false,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"David Barr wrote:\n\n> Patch 1 adds the required infrastructure to fast-import.\n> This features the addition of the cat-blob command\n\nPatch 1: maybe someone wants to pick this up and make the minor\nchanges it needs (a test or two to maintain sanity)?\n\n> Patch 2 adds the basic parsing necessary to process the v3 format.\n\nThe log message doesn't give context but the patch is good\nand safe.  Unknown keys are ignored so it is basically a\nno-op except for using a little more memory.\n\n> Patch 3 adds logic around decoding prop deltas.\n\nIt would be nice if someone who is not Junio cleans up the style.\n\nPatch 4 (unmentioned for some reason): the log message doesn't give\ncontext but the patch is good.  I think this could be picked up right\naway.  There would be semantically unimportant merge conflicts if\ncherry-picking without the patches introducing buffer_read_binary()\nand changing buffer_copy_bytes() to take an off_t.\n\n> Patch 5 integrates svn-fe with svn-da to decode text deltas.\n\nI like it a lot but am interested in the follow-ups to Ram's comments.\nOf course this requires the svn-da series so I'd prefer to give it\na few more days' cooking.\n\nSummary:\n\n - patch 4 could be picked up right away imho\n - the rest need some work, but not much\n - the series is available from\n   git://github.com/barrbrain/git.git svn-fe3\n\nRegards,\nJonathan\n"},{"id":"153715","messageId":"20101018121822.GG22376@kytes","threadId":"25454","inReplyTo":"20101018092418.GB5425@burratino","subject":"Re: [PATCH 5/5] svn-fe: Use the cat-blob command to apply deltas","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2010-10-18T12:18:28Z","receivedAt":"2010-10-18T12:18:28Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Hi Jonathan,\n\nJonathan Nieder writes:\n> Ramkumar Ramachandra wrote:\n> > David Barr writes:\n> \n> >> +\tif (!backchannel.infile)\n> >> +\t\tbackchannel.infile = fdopen(REPORT_FILENO, \"r\");\n> >> +\tif (!backchannel.infile)\n> >> +\t\treturn error(\"Could not open backchannel fd: %d\", REPORT_FILENO);\n> >\n> > REPORT_FILENO = 3 is hard-coded. Is this intended? Maybe a\n> > command-line option to specify the fd?\n> \n> fast-import gets the --cat-file-fd parameter to choose between stdout,\n> stdin-as-socket, stderr, or another fd (not necessarily 3 because it\n> might have to compete with other similar features some day).\n> \n> For svn-fe, it is just like another stdin.  stdin is always fd 0,\n> so...\n> \n> For callers other than svn-fe, it would be especially useful to\n> make it configurable, yes.\n\nRight, got it.\n\n> >> +\ttail = buffer_read_line(&backchannel);\n> >> +\tif (!tail)\n> >> +\t\treturn 1;\n> >\n> > Could you clarify when exactly will this happen?\n> \n> buffer_read_line() returns NULL on error and when data is exhausted\n> without the trailing newline appearing.  The input here is supposed to\n> be just a single newline (trimmed to an empty string).\n\nThanks for the clarification.\n\n> >> +\tlong preimage_len = 0;\n> >> +\n> >> +\tif (delta) {\n> >> +\t\tif (!preimage.infile)\n> >> +\t\t\tpreimage.infile = tmpfile();\n> >\n> > Didn't you later decide against this and use one tmpfile instead?\n> \n> This is a single tempfile (because static).  Or am I missing\n> something?\n\nEr, sorry about that. When I saw this code, it immediately reminded me\nof one of David's commits that used several temporary files- a later\none made it a global variable. I didn't notice the static here.\n\n> >> +\t\tif (!preimage.infile)\n> >> +\t\t\tdie(\"Unable to open temp file for blob retrieval\");\n> >> +\t\tif (srcMark) {\n> >> +\t\t\tprintf(\"cat-blob :%\"PRIu32\"\\n\", srcMark);\n> >> +\t\t\tfflush(stdout);\n> >> +\t\t\tif (srcMode == REPO_MODE_LNK)\n> >> +\t\t\t\tfwrite(\"link \", 1, 5, preimage.infile);\n> >\n> > Special handling for symbolic links. Perhaps you should mention it in\n> > a comment here?\n> \n> Or better yet, a comment in the commit message. :)\n\n*nod*\n\n> >> +\t\t\tif (fast_export_save_blob(preimage.infile))\n> >> +\t\t\t\tdie(\"Failed to retrieve blob for delta application\");\n> >> +\t\t}\n> >> +\t\tpreimage_len = ftell(preimage.infile);\n> >> +\t\tfseek(preimage.infile, 0, SEEK_SET);\n> >> +\t\tif (!postimage.infile)\n> >> +\t\t\tpostimage.infile = tmpfile();\n> >\n> > One tmpfile?\n> \n> Do you mean letting the preimage and postimage share a file?\n\nNo :)\n\n> [...]\n> >>  \tprintf(\"blob\\nmark :%\"PRIu32\"\\ndata %\"PRIu32\"\\n\", mark, len);\n> >> -\tbuffer_copy_bytes(input, stdout, len);\n> >> +\tif (!delta)\n> >> +\t\tbuffer_copy_bytes(input, stdout, len);\n> >> +\telse\n> >> +\t\tbuffer_copy_bytes(&postimage, stdout, len);\n> >>  \tfputc('\\n', stdout);\n> >\n> > I should have asked this a long time ago: why the extra newline?\n> \n> From the fast-import manual:\n> \n> \tThe LF after <raw> is optional (it used to be required)\n> \tbut recommended. Always including it makes debugging a\n> \tfast-import stream easier as the next command always\n> \tstarts in column 0 of the next line, even if <raw> did\n> \tnot end with an LF.\n\nThanks for the explanation. I really should have looked this up\nearlier, but I suppose it's not a biggie.\n\n-- Ram\n"},{"id":"153718","messageId":"20101018151011.GH22376@kytes","threadId":"25454","inReplyTo":"1287147256-9457-4-git-send-email-david.barr@cordelta.com","subject":"Re: [PATCH 3/5] vcs-svn: Implement prop-delta handling.","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2010-10-18T15:10:25Z","receivedAt":"2010-10-18T15:10:25Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Hi again,\n\nHere's another review.\n\nDavid Barr writes:\n> By testing against the Apache Software Foundation\n> repository, some simple rules for decoding prop\n> deltas were derived.\n> \n> 'Node-action: replace' implies the empty prop set\n> as the base for the delta.\n> Otherwise, if a copyfrom source is given that node\n> forms the basis for the delta.\n> Lastly, if the destination path exists in the active\n> revision it forms the basis.\n> \n> The same rules ought to apply to text deltas as well.\n> \n> Apply these rules to prop handling.\n\n> Add a placeholder srcMark parameter to fast_export_blob().\n\nIs this related to the Prop-delta handling? Why is it in this patch?\n\n> Signed-off-by: David Barr <david.barr@cordelta.com>\n\nNit: I don't know how you managed to wrap your commit message like\nthat- it looks like you did it by hand. My Emacs wraps at 70\ncharacters, and that seems to be the convention in git.git as well.\n\nAlso, I don't like the commit message. Maybe something like this would\nbe clearer?\n\n-- 8< --\nHandle property deltas that occur in dumpfile v3. While \"Prop-delta:\nfalse\" trivially implies that all the properties are given in full,\n\"Prop-delta: true\" implies a delta against:\n\n1. The props of the previous revision of the node when `Node-action`\n   is `change`.\n2. Nothing when `Node-action` is `add`. However, when\n   `Node-copyfrom-path`/ `Node-copyfrom-rev` headers are present, the\n   delta is against the node being copied.\n3. Nothing when `Node-action` is `replace` and the destination path\n   doesn't already exist in the current revision. If\n   `Node-copyfrom-path`/ `Node-copyfrom-rev` headers are present, the\n   delta is against the node being copied. Finally, if the destination\n   path already exists in the current revision, the delta is against\n   the props of that node.\n\n> diff --git a/vcs-svn/fast_export.c b/vcs-svn/fast_export.c\n> index 260cf50..d984aaa 100644\n> --- a/vcs-svn/fast_export.c\n> +++ b/vcs-svn/fast_export.c\n> @@ -63,7 +63,9 @@ void fast_export_commit(uint32_t revision, uint32_t author, char *log,\n>  \tprintf(\"progress Imported commit %\"PRIu32\".\\n\\n\", revision);\n>  }\n>  \n> -void fast_export_blob(uint32_t mode, uint32_t mark, uint32_t len, struct line_buffer *input)\n> +void fast_export_blob(uint32_t mode, uint32_t mark, uint32_t len,\n> +\t\t\tuint32_t delta, uint32_t srcMark, uint32_t srcMode,\n> +\t\t\tstruct line_buffer *input)\n\nNote to self: You've switched indentation style from \"tabs to align +\nspaces to indent\" to the \"Linux tabs only\" style used in linux.git.\n\nWait, does this change belong here?\n\n> --- a/vcs-svn/fast_export.h\n> +++ b/vcs-svn/fast_export.h\n> @@ -9,6 +9,7 @@ void fast_export_modify(uint32_t depth, uint32_t *path, uint32_t mode,\n>  void fast_export_commit(uint32_t revision, uint32_t author, char *log,\n>  \t\t\tuint32_t uuid, uint32_t url, unsigned long timestamp);\n>  void fast_export_blob(uint32_t mode, uint32_t mark, uint32_t len,\n> -\t\t      struct line_buffer *input);\n> +\t\t\tuint32_t delta, uint32_t srcMark, uint32_t srcMode,\n> +\t\t\tstruct line_buffer *input);\n\nAnd this?\n\n> diff --git a/vcs-svn/repo_tree.c b/vcs-svn/repo_tree.c\n> index e94d91d..b616bda 100644\n> --- a/vcs-svn/repo_tree.c\n> +++ b/vcs-svn/repo_tree.c\n> @@ -157,6 +157,29 @@ static void repo_write_dirent(uint32_t *path, uint32_t mode,\n>  \t\tdent_remove(&dir_pointer(parent_dir_o)->entries, dent);\n>  }\n>  \n> +uint32_t repo_read_mark(uint32_t revision, uint32_t *path)\n> +{\n> +\tuint32_t mode = 0, content_offset = 0;\n> +\tstruct repo_dirent *src_dent;\n> +\tsrc_dent = repo_read_dirent(revision, path);\n> +\tif (src_dent != NULL) {\n> +\t\tmode = src_dent->mode;\n> +\t\tcontent_offset = src_dent->content_offset;\n> +\t}\n> +\treturn mode && mode != REPO_MODE_DIR ? content_offset : 0;\n\nMake this clearer with an `if` statement perhaps? This looks ugly,\nespecially with the reader having to parse it with the correct\noperator precedence in mind.\n\n> +uint32_t repo_read_mode(uint32_t revision, uint32_t *path)\n> +{\n> +\tuint32_t mode = 0;\n> +\tstruct repo_dirent *src_dent;\n> +\tsrc_dent = repo_read_dirent(revision, path);\n> +\tif (src_dent != NULL) {\n> +\t\tmode = src_dent->mode;\n> +\t}\n\nStyle nit: Unnecessary braces around `if` statement.\n\nWait, what does all this have to do with prop deltas? Ok, I found this\nslightly confusing -- I just went through the rest of the patch and\nfound that repo_read_mode and repo_read_mark are dependencies of the\nprop-delta handling code. Maybe put them in a separate patch\nimmediately preceeding this one?\n\n> diff --git a/vcs-svn/svndump.c b/vcs-svn/svndump.c\n> index 458053e..3431c22 100644\n> --- a/vcs-svn/svndump.c\n> +++ b/vcs-svn/svndump.c\n> @@ -45,7 +45,7 @@ static char* log_copy(uint32_t length, char *log)\n>  }\n>  \n>  static struct {\n> -\tuint32_t action, propLength, textLength, srcRev, srcMode, mark, type;\n> +\tuint32_t action, propLength, textLength, srcRev, srcMode, srcMark, mark, type;\n>  \tuint32_t src[REPO_MAX_PATH_DEPTH], dst[REPO_MAX_PATH_DEPTH];\n>  \tuint32_t text_delta, prop_delta;\n>  \tchar text_delta_base_md5[MD5_HEX_LENGTH + 1];\n> @@ -86,6 +86,7 @@ static void reset_node_ctx(char *fname)\n>  \tnode_ctx.src[0] = ~0;\n>  \tnode_ctx.srcRev = 0;\n>  \tnode_ctx.srcMode = 0;\n> +\tnode_ctx.srcMark = 0;\n>  \tpool_tok_seq(REPO_MAX_PATH_DEPTH, node_ctx.dst, \"/\", fname);\n>  \tnode_ctx.mark = 0;\n>  \tnode_ctx.text_delta = 0;\n> @@ -168,17 +169,42 @@ static void read_props(void)\n>  \t\t\t}\n>  \t\t\tkey = ~0;\n>  \t\t\tbuffer_read_line(&input);\n> +\t\t} else if (!strncmp(t, \"D \", 2)) {\n> +\t\t\tlen = atoi(&t[2]);\n> +\t\t\tkey = pool_intern(buffer_read_string(&input, len));\n> +\t\t\tbuffer_read_line(&input);\n> +\t\t\tif (key == keys.svn_executable) {\n> +\t\t\t\tif (node_ctx.type == REPO_MODE_EXE)\n> +\t\t\t\t\tnode_ctx.type = REPO_MODE_BLB;\n> +\t\t\t} else if (key == keys.svn_special) {\n> +\t\t\t\tif (node_ctx.type == REPO_MODE_LNK)\n> +\t\t\t\t\tnode_ctx.type = REPO_MODE_BLB;\n> +\t\t\t}\n> +\t\t\tkey = ~0;\n\nDeleted props have to be printed in dumpfile v3 (for obvious reasons:\nhow else would we indicate a delta?). I know this, but I don't know\nwhat another reviewer would make of this. You should mention it\nexplicitly in a comment or in the commit message.\n\nNote to other reviewers:\n\nProps are printed as\nV <key length>\n<key>\nK <value length>\n<value>\n\nDeleted props are printed as\nD <key length>\n<key>\n\nYes, value is omitted.\n\nThis change populates the context with deleted props (more precisely:\nchanges the context when deleted props are encountered). I'm not\nentirely happy that it's part of this patch: why not squash it into\n2/5?\n\n>  static void handle_node(void)\n>  {\n> +\tif (node_ctx.prop_delta) {\n> +\t\tif (node_ctx.srcRev)\n> +\t\t\tnode_ctx.srcMode = repo_read_mode(node_ctx.srcRev, node_ctx.src);\n> +\t\telse\n> +\t\t\tnode_ctx.srcMode = repo_read_mode(rev_ctx.revision, node_ctx.dst);\n> +\t\tif (node_ctx.srcMode && node_ctx.action != NODEACT_REPLACE)\n> +\t\t\tnode_ctx.type = node_ctx.srcMode;\n> +\t}\n> +\n\nOkay, the code to handle prop deltas. As a note to self and for the\nbenefit of other reviewers, here's the English version of the above:\n0. Mode can be one of REPO_MODE_DIR, REPO_MODE_BLB, REPO_MODE_EXE, and\n   REPO_MODE_LNK.\n1. If Node-copyfrom-rev/ Node-copyfrom-path are present, set srcMode\n   to the mode of the source node (as present in the source revision\n   ofcourse).\n2. If not, set the srcMode to the mode of the dst path in the previous\n   revision.\n\nAfter doing this, if srcMode is present and if Node-action is not\nreplace, set the mode (called `type` for historical reasons*?) of the\ndestination node to that of the source node. Now that we've copied the\nprops successfully, the call to read_props() in line 200 will reads\nthe props of the current revision and update the mode accordingly.\n\n* Wait, why must we be stuck with this historical cruft?\n\nSigned-off-by: Ramkumar Ramachandra <artagnon@gmail.com>\n-- 8< --\ndiff --git a/vcs-svn/svndump.c b/vcs-svn/svndump.c\nindex 3431c22..9878e3a 100644\n--- a/vcs-svn/svndump.c\n+++ b/vcs-svn/svndump.c\n@@ -45,7 +45,7 @@ static char* log_copy(uint32_t length, char *log)\n }\n \n static struct {\n-\tuint32_t action, propLength, textLength, srcRev, srcMode, srcMark, mark, type;\n+\tuint32_t action, propLength, textLength, srcRev, srcMode, srcMark, mark, mode;\n \tuint32_t src[REPO_MAX_PATH_DEPTH], dst[REPO_MAX_PATH_DEPTH];\n \tuint32_t text_delta, prop_delta;\n \tchar text_delta_base_md5[MD5_HEX_LENGTH + 1];\n@@ -79,7 +79,7 @@ static struct {\n \n static void reset_node_ctx(char *fname)\n {\n-\tnode_ctx.type = 0;\n+\tnode_ctx.mode = 0;\n \tnode_ctx.action = NODEACT_UNKNOWN;\n \tnode_ctx.propLength = LENGTH_UNKNOWN;\n \tnode_ctx.textLength = LENGTH_UNKNOWN;\n@@ -163,9 +163,9 @@ static void read_props(void)\n \t\t\t\tif (parse_date_basic(val, &rev_ctx.timestamp, NULL))\n \t\t\t\t\tfprintf(stderr, \"Invalid timestamp: %s\\n\", val);\n \t\t\t} else if (key == keys.svn_executable) {\n-\t\t\t\tnode_ctx.type = REPO_MODE_EXE;\n+\t\t\t\tnode_ctx.mode = REPO_MODE_EXE;\n \t\t\t} else if (key == keys.svn_special) {\n-\t\t\t\tnode_ctx.type = REPO_MODE_LNK;\n+\t\t\t\tnode_ctx.mode = REPO_MODE_LNK;\n \t\t\t}\n \t\t\tkey = ~0;\n \t\t\tbuffer_read_line(&input);\n@@ -174,11 +174,11 @@ static void read_props(void)\n \t\t\tkey = pool_intern(buffer_read_string(&input, len));\n \t\t\tbuffer_read_line(&input);\n \t\t\tif (key == keys.svn_executable) {\n-\t\t\t\tif (node_ctx.type == REPO_MODE_EXE)\n-\t\t\t\t\tnode_ctx.type = REPO_MODE_BLB;\n+\t\t\t\tif (node_ctx.mode == REPO_MODE_EXE)\n+\t\t\t\t\tnode_ctx.mode = REPO_MODE_BLB;\n \t\t\t} else if (key == keys.svn_special) {\n-\t\t\t\tif (node_ctx.type == REPO_MODE_LNK)\n-\t\t\t\t\tnode_ctx.type = REPO_MODE_BLB;\n+\t\t\t\tif (node_ctx.mode == REPO_MODE_LNK)\n+\t\t\t\t\tnode_ctx.mode = REPO_MODE_BLB;\n \t\t\t}\n \t\t\tkey = ~0;\n \t\t}\n@@ -193,7 +193,7 @@ static void handle_node(void)\n \t\telse\n \t\t\tnode_ctx.srcMode = repo_read_mode(rev_ctx.revision, node_ctx.dst);\n \t\tif (node_ctx.srcMode && node_ctx.action != NODEACT_REPLACE)\n-\t\t\tnode_ctx.type = node_ctx.srcMode;\n+\t\t\tnode_ctx.mode = node_ctx.srcMode;\n \t}\n \n \tif (node_ctx.propLength != LENGTH_UNKNOWN && node_ctx.propLength)\n@@ -207,7 +207,7 @@ static void handle_node(void)\n \t}\n \n \tif (node_ctx.textLength != LENGTH_UNKNOWN &&\n-\t    node_ctx.type != REPO_MODE_DIR)\n+\t    node_ctx.mode != REPO_MODE_DIR)\n \t\tnode_ctx.mark = next_blob_mark();\n \n \tif (node_ctx.action == NODEACT_DELETE) {\n@@ -215,27 +215,27 @@ static void handle_node(void)\n \t} else if (node_ctx.action == NODEACT_CHANGE ||\n \t\t\t   node_ctx.action == NODEACT_REPLACE) {\n \t\tif (node_ctx.action == NODEACT_REPLACE &&\n-\t\t    node_ctx.type == REPO_MODE_DIR)\n+\t\t    node_ctx.mode == REPO_MODE_DIR)\n \t\t\trepo_replace(node_ctx.dst, node_ctx.mark);\n \t\telse if (node_ctx.propLength != LENGTH_UNKNOWN)\n-\t\t\trepo_modify(node_ctx.dst, node_ctx.type, node_ctx.mark);\n+\t\t\trepo_modify(node_ctx.dst, node_ctx.mode, node_ctx.mark);\n \t\telse if (node_ctx.textLength != LENGTH_UNKNOWN)\n \t\t\tnode_ctx.srcMode = repo_replace(node_ctx.dst, node_ctx.mark);\n \t} else if (node_ctx.action == NODEACT_ADD) {\n \t\tif (node_ctx.srcRev && node_ctx.propLength != LENGTH_UNKNOWN)\n-\t\t\trepo_modify(node_ctx.dst, node_ctx.type, node_ctx.mark);\n+\t\t\trepo_modify(node_ctx.dst, node_ctx.mode, node_ctx.mark);\n \t\telse if (node_ctx.srcRev && node_ctx.textLength != LENGTH_UNKNOWN)\n \t\t\tnode_ctx.srcMode = repo_replace(node_ctx.dst, node_ctx.mark);\n-\t\telse if ((node_ctx.type == REPO_MODE_DIR && !node_ctx.srcRev) ||\n+\t\telse if ((node_ctx.mode == REPO_MODE_DIR && !node_ctx.srcRev) ||\n \t\t\t node_ctx.textLength != LENGTH_UNKNOWN)\n-\t\t\trepo_add(node_ctx.dst, node_ctx.type, node_ctx.mark);\n+\t\t\trepo_add(node_ctx.dst, node_ctx.mode, node_ctx.mark);\n \t}\n \n \tif (node_ctx.propLength == LENGTH_UNKNOWN && node_ctx.srcMode)\n-\t\tnode_ctx.type = node_ctx.srcMode;\n+\t\tnode_ctx.mode = node_ctx.srcMode;\n \n \tif (node_ctx.mark)\n-\t\tfast_export_blob(node_ctx.type, node_ctx.mark, node_ctx.textLength,\n+\t\tfast_export_blob(node_ctx.mode, node_ctx.mark, node_ctx.textLength,\n \t\t\t\tnode_ctx.text_delta, node_ctx.srcMark, node_ctx.srcMode,\n \t\t\t\t&input);\n \telse if (node_ctx.textLength != LENGTH_UNKNOWN)\n@@ -284,9 +284,9 @@ void svndump_read(const char *url)\n \t\t\treset_node_ctx(val);\n \t\t} else if (key == keys.node_kind) {\n \t\t\tif (!strcmp(val, \"dir\"))\n-\t\t\t\tnode_ctx.type = REPO_MODE_DIR;\n+\t\t\t\tnode_ctx.mode = REPO_MODE_DIR;\n \t\t\telse if (!strcmp(val, \"file\"))\n-\t\t\t\tnode_ctx.type = REPO_MODE_BLB;\n+\t\t\t\tnode_ctx.mode = REPO_MODE_BLB;\n \t\t\telse\n \t\t\t\tfprintf(stderr, \"Unknown node-kind: %s\\n\", val);\n \t\t} else if (key == keys.node_action) {\n\n\n>  \tif (node_ctx.propLength != LENGTH_UNKNOWN && node_ctx.propLength)\n>  \t\tread_props();\n>  \n> -\tif (node_ctx.srcRev)\n> +\tif (node_ctx.srcRev) {\n> +\t\tnode_ctx.srcMark = repo_read_mark(node_ctx.srcRev, node_ctx.src);\n>  \t\tnode_ctx.srcMode = repo_copy(node_ctx.srcRev, node_ctx.src, node_ctx.dst);\n> +\t} else {\n> +\t\tnode_ctx.srcMark = repo_read_mark(rev_ctx.revision, node_ctx.dst);\n> +\t}\n\nNit: Braces around `else` branch.\nAgain, does this change belong here?\n\nOverall, a pleasant read. Thanks.\n\n-- Ram\n"},{"id":"156157","messageId":"20101119094738.GD19061@burratino","threadId":"25454","inReplyTo":"20101119093530.GA19061@burratino","subject":"[PATCH 3/4] fast-import: let importers retrieve blobs","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-11-19T09:47:38Z","receivedAt":"2010-11-19T09:47:38Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"From: David Barr <david.barr@cordelta.com>\n\nNew objects written by fast-import are not available immediately.\nUntil a checkpoint has been started and finishes writing the pack\nindex, any new blobs will not be accessible using standard git tools.\n\nSo introduce a new way to access them: a \"cat-blob\" command in the\ncommand stream requests for fast-import to print a blob to stdout or a\nfile descriptor specified by the argument to --cat-blob-fd.  The value\nfor cat-blob-fd cannot be specified in the stream because that would\nbe a layering violation: the decision of where to direct a stream has\nto be made when fast-import is started anyway, so we might as well\nmake the stream format is independent of that detail.\n\nOutput uses the same format as \"git cat-file --batch\".\n\nThanks to Sverre Rabbelier and Sam Vilain for guidance in designing\nthe protocol.\n\nBased-on-patch-by: Jonathan Nieder <jrnieder@gmail.com>\nSigned-off-by: David Barr <david.barr@cordelta.com>\nAcked-by: Ramkumar Ramachandra <artagnon@gmail.com>\nSigned-off-by: Jonathan Nieder <jrnieder@gmail.com>\n---\nIf you use this, you might want to do something like:\n\n\tblob \n\tmark :1\n\tdata <<EOT\n\ttesting 1 2 3\n\tEOT\n\tcat-blob :1\n\nbefore proceeding with the rest of the stream.  This allows wiring\nmistakes to be caught early.\n\n Documentation/git-fast-import.txt |   41 ++++++++\n fast-import.c                     |   95 ++++++++++++++++++\n t/t9300-fast-import.sh            |  193 ++++++++++++++++++++++++++++++++++++-\n 3 files changed, 327 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/git-fast-import.txt b/Documentation/git-fast-import.txt\nindex 3bf04e3..be444da 100644\n--- a/Documentation/git-fast-import.txt\n+++ b/Documentation/git-fast-import.txt\n@@ -92,6 +92,11 @@ OPTIONS\n \t--(no-)-relative-marks= with the --(import|export)-marks=\n \toptions.\n \n+--cat-blob-fd=<fd>::\n+\tSpecify the file descriptor that will be written to\n+\twhen the `cat-blob` command is encountered in the stream.\n+\tThe default behaviour is to write to `stdout`.\n+\n --export-pack-edges=<file>::\n \tAfter creating a packfile, print a line of data to\n \t<file> listing the filename of the packfile and the last\n@@ -320,6 +325,11 @@ and control the current import process.  More detailed discussion\n \tstandard output.  This command is optional and is not needed\n \tto perform an import.\n \n+`cat-blob`::\n+\tCauses fast-import to print a blob in 'cat-file --batch'\n+\tformat to the file descriptor set with `--cat-blob-fd` or\n+\t`stdout` if unspecified.\n+\n `feature`::\n \tRequire that fast-import supports the specified feature, or\n \tabort if it does not.\n@@ -872,6 +882,29 @@ Placing a `progress` command immediately after a `checkpoint` will\n inform the reader when the `checkpoint` has been completed and it\n can safely access the refs that fast-import updated.\n \n+`cat-blob`\n+~~~~~~~~~~\n+Causes fast-import to print a blob to a file descriptor previously\n+arranged with the `--cat-blob-fd` argument.  The command otherwise\n+has no impact on the current import; its main purpose is to\n+retrieve blobs that may be in fast-import's memory but not\n+accessible from the target repository.\n+\n+....\n+\t'cat-blob' SP <dataref> LF\n+....\n+\n+The `<dataref>` can be either a mark reference (`:<idnum>`)\n+set previously or a full 40-byte SHA-1 of a Git blob, preexisting or\n+ready to be written.\n+\n+output uses the same format as `git cat-file --batch`:\n+\n+====\n+\t<sha1> SP 'blob' SP <size> LF\n+\t<contents> LF\n+====\n+\n `feature`\n ~~~~~~~~~\n Require that fast-import supports the specified feature, or abort if\n@@ -898,6 +931,13 @@ import-marks::\n \tsecond, an --import-marks= command-line option overrides\n \tany \"feature import-marks\" command in the stream.\n \n+cat-blob::\n+\tIgnored.  Versions of fast-import not supporting the\n+\t\"cat-blob\" command will exit with a message indicating so.\n+\tThis lets the import error out early with a clear message,\n+\trather than wasting time on the early part of an import\n+\tbefore the unsupported command is detected.\n+\n `option`\n ~~~~~~~~\n Processes the specified option so that git fast-import behaves in a\n@@ -923,6 +963,7 @@ not be passed as option:\n * date-format\n * import-marks\n * export-marks\n+* cat-blob-fd\n * force\n \n Crash Reports\ndiff --git a/fast-import.c b/fast-import.c\nindex 959afef..88547c6 100644\n--- a/fast-import.c\n+++ b/fast-import.c\n@@ -55,6 +55,8 @@ Format of STDIN stream:\n     ('from' sp committish lf)?\n     lf?;\n \n+  cat_blob ::= 'cat-blob' sp (hexsha1 | idnum) lf;\n+\n   checkpoint ::= 'checkpoint' lf\n     lf?;\n \n@@ -361,6 +363,9 @@ static uintmax_t next_mark;\n static struct strbuf new_data = STRBUF_INIT;\n static int seen_data_command;\n \n+/* Where to write output of cat-blob commands */\n+static int cat_blob_fd = STDOUT_FILENO;\n+\n static void parse_argv(void);\n \n static void write_branch_report(FILE *rpt, struct branch *b)\n@@ -2689,6 +2694,79 @@ static void parse_reset_branch(void)\n \t\tunread_command_buf = 1;\n }\n \n+static void cat_blob_write(const char *buf, unsigned long size)\n+{\n+\tif (write_in_full(cat_blob_fd, buf, size) != size)\n+\t\tdie_errno(\"Write to frontend failed\");\n+}\n+\n+static void cat_blob(struct object_entry *oe, unsigned char sha1[20])\n+{\n+\tstruct strbuf line = STRBUF_INIT;\n+\tunsigned long size;\n+\tenum object_type type = 0;\n+\tchar *buf;\n+\n+\tif (!oe || oe->pack_id == MAX_PACK_ID) {\n+\t\tbuf = read_sha1_file(sha1, &type, &size);\n+\t} else {\n+\t\ttype = oe->type;\n+\t\tbuf = gfi_unpack_entry(oe, &size);\n+\t}\n+\n+\t/*\n+\t * Output based on batch_one_object() from cat-file.c.\n+\t */\n+\tif (type <= 0) {\n+\t\tstrbuf_reset(&line);\n+\t\tstrbuf_addf(&line, \"%s missing\\n\", sha1_to_hex(sha1));\n+\t\tcat_blob_write(line.buf, line.len);\n+\t\tfree(buf);\n+\t\treturn;\n+\t}\n+\tif (!buf)\n+\t\tdie(\"Can't read object %s\", sha1_to_hex(sha1));\n+\tif (type != OBJ_BLOB)\n+\t\tdie(\"Object %s is a %s but a blob was expected.\",\n+\t\t    sha1_to_hex(sha1), typename(type));\n+\tstrbuf_reset(&line);\n+\tstrbuf_addf(&line, \"%s %s %lu\\n\", sha1_to_hex(sha1),\n+\t\t\t\t\t\ttypename(type), size);\n+\tcat_blob_write(line.buf, line.len);\n+\tcat_blob_write(buf, size);\n+\tcat_blob_write(\"\\n\", 1);\n+\tfree(buf);\n+}\n+\n+static void parse_cat_blob(void)\n+{\n+\tconst char *p;\n+\tstruct object_entry *oe = oe;\n+\tunsigned char sha1[20];\n+\n+\t/* cat-blob SP <object> LF */\n+\tp = command_buf.buf + strlen(\"cat-blob \");\n+\tif (*p == ':') {\n+\t\tchar *x;\n+\t\toe = find_mark(strtoumax(p + 1, &x, 10));\n+\t\tif (x == p + 1)\n+\t\t\tdie(\"Invalid mark: %s\", command_buf.buf);\n+\t\tif (!oe)\n+\t\t\tdie(\"Unknown mark: %s\", command_buf.buf);\n+\t\tif (*x)\n+\t\t\tdie(\"Garbage after mark: %s\", command_buf.buf);\n+\t\thashcpy(sha1, oe->idx.sha1);\n+\t} else {\n+\t\tif (get_sha1_hex(p, sha1))\n+\t\t\tdie(\"Invalid SHA1: %s\", command_buf.buf);\n+\t\tif (p[40])\n+\t\t\tdie(\"Garbage after SHA1: %s\", command_buf.buf);\n+\t\toe = find_object(sha1);\n+\t}\n+\n+\tcat_blob(oe, sha1);\n+}\n+\n static void parse_checkpoint(void)\n {\n \tif (object_count) {\n@@ -2771,6 +2849,14 @@ static void option_export_marks(const char *marks)\n \texport_marks_file = make_fast_import_path(marks);\n }\n \n+static void option_cat_blob_fd(const char *fd)\n+{\n+\tunsigned long n = ulong_arg(\"--cat-blob-fd\", fd);\n+\tif (n > (unsigned long) INT_MAX)\n+\t\tdie(\"--cat-blob-fd cannot exceed %d\", INT_MAX);\n+\tcat_blob_fd = (int) n;\n+}\n+\n static void option_export_pack_edges(const char *edges)\n {\n \tif (pack_edges)\n@@ -2824,6 +2910,8 @@ static int parse_one_feature(const char *feature, int from_stream)\n \t\toption_import_marks(feature + 13, from_stream);\n \t} else if (!prefixcmp(feature, \"export-marks=\")) {\n \t\toption_export_marks(feature + 13);\n+\t} else if (!strcmp(feature, \"cat-blob\")) {\n+\t\t; /* Don't die - this feature is supported */\n \t} else if (!prefixcmp(feature, \"relative-marks\")) {\n \t\trelative_marks_paths = 1;\n \t} else if (!prefixcmp(feature, \"no-relative-marks\")) {\n@@ -2918,6 +3006,11 @@ static void parse_argv(void)\n \t\tif (parse_one_feature(a + 2, 0))\n \t\t\tcontinue;\n \n+\t\tif (!prefixcmp(a + 2, \"cat-blob-fd=\")) {\n+\t\t\toption_cat_blob_fd(a + 2 + strlen(\"cat-blob-fd=\"));\n+\t\t\tcontinue;\n+\t\t}\n+\n \t\tdie(\"unknown option %s\", a);\n \t}\n \tif (i != global_argc)\n@@ -2969,6 +3062,8 @@ int main(int argc, const char **argv)\n \t\t\tparse_new_tag();\n \t\telse if (!prefixcmp(command_buf.buf, \"reset \"))\n \t\t\tparse_reset_branch();\n+\t\telse if (!prefixcmp(command_buf.buf, \"cat-blob \"))\n+\t\t\tparse_cat_blob();\n \t\telse if (!strcmp(\"checkpoint\", command_buf.buf))\n \t\t\tparse_checkpoint();\n \t\telse if (!prefixcmp(command_buf.buf, \"progress \"))\ndiff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh\nindex 2c27da6..3e2741b 100755\n--- a/t/t9300-fast-import.sh\n+++ b/t/t9300-fast-import.sh\n@@ -23,11 +23,18 @@ file5_data='an inline file.\n file6_data='#!/bin/sh\n echo \"$@\"'\n \n+>empty\n+\n ###\n ### series A\n ###\n \n test_tick\n+\n+test_expect_success 'empty stream succeeds' '\n+\tgit fast-import </dev/null\n+'\n+\n cat >input <<INPUT_END\n blob\n mark :2\n@@ -1501,6 +1508,190 @@ test_expect_success 'R: feature no-relative-marks should be honoured' '\n     test_cmp marks.new non-relative.out\n '\n \n+test_expect_success 'R: feature cat-blob supported' '\n+\techo \"feature cat-blob\" |\n+\tgit fast-import\n+'\n+\n+test_expect_success 'R: cat-blob-fd must be a nonnegative integer' '\n+\ttest_must_fail git fast-import --cat-blob-fd=-1 </dev/null\n+'\n+\n+test_expect_success 'R: print old blob' '\n+\tblob=$(echo \"yes it can\" | git hash-object -w --stdin) &&\n+\tcat >expect <<-EOF &&\n+\t${blob} blob 11\n+\tyes it can\n+\n+\tEOF\n+\techo \"cat-blob $blob\" |\n+\tgit fast-import --cat-blob-fd=6 6>actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'R: in-stream cat-blob-fd not respected' '\n+\techo hello >greeting &&\n+\tblob=$(git hash-object -w greeting) &&\n+\tcat >expect <<-EOF &&\n+\t${blob} blob 6\n+\thello\n+\n+\tEOF\n+\tgit fast-import --cat-blob-fd=3 3>actual.3 >actual.1 <<-EOF &&\n+\tcat-blob $blob\n+\tEOF\n+\ttest_cmp expect actual.3 &&\n+\ttest_cmp empty actual.1 &&\n+\tgit fast-import 3>actual.3 >actual.1 <<-EOF &&\n+\toption cat-blob-fd=3\n+\tcat-blob $blob\n+\tEOF\n+\ttest_cmp empty actual.3 &&\n+\ttest_cmp expect actual.1\n+'\n+\n+test_expect_success 'R: print new blob' '\n+\tblob=$(echo \"yep yep yep\" | git hash-object --stdin) &&\n+\tcat >expect <<-EOF &&\n+\t${blob} blob 12\n+\tyep yep yep\n+\n+\tEOF\n+\tgit fast-import --cat-blob-fd=6 6>actual <<-\\EOF &&\n+\tblob\n+\tmark :1\n+\tdata <<BLOB_END\n+\tyep yep yep\n+\tBLOB_END\n+\tcat-blob :1\n+\tEOF\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'R: print new blob by sha1' '\n+\tblob=$(echo \"a new blob named by sha1\" | git hash-object --stdin) &&\n+\tcat >expect <<-EOF &&\n+\t${blob} blob 25\n+\ta new blob named by sha1\n+\n+\tEOF\n+\tgit fast-import --cat-blob-fd=6 6>actual <<-EOF &&\n+\tblob\n+\tdata <<BLOB_END\n+\ta new blob named by sha1\n+\tBLOB_END\n+\tcat-blob $blob\n+\tEOF\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'setup: big file' '\n+\t(\n+\t\techo \"the quick brown fox jumps over the lazy dog\" >big &&\n+\t\tfor i in 1 2 3\n+\t\tdo\n+\t\t\tcat big big big big >bigger &&\n+\t\t\tcat bigger bigger bigger bigger >big ||\n+\t\t\texit\n+\t\tdone\n+\t)\n+'\n+\n+test_expect_success 'R: print two blobs to stdout' '\n+\tblob1=$(git hash-object big) &&\n+\tblob1_len=$(wc -c <big) &&\n+\tblob2=$(echo hello | git hash-object --stdin) &&\n+\t{\n+\t\techo ${blob1} blob $blob1_len &&\n+\t\tcat big &&\n+\t\tcat <<-EOF\n+\n+\t\t${blob2} blob 6\n+\t\thello\n+\n+\t\tEOF\n+\t} >expect &&\n+\t{\n+\t\tcat <<-\\END_PART1 &&\n+\t\t\tblob\n+\t\t\tmark :1\n+\t\t\tdata <<data_end\n+\t\tEND_PART1\n+\t\tcat big &&\n+\t\tcat <<-\\EOF\n+\t\t\tdata_end\n+\t\t\tblob\n+\t\t\tmark :2\n+\t\t\tdata <<data_end\n+\t\t\thello\n+\t\t\tdata_end\n+\t\t\tcat-blob :1\n+\t\t\tcat-blob :2\n+\t\tEOF\n+\t} |\n+\tgit fast-import >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'setup: have pipes?' '\n+\trm -f frob &&\n+\tif mkfifo frob\n+\tthen\n+\t\ttest_set_prereq PIPE\n+\tfi\n+'\n+\n+test_expect_success PIPE 'R: copy using cat-file' '\n+\texpect_id=$(git hash-object big) &&\n+\texpect_len=$(wc -c <big) &&\n+\techo $expect_id blob $expect_len >expect.response &&\n+\n+\trm -f blobs &&\n+\tcat >frontend <<-\\FRONTEND_END &&\n+\t#!/bin/sh\n+\tcat <<EOF &&\n+\tfeature cat-blob\n+\tblob\n+\tmark :1\n+\tdata <<BLOB\n+\tEOF\n+\tcat big\n+\tcat <<EOF\n+\tBLOB\n+\tcat-blob :1\n+\tEOF\n+\n+\tread blob_id type size <&3 &&\n+\techo \"$blob_id $type $size\" >response &&\n+\tdd if=/dev/stdin of=blob bs=$size count=1 <&3 &&\n+\tread newline <&3 &&\n+\n+\tcat <<EOF &&\n+\tcommit refs/heads/copied\n+\tcommitter $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE\n+\tdata <<COMMIT\n+\tcopy big file as file3\n+\tCOMMIT\n+\tM 644 inline file3\n+\tdata <<BLOB\n+\tEOF\n+\tcat blob &&\n+\tcat <<EOF\n+\tBLOB\n+\tEOF\n+\tFRONTEND_END\n+\n+\tmkfifo blobs &&\n+\t(\n+\t\texport GIT_COMMITTER_NAME GIT_COMMITTER_EMAIL GIT_COMMITTER_DATE &&\n+\t\tsh frontend 3<blobs |\n+\t\tgit fast-import --cat-blob-fd=3 3>blobs\n+\t) &&\n+\tgit show copied:file3 >actual &&\n+\ttest_cmp expect.response response &&\n+\ttest_cmp big actual\n+'\n+\n cat >input << EOF\n option git quiet\n blob\n@@ -1509,8 +1700,6 @@ hi\n \n EOF\n \n-touch empty\n-\n test_expect_success 'R: quiet option results in no stats being output' '\n     cat input | git fast-import 2> output &&\n     test_cmp empty output\n-- \n1.7.2.3\n"},{"id":"156158","messageId":"20101119095112.GE19061@burratino","threadId":"25454","inReplyTo":"20101119093530.GA19061@burratino","subject":"[PATCH 4/4] fast-import: Allow cat-blob requests at arbitrary points in stream","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-11-19T09:51:13Z","receivedAt":"2010-11-19T09:51:13Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"The new rule: a \"cat-blob\" can be inserted wherever a comment is\nallowed, which means at the start of any line except in the middle of\na \"data\" command.\n\nThis saves frontends from having to loop over everything they want to\ncommit in the next commit and cat-ing the necessary objects in\nadvance.\n\nSigned-off-by: Jonathan Nieder <jrnieder@gmail.com>\nSigned-off-by: David Barr <david.barr@cordelta.com>\nSigned-off-by: Jonathan Nieder <jrnieder@gmail.com>\n---\nThat's the end of the series.  Thanks for reading.\n\nThe early history is at [1] if that's your kind of thing.\n\n[1] http://thread.gmane.org/gmane.comp.version-control.git/150005/focus=155417\n\n Documentation/git-fast-import.txt |    4 ++\n fast-import.c                     |   28 +++++++++-------\n t/t9300-fast-import.sh            |   66 +++++++++++++++++++++++++++++++++++++\n 3 files changed, 86 insertions(+), 12 deletions(-)\n\ndiff --git a/Documentation/git-fast-import.txt b/Documentation/git-fast-import.txt\nindex be444da..d569564 100644\n--- a/Documentation/git-fast-import.txt\n+++ b/Documentation/git-fast-import.txt\n@@ -905,6 +905,10 @@ output uses the same format as `git cat-file --batch`:\n \t<contents> LF\n ====\n \n+This command can be used anywhere in the stream that comments are\n+accepted.  In particular, the `cat-blob` command can be used in the\n+middle of a commit but not in the middle of a `data` command.\n+\n `feature`\n ~~~~~~~~~\n Require that fast-import supports the specified feature, or abort if\ndiff --git a/fast-import.c b/fast-import.c\nindex 88547c6..33c6981 100644\n--- a/fast-import.c\n+++ b/fast-import.c\n@@ -55,8 +55,6 @@ Format of STDIN stream:\n     ('from' sp committish lf)?\n     lf?;\n \n-  cat_blob ::= 'cat-blob' sp (hexsha1 | idnum) lf;\n-\n   checkpoint ::= 'checkpoint' lf\n     lf?;\n \n@@ -134,14 +132,17 @@ Format of STDIN stream:\n   ts    ::= # time since the epoch in seconds, ascii base10 notation;\n   tz    ::= # GIT style timezone;\n \n-     # note: comments may appear anywhere in the input, except\n-     # within a data command.  Any form of the data command\n-     # always escapes the related input from comment processing.\n+     # note: comments and cat requests may appear anywhere\n+     # in the input, except within a data command.  Any form\n+     # of the data command always escapes the related input\n+     # from comment processing.\n      #\n      # In case it is not clear, the '#' that starts the comment\n      # must be the first character on that line (an lf\n      # preceded it).\n      #\n+  cat_blob ::= 'cat-blob' sp (hexsha1 | idnum) lf;\n+\n   comment ::= '#' not_lf* lf;\n   not_lf  ::= # Any byte that is not ASCII newline (LF);\n */\n@@ -367,6 +368,7 @@ static int seen_data_command;\n static int cat_blob_fd = STDOUT_FILENO;\n \n static void parse_argv(void);\n+static void parse_cat_blob(void);\n \n static void write_branch_report(FILE *rpt, struct branch *b)\n {\n@@ -1791,7 +1793,6 @@ static void read_marks(void)\n \tfclose(f);\n }\n \n-\n static int read_next_command(void)\n {\n \tstatic int stdin_eof = 0;\n@@ -1801,7 +1802,7 @@ static int read_next_command(void)\n \t\treturn EOF;\n \t}\n \n-\tdo {\n+\tfor (;;) {\n \t\tif (unread_command_buf) {\n \t\t\tunread_command_buf = 0;\n \t\t} else {\n@@ -1834,9 +1835,14 @@ static int read_next_command(void)\n \t\t\trc->prev->next = rc;\n \t\t\tcmd_tail = rc;\n \t\t}\n-\t} while (command_buf.buf[0] == '#');\n-\n-\treturn 0;\n+\t\tif (!prefixcmp(command_buf.buf, \"cat-blob \")) {\n+\t\t\tparse_cat_blob();\n+\t\t\tcontinue;\n+\t\t}\n+\t\tif (command_buf.buf[0] == '#')\n+\t\t\tcontinue;\n+\t\treturn 0;\n+\t}\n }\n \n static void skip_optional_lf(void)\n@@ -3062,8 +3068,6 @@ int main(int argc, const char **argv)\n \t\t\tparse_new_tag();\n \t\telse if (!prefixcmp(command_buf.buf, \"reset \"))\n \t\t\tparse_reset_branch();\n-\t\telse if (!prefixcmp(command_buf.buf, \"cat-blob \"))\n-\t\t\tparse_cat_blob();\n \t\telse if (!strcmp(\"checkpoint\", command_buf.buf))\n \t\t\tparse_checkpoint();\n \t\telse if (!prefixcmp(command_buf.buf, \"progress \"))\ndiff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh\nindex 3e2741b..2a050c7 100755\n--- a/t/t9300-fast-import.sh\n+++ b/t/t9300-fast-import.sh\n@@ -1692,6 +1692,72 @@ test_expect_success PIPE 'R: copy using cat-file' '\n \ttest_cmp big actual\n '\n \n+test_expect_success PIPE 'R: print blob mid-commit' '\n+\trm -f blobs &&\n+\techo \"A blob from _before_ the commit.\" >expect &&\n+\tmkfifo blobs &&\n+\t(\n+\t\texec 3<blobs &&\n+\t\tcat <<-EOF &&\n+\t\tfeature cat-blob\n+\t\tblob\n+\t\tmark :1\n+\t\tdata <<BLOB\n+\t\tA blob from _before_ the commit.\n+\t\tBLOB\n+\t\tcommit refs/heads/temporary\n+\t\tcommitter $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE\n+\t\tdata <<COMMIT\n+\t\tEmpty commit\n+\t\tCOMMIT\n+\t\tcat-blob :1\n+\t\tEOF\n+\n+\t\tread blob_id type size <&3 &&\n+\t\tdd if=/dev/stdin of=actual bs=$size count=1 <&3 &&\n+\t\tread newline <&3 &&\n+\n+\t\techo\n+\t) |\n+\tgit fast-import --cat-blob-fd=3 3>blobs &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success PIPE 'R: print staged blob within commit' '\n+\trm -f blobs &&\n+\techo \"A blob from _within_ the commit.\" >expect &&\n+\tmkfifo blobs &&\n+\t(\n+\t\texec 3<blobs &&\n+\t\tcat <<-EOF &&\n+\t\tfeature cat-blob\n+\t\tcommit refs/heads/within\n+\t\tcommitter $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE\n+\t\tdata <<COMMIT\n+\t\tEmpty commit\n+\t\tCOMMIT\n+\t\tM 644 inline within\n+\t\tdata <<BLOB\n+\t\tA blob from _within_ the commit.\n+\t\tBLOB\n+\t\tEOF\n+\n+\t\tto_get=$(\n+\t\t\techo \"A blob from _within_ the commit.\" |\n+\t\t\tgit hash-object --stdin\n+\t\t) &&\n+\t\techo \"cat-blob $to_get\" &&\n+\n+\t\tread blob_id type size <&3 &&\n+\t\tdd if=/dev/stdin of=actual bs=$size count=1 <&3 &&\n+\t\tread newline <&3 &&\n+\n+\t\techo deleteall\n+\t) |\n+\tgit fast-import --cat-blob-fd=3 3>blobs &&\n+\ttest_cmp expect actual\n+'\n+\n cat >input << EOF\n option git quiet\n blob\n-- \n1.7.2.3\n"},{"id":"156167","messageId":"AANLkTinverp9axgvtWtb4SwTAYJTkWnN1ejd5Ce3symm@mail.gmail.com","threadId":"25454","inReplyTo":"20101119094045.GC19061@burratino","subject":"Re: [PATCH 2/4] fast-import: clarify documentation of \"feature\" command","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2010-11-19T11:58:49Z","receivedAt":"2010-11-19T11:58:49Z","isPatch":true,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"Heya,\n\nOn Fri, Nov 19, 2010 at 10:40, Jonathan Nieder <jrnieder@gmail.com> wrote:\n> Make this more obvious by being more explicit about how the analogy\n> between most \"feature\" commands and command-line options works.  Treat\n> the feature (import-marks) that does not fit this analogy separately.\n\nAcked-by: Sverre Rabbelier <srabbelier@gmail.com>\n\n> In particular, it is not obvious to me whether cat-blob, ls-tree,\n> and so on ought to be considered a single feature but with the\n> feature command syntax, we could dodge the issue. :)  Sane?\n\nYes, I like that idea, although I'm not sure how clean the\nimplementation would be :)\n\n-- \nCheers,\n\nSverre Rabbelier\n"},{"id":"156768","messageId":"20101128194131.GA19998@burratino","threadId":"25454","inReplyTo":"1287147256-9457-2-git-send-email-david.barr@cordelta.com","subject":"[PATCH/RFC v3 resend 0/4] fast-import: Let importers retrieve blobs","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-11-28T19:41:31Z","receivedAt":"2010-11-28T19:41:31Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"[resending since git@vger doesn't seem to have accepted the previous\ncopy.  Sorry for the noise.]\n\nDavid Barr wrote:\n\n> So introduce another way: a \"cat-blob\" command introduced in the\n> command stream requests for fast-import to print a blob to stdout\n> or a file descriptor specified by the argument --cat-blob-fd.\n> \n> The output uses the same format as \"git cat-file --batch\".\n\nI am very fond of this patch.  Still, the fact remains that until this\ncommand is implemented by some other fast-import backend, it is hard\nto know what git-specific concepts are encoded in its current\nimplementation.  (I am in particular worried about what should be\ngauranteed about the blob_identifier in the\n\n\tblob blob_identifier 1823\n\nline and whether\n\n\tcat-blob blob_name\n\tcat-blob :11\n\nare going to overlap and cause trouble for some backends.)\n\nOn the other hand, development of svn-fe continues to benefit from\ncat-blob and its cousins ls-tree and ls[1].  and there has been brief\ndiscussion of using cat-blob to make cvs2git more friendly.  So here's\na reroll, since it seems clear that this feature should be in future\nversions of fast-import in some form.\n\nThoughts welcome, as always (even as simple as \"I have a bad feeling\nabout this\" or \"everything in this patch looks ready to go\").\n\nPatches based against v1.7.0.7 for no particular reason.\n\nDavid Barr (1):\n  fast-import: let importers retrieve blobs\n\nJonathan Nieder (3):\n  fast-import: stricter parsing of integer options\n  fast-import: clarify documentation of \"feature\" command\n  fast-import: Allow cat-blob requests at arbitrary points in stream\n\n Documentation/git-fast-import.txt |   76 ++++++++---\n fast-import.c                     |  128 ++++++++++++++++--\n t/t9300-fast-import.sh            |  267 ++++++++++++++++++++++++++++++++++++-\n 3 files changed, 442 insertions(+), 29 deletions(-)\n\n[1] ls-tree commit_name \"path/to/file\" would have output format\n100644 blob blob_identifier\tpath/to/file and within a commit\ncommand, ls \"path/to/file\" would produce output in the same\nformat.\n"},{"id":"156769","messageId":"20101128194246.GB19998@burratino","threadId":"25454","inReplyTo":"20101128194131.GA19998@burratino","subject":"[PATCH 1/4] fast-import: stricter parsing of integer options","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-11-28T19:42:46Z","receivedAt":"2010-11-28T19:42:46Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Check the result from strtoul to avoid accepting arguments like\n--depth=-1 and --active-branches=foo,bar,baz.\n\nRequested-by: Ramkumar Ramachandra <artagnon@gmail.com>\nSigned-off-by: Jonathan Nieder <jrnieder@gmail.com>\n---\nSee http://thread.gmane.org/gmane.comp.version-control.git/159117/focus=159236\nfor context.\n\n fast-import.c          |   13 +++++++++++--\n t/t9300-fast-import.sh |    8 ++++++++\n 2 files changed, 19 insertions(+), 2 deletions(-)\n\ndiff --git a/fast-import.c b/fast-import.c\nindex 74f08bd..959afef 100644\n--- a/fast-import.c\n+++ b/fast-import.c\n@@ -2745,16 +2745,25 @@ static void option_date_format(const char *fmt)\n \t\tdie(\"unknown --date-format argument %s\", fmt);\n }\n \n+static unsigned long ulong_arg(const char *option, const char *arg)\n+{\n+\tchar *endptr;\n+\tunsigned long rv = strtoul(arg, &endptr, 0);\n+\tif (strchr(arg, '-') || endptr == arg || *endptr)\n+\t\tdie(\"%s: argument must be an unsigned integer\", option);\n+\treturn rv;\n+}\n+\n static void option_depth(const char *depth)\n {\n-\tmax_depth = strtoul(depth, NULL, 0);\n+\tmax_depth = ulong_arg(\"--depth\", depth);\n \tif (max_depth > MAX_DEPTH)\n \t\tdie(\"--depth cannot exceed %u\", MAX_DEPTH);\n }\n \n static void option_active_branches(const char *branches)\n {\n-\tmax_active_branches = strtoul(branches, NULL, 0);\n+\tmax_active_branches = ulong_arg(\"--active-branches\", branches);\n }\n \n static void option_export_marks(const char *marks)\ndiff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh\nindex 131f032..2c27da6 100755\n--- a/t/t9300-fast-import.sh\n+++ b/t/t9300-fast-import.sh\n@@ -1528,6 +1528,14 @@ test_expect_success 'R: unknown commandline options are rejected' '\\\n     test_must_fail git fast-import --non-existing-option < /dev/null\n '\n \n+test_expect_success 'R: die on invalid option argument' '\n+\techo \"option git active-branches=-5\" |\n+\ttest_must_fail git fast-import &&\n+\techo \"option git depth=\" |\n+\ttest_must_fail git fast-import &&\n+\ttest_must_fail git fast-import --depth=\"5 elephants\" </dev/null\n+'\n+\n cat >input <<EOF\n option non-existing-vcs non-existing-option\n EOF\n"},{"id":"156770","messageId":"20101128194357.GC19998@burratino","threadId":"25454","inReplyTo":"20101128194131.GA19998@burratino","subject":"[PATCH 2/4] fast-import: clarify documentation of \"feature\" command","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-11-28T19:43:57Z","receivedAt":"2010-11-28T19:43:57Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"The \"feature\" command allows streams to specify options for the import\nthat must not be ignored.  Logically, they are part of the stream,\neven though technically most supported features are synonyms to\ncommand-line options.\n\nMake this more obvious by being more explicit about how the analogy\nbetween most \"feature\" commands and command-line options works.  Treat\nthe feature (import-marks) that does not fit this analogy separately.\n\nSigned-off-by: Jonathan Nieder <jrnieder@gmail.com>\nAcked-by: Sverre Rabbelier <srabbelier@gmail.com>\n---\nSide note: I am thinking of introducing a syntax\n\n\t'feature' SP 'command' SP <command name> LF\n\nwhich would just check if <command name> is a recognized command.\nThis way, when a feature introduces a new command, it would get\na feature name to go along with that with no extra effort.\n\nIn particular, it is not obvious to me whether cat-blob, ls-tree,\nand so on ought to be considered a single feature but with the\nfeature command syntax, we could dodge the issue. :)  Sane?\n\n Documentation/git-fast-import.txt |   33 +++++++++++++++------------------\n 1 files changed, 15 insertions(+), 18 deletions(-)\n\ndiff --git a/Documentation/git-fast-import.txt b/Documentation/git-fast-import.txt\nindex 19082b0..3bf04e3 100644\n--- a/Documentation/git-fast-import.txt\n+++ b/Documentation/git-fast-import.txt\n@@ -878,28 +878,25 @@ Require that fast-import supports the specified feature, or abort if\n it does not.\n \n ....\n-\t'feature' SP <feature> LF\n+\t'feature' SP <feature> ('=' <argument>)? LF\n ....\n \n-The <feature> part of the command may be any string matching\n-^[a-zA-Z][a-zA-Z-]*$ and should be understood by fast-import.\n+The <feature> part of the command may be any one of the following:\n \n-Feature work identical as their option counterparts with the\n-exception of the import-marks feature, see below.\n+date-format::\n+export-marks::\n+relative-marks::\n+no-relative-marks::\n+force::\n+\tAct as though the corresponding command-line option with\n+\ta leading '--' was passed on the command line\n+\t(see OPTIONS, above).\n \n-The following features are currently supported:\n-\n-* date-format\n-* import-marks\n-* export-marks\n-* relative-marks\n-* no-relative-marks\n-* force\n-\n-The import-marks behaves differently from when it is specified as\n-commandline option in that only one \"feature import-marks\" is allowed\n-per stream. Also, any --import-marks= specified on the commandline\n-will override those from the stream (if any).\n+import-marks::\n+\tLike --import-marks except in two respects: first, only one\n+\t\"feature import-marks\" command is allowed per stream;\n+\tsecond, an --import-marks= command-line option overrides\n+\tany \"feature import-marks\" command in the stream.\n \n `option`\n ~~~~~~~~\n"},{"id":"156771","messageId":"20101128194501.GD19998@burratino","threadId":"25454","inReplyTo":"20101128194131.GA19998@burratino","subject":"[PATCH 3/4] fast-import: let importers retrieve blobs","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-11-28T19:45:01Z","receivedAt":"2010-11-28T19:45:01Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"From: David Barr <david.barr@cordelta.com>\n\nNew objects written by fast-import are not available immediately.\nUntil a checkpoint has been started and finishes writing the pack\nindex, any new blobs will not be accessible using standard git tools.\n\nSo introduce a new way to access them: a \"cat-blob\" command in the\ncommand stream requests for fast-import to print a blob to stdout or a\nfile descriptor specified by the argument to --cat-blob-fd.  The value\nfor cat-blob-fd cannot be specified in the stream because that would\nbe a layering violation: the decision of where to direct a stream has\nto be made when fast-import is started anyway, so we might as well\nmake the stream format is independent of that detail.\n\nOutput uses the same format as \"git cat-file --batch\".\n\nThanks to Sverre Rabbelier and Sam Vilain for guidance in designing\nthe protocol.\n\nBased-on-patch-by: Jonathan Nieder <jrnieder@gmail.com>\nSigned-off-by: David Barr <david.barr@cordelta.com>\nAcked-by: Ramkumar Ramachandra <artagnon@gmail.com>\nSigned-off-by: Jonathan Nieder <jrnieder@gmail.com>\n---\nIf you use this, you might want to do something like:\n\n\tblob \n\tmark :1\n\tdata <<EOT\n\ttesting 1 2 3\n\tEOT\n\tcat-blob :1\n\nbefore proceeding with the rest of the stream.  This allows wiring\nmistakes to be caught early.\n\n Documentation/git-fast-import.txt |   41 ++++++++\n fast-import.c                     |   95 ++++++++++++++++++\n t/t9300-fast-import.sh            |  193 ++++++++++++++++++++++++++++++++++++-\n 3 files changed, 327 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/git-fast-import.txt b/Documentation/git-fast-import.txt\nindex 3bf04e3..be444da 100644\n--- a/Documentation/git-fast-import.txt\n+++ b/Documentation/git-fast-import.txt\n@@ -92,6 +92,11 @@ OPTIONS\n \t--(no-)-relative-marks= with the --(import|export)-marks=\n \toptions.\n \n+--cat-blob-fd=<fd>::\n+\tSpecify the file descriptor that will be written to\n+\twhen the `cat-blob` command is encountered in the stream.\n+\tThe default behaviour is to write to `stdout`.\n+\n --export-pack-edges=<file>::\n \tAfter creating a packfile, print a line of data to\n \t<file> listing the filename of the packfile and the last\n@@ -320,6 +325,11 @@ and control the current import process.  More detailed discussion\n \tstandard output.  This command is optional and is not needed\n \tto perform an import.\n \n+`cat-blob`::\n+\tCauses fast-import to print a blob in 'cat-file --batch'\n+\tformat to the file descriptor set with `--cat-blob-fd` or\n+\t`stdout` if unspecified.\n+\n `feature`::\n \tRequire that fast-import supports the specified feature, or\n \tabort if it does not.\n@@ -872,6 +882,29 @@ Placing a `progress` command immediately after a `checkpoint` will\n inform the reader when the `checkpoint` has been completed and it\n can safely access the refs that fast-import updated.\n \n+`cat-blob`\n+~~~~~~~~~~\n+Causes fast-import to print a blob to a file descriptor previously\n+arranged with the `--cat-blob-fd` argument.  The command otherwise\n+has no impact on the current import; its main purpose is to\n+retrieve blobs that may be in fast-import's memory but not\n+accessible from the target repository.\n+\n+....\n+\t'cat-blob' SP <dataref> LF\n+....\n+\n+The `<dataref>` can be either a mark reference (`:<idnum>`)\n+set previously or a full 40-byte SHA-1 of a Git blob, preexisting or\n+ready to be written.\n+\n+output uses the same format as `git cat-file --batch`:\n+\n+====\n+\t<sha1> SP 'blob' SP <size> LF\n+\t<contents> LF\n+====\n+\n `feature`\n ~~~~~~~~~\n Require that fast-import supports the specified feature, or abort if\n@@ -898,6 +931,13 @@ import-marks::\n \tsecond, an --import-marks= command-line option overrides\n \tany \"feature import-marks\" command in the stream.\n \n+cat-blob::\n+\tIgnored.  Versions of fast-import not supporting the\n+\t\"cat-blob\" command will exit with a message indicating so.\n+\tThis lets the import error out early with a clear message,\n+\trather than wasting time on the early part of an import\n+\tbefore the unsupported command is detected.\n+\n `option`\n ~~~~~~~~\n Processes the specified option so that git fast-import behaves in a\n@@ -923,6 +963,7 @@ not be passed as option:\n * date-format\n * import-marks\n * export-marks\n+* cat-blob-fd\n * force\n \n Crash Reports\ndiff --git a/fast-import.c b/fast-import.c\nindex 959afef..88547c6 100644\n--- a/fast-import.c\n+++ b/fast-import.c\n@@ -55,6 +55,8 @@ Format of STDIN stream:\n     ('from' sp committish lf)?\n     lf?;\n \n+  cat_blob ::= 'cat-blob' sp (hexsha1 | idnum) lf;\n+\n   checkpoint ::= 'checkpoint' lf\n     lf?;\n \n@@ -361,6 +363,9 @@ static uintmax_t next_mark;\n static struct strbuf new_data = STRBUF_INIT;\n static int seen_data_command;\n \n+/* Where to write output of cat-blob commands */\n+static int cat_blob_fd = STDOUT_FILENO;\n+\n static void parse_argv(void);\n \n static void write_branch_report(FILE *rpt, struct branch *b)\n@@ -2689,6 +2694,79 @@ static void parse_reset_branch(void)\n \t\tunread_command_buf = 1;\n }\n \n+static void cat_blob_write(const char *buf, unsigned long size)\n+{\n+\tif (write_in_full(cat_blob_fd, buf, size) != size)\n+\t\tdie_errno(\"Write to frontend failed\");\n+}\n+\n+static void cat_blob(struct object_entry *oe, unsigned char sha1[20])\n+{\n+\tstruct strbuf line = STRBUF_INIT;\n+\tunsigned long size;\n+\tenum object_type type = 0;\n+\tchar *buf;\n+\n+\tif (!oe || oe->pack_id == MAX_PACK_ID) {\n+\t\tbuf = read_sha1_file(sha1, &type, &size);\n+\t} else {\n+\t\ttype = oe->type;\n+\t\tbuf = gfi_unpack_entry(oe, &size);\n+\t}\n+\n+\t/*\n+\t * Output based on batch_one_object() from cat-file.c.\n+\t */\n+\tif (type <= 0) {\n+\t\tstrbuf_reset(&line);\n+\t\tstrbuf_addf(&line, \"%s missing\\n\", sha1_to_hex(sha1));\n+\t\tcat_blob_write(line.buf, line.len);\n+\t\tfree(buf);\n+\t\treturn;\n+\t}\n+\tif (!buf)\n+\t\tdie(\"Can't read object %s\", sha1_to_hex(sha1));\n+\tif (type != OBJ_BLOB)\n+\t\tdie(\"Object %s is a %s but a blob was expected.\",\n+\t\t    sha1_to_hex(sha1), typename(type));\n+\tstrbuf_reset(&line);\n+\tstrbuf_addf(&line, \"%s %s %lu\\n\", sha1_to_hex(sha1),\n+\t\t\t\t\t\ttypename(type), size);\n+\tcat_blob_write(line.buf, line.len);\n+\tcat_blob_write(buf, size);\n+\tcat_blob_write(\"\\n\", 1);\n+\tfree(buf);\n+}\n+\n+static void parse_cat_blob(void)\n+{\n+\tconst char *p;\n+\tstruct object_entry *oe = oe;\n+\tunsigned char sha1[20];\n+\n+\t/* cat-blob SP <object> LF */\n+\tp = command_buf.buf + strlen(\"cat-blob \");\n+\tif (*p == ':') {\n+\t\tchar *x;\n+\t\toe = find_mark(strtoumax(p + 1, &x, 10));\n+\t\tif (x == p + 1)\n+\t\t\tdie(\"Invalid mark: %s\", command_buf.buf);\n+\t\tif (!oe)\n+\t\t\tdie(\"Unknown mark: %s\", command_buf.buf);\n+\t\tif (*x)\n+\t\t\tdie(\"Garbage after mark: %s\", command_buf.buf);\n+\t\thashcpy(sha1, oe->idx.sha1);\n+\t} else {\n+\t\tif (get_sha1_hex(p, sha1))\n+\t\t\tdie(\"Invalid SHA1: %s\", command_buf.buf);\n+\t\tif (p[40])\n+\t\t\tdie(\"Garbage after SHA1: %s\", command_buf.buf);\n+\t\toe = find_object(sha1);\n+\t}\n+\n+\tcat_blob(oe, sha1);\n+}\n+\n static void parse_checkpoint(void)\n {\n \tif (object_count) {\n@@ -2771,6 +2849,14 @@ static void option_export_marks(const char *marks)\n \texport_marks_file = make_fast_import_path(marks);\n }\n \n+static void option_cat_blob_fd(const char *fd)\n+{\n+\tunsigned long n = ulong_arg(\"--cat-blob-fd\", fd);\n+\tif (n > (unsigned long) INT_MAX)\n+\t\tdie(\"--cat-blob-fd cannot exceed %d\", INT_MAX);\n+\tcat_blob_fd = (int) n;\n+}\n+\n static void option_export_pack_edges(const char *edges)\n {\n \tif (pack_edges)\n@@ -2824,6 +2910,8 @@ static int parse_one_feature(const char *feature, int from_stream)\n \t\toption_import_marks(feature + 13, from_stream);\n \t} else if (!prefixcmp(feature, \"export-marks=\")) {\n \t\toption_export_marks(feature + 13);\n+\t} else if (!strcmp(feature, \"cat-blob\")) {\n+\t\t; /* Don't die - this feature is supported */\n \t} else if (!prefixcmp(feature, \"relative-marks\")) {\n \t\trelative_marks_paths = 1;\n \t} else if (!prefixcmp(feature, \"no-relative-marks\")) {\n@@ -2918,6 +3006,11 @@ static void parse_argv(void)\n \t\tif (parse_one_feature(a + 2, 0))\n \t\t\tcontinue;\n \n+\t\tif (!prefixcmp(a + 2, \"cat-blob-fd=\")) {\n+\t\t\toption_cat_blob_fd(a + 2 + strlen(\"cat-blob-fd=\"));\n+\t\t\tcontinue;\n+\t\t}\n+\n \t\tdie(\"unknown option %s\", a);\n \t}\n \tif (i != global_argc)\n@@ -2969,6 +3062,8 @@ int main(int argc, const char **argv)\n \t\t\tparse_new_tag();\n \t\telse if (!prefixcmp(command_buf.buf, \"reset \"))\n \t\t\tparse_reset_branch();\n+\t\telse if (!prefixcmp(command_buf.buf, \"cat-blob \"))\n+\t\t\tparse_cat_blob();\n \t\telse if (!strcmp(\"checkpoint\", command_buf.buf))\n \t\t\tparse_checkpoint();\n \t\telse if (!prefixcmp(command_buf.buf, \"progress \"))\ndiff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh\nindex 2c27da6..3e2741b 100755\n--- a/t/t9300-fast-import.sh\n+++ b/t/t9300-fast-import.sh\n@@ -23,11 +23,18 @@ file5_data='an inline file.\n file6_data='#!/bin/sh\n echo \"$@\"'\n \n+>empty\n+\n ###\n ### series A\n ###\n \n test_tick\n+\n+test_expect_success 'empty stream succeeds' '\n+\tgit fast-import </dev/null\n+'\n+\n cat >input <<INPUT_END\n blob\n mark :2\n@@ -1501,6 +1508,190 @@ test_expect_success 'R: feature no-relative-marks should be honoured' '\n     test_cmp marks.new non-relative.out\n '\n \n+test_expect_success 'R: feature cat-blob supported' '\n+\techo \"feature cat-blob\" |\n+\tgit fast-import\n+'\n+\n+test_expect_success 'R: cat-blob-fd must be a nonnegative integer' '\n+\ttest_must_fail git fast-import --cat-blob-fd=-1 </dev/null\n+'\n+\n+test_expect_success 'R: print old blob' '\n+\tblob=$(echo \"yes it can\" | git hash-object -w --stdin) &&\n+\tcat >expect <<-EOF &&\n+\t${blob} blob 11\n+\tyes it can\n+\n+\tEOF\n+\techo \"cat-blob $blob\" |\n+\tgit fast-import --cat-blob-fd=6 6>actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'R: in-stream cat-blob-fd not respected' '\n+\techo hello >greeting &&\n+\tblob=$(git hash-object -w greeting) &&\n+\tcat >expect <<-EOF &&\n+\t${blob} blob 6\n+\thello\n+\n+\tEOF\n+\tgit fast-import --cat-blob-fd=3 3>actual.3 >actual.1 <<-EOF &&\n+\tcat-blob $blob\n+\tEOF\n+\ttest_cmp expect actual.3 &&\n+\ttest_cmp empty actual.1 &&\n+\tgit fast-import 3>actual.3 >actual.1 <<-EOF &&\n+\toption cat-blob-fd=3\n+\tcat-blob $blob\n+\tEOF\n+\ttest_cmp empty actual.3 &&\n+\ttest_cmp expect actual.1\n+'\n+\n+test_expect_success 'R: print new blob' '\n+\tblob=$(echo \"yep yep yep\" | git hash-object --stdin) &&\n+\tcat >expect <<-EOF &&\n+\t${blob} blob 12\n+\tyep yep yep\n+\n+\tEOF\n+\tgit fast-import --cat-blob-fd=6 6>actual <<-\\EOF &&\n+\tblob\n+\tmark :1\n+\tdata <<BLOB_END\n+\tyep yep yep\n+\tBLOB_END\n+\tcat-blob :1\n+\tEOF\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'R: print new blob by sha1' '\n+\tblob=$(echo \"a new blob named by sha1\" | git hash-object --stdin) &&\n+\tcat >expect <<-EOF &&\n+\t${blob} blob 25\n+\ta new blob named by sha1\n+\n+\tEOF\n+\tgit fast-import --cat-blob-fd=6 6>actual <<-EOF &&\n+\tblob\n+\tdata <<BLOB_END\n+\ta new blob named by sha1\n+\tBLOB_END\n+\tcat-blob $blob\n+\tEOF\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'setup: big file' '\n+\t(\n+\t\techo \"the quick brown fox jumps over the lazy dog\" >big &&\n+\t\tfor i in 1 2 3\n+\t\tdo\n+\t\t\tcat big big big big >bigger &&\n+\t\t\tcat bigger bigger bigger bigger >big ||\n+\t\t\texit\n+\t\tdone\n+\t)\n+'\n+\n+test_expect_success 'R: print two blobs to stdout' '\n+\tblob1=$(git hash-object big) &&\n+\tblob1_len=$(wc -c <big) &&\n+\tblob2=$(echo hello | git hash-object --stdin) &&\n+\t{\n+\t\techo ${blob1} blob $blob1_len &&\n+\t\tcat big &&\n+\t\tcat <<-EOF\n+\n+\t\t${blob2} blob 6\n+\t\thello\n+\n+\t\tEOF\n+\t} >expect &&\n+\t{\n+\t\tcat <<-\\END_PART1 &&\n+\t\t\tblob\n+\t\t\tmark :1\n+\t\t\tdata <<data_end\n+\t\tEND_PART1\n+\t\tcat big &&\n+\t\tcat <<-\\EOF\n+\t\t\tdata_end\n+\t\t\tblob\n+\t\t\tmark :2\n+\t\t\tdata <<data_end\n+\t\t\thello\n+\t\t\tdata_end\n+\t\t\tcat-blob :1\n+\t\t\tcat-blob :2\n+\t\tEOF\n+\t} |\n+\tgit fast-import >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'setup: have pipes?' '\n+\trm -f frob &&\n+\tif mkfifo frob\n+\tthen\n+\t\ttest_set_prereq PIPE\n+\tfi\n+'\n+\n+test_expect_success PIPE 'R: copy using cat-file' '\n+\texpect_id=$(git hash-object big) &&\n+\texpect_len=$(wc -c <big) &&\n+\techo $expect_id blob $expect_len >expect.response &&\n+\n+\trm -f blobs &&\n+\tcat >frontend <<-\\FRONTEND_END &&\n+\t#!/bin/sh\n+\tcat <<EOF &&\n+\tfeature cat-blob\n+\tblob\n+\tmark :1\n+\tdata <<BLOB\n+\tEOF\n+\tcat big\n+\tcat <<EOF\n+\tBLOB\n+\tcat-blob :1\n+\tEOF\n+\n+\tread blob_id type size <&3 &&\n+\techo \"$blob_id $type $size\" >response &&\n+\tdd if=/dev/stdin of=blob bs=$size count=1 <&3 &&\n+\tread newline <&3 &&\n+\n+\tcat <<EOF &&\n+\tcommit refs/heads/copied\n+\tcommitter $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE\n+\tdata <<COMMIT\n+\tcopy big file as file3\n+\tCOMMIT\n+\tM 644 inline file3\n+\tdata <<BLOB\n+\tEOF\n+\tcat blob &&\n+\tcat <<EOF\n+\tBLOB\n+\tEOF\n+\tFRONTEND_END\n+\n+\tmkfifo blobs &&\n+\t(\n+\t\texport GIT_COMMITTER_NAME GIT_COMMITTER_EMAIL GIT_COMMITTER_DATE &&\n+\t\tsh frontend 3<blobs |\n+\t\tgit fast-import --cat-blob-fd=3 3>blobs\n+\t) &&\n+\tgit show copied:file3 >actual &&\n+\ttest_cmp expect.response response &&\n+\ttest_cmp big actual\n+'\n+\n cat >input << EOF\n option git quiet\n blob\n@@ -1509,8 +1700,6 @@ hi\n \n EOF\n \n-touch empty\n-\n test_expect_success 'R: quiet option results in no stats being output' '\n     cat input | git fast-import 2> output &&\n     test_cmp empty output\n"},{"id":"156772","messageId":"20101128194558.GE19998@burratino","threadId":"25454","inReplyTo":"20101128194131.GA19998@burratino","subject":"[PATCH 4/4] fast-import: Allow cat-blob requests at arbitrary points in stream","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-11-28T19:45:58Z","receivedAt":"2010-11-28T19:45:58Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"The new rule: a \"cat-blob\" can be inserted wherever a comment is\nallowed, which means at the start of any line except in the middle of\na \"data\" command.\n\nThis saves frontends from having to loop over everything they want to\ncommit in the next commit and cat-ing the necessary objects in\nadvance.\n\nSigned-off-by: Jonathan Nieder <jrnieder@gmail.com>\nSigned-off-by: David Barr <david.barr@cordelta.com>\nSigned-off-by: Jonathan Nieder <jrnieder@gmail.com>\n---\nThat's the end of the series.  Thanks for reading.\n\nThe early history is at [1] if that's your kind of thing.\n\n[1] http://thread.gmane.org/gmane.comp.version-control.git/150005/focus=155417\n\n Documentation/git-fast-import.txt |    4 ++\n fast-import.c                     |   28 +++++++++-------\n t/t9300-fast-import.sh            |   66 +++++++++++++++++++++++++++++++++++++\n 3 files changed, 86 insertions(+), 12 deletions(-)\n\ndiff --git a/Documentation/git-fast-import.txt b/Documentation/git-fast-import.txt\nindex be444da..d569564 100644\n--- a/Documentation/git-fast-import.txt\n+++ b/Documentation/git-fast-import.txt\n@@ -905,6 +905,10 @@ output uses the same format as `git cat-file --batch`:\n \t<contents> LF\n ====\n \n+This command can be used anywhere in the stream that comments are\n+accepted.  In particular, the `cat-blob` command can be used in the\n+middle of a commit but not in the middle of a `data` command.\n+\n `feature`\n ~~~~~~~~~\n Require that fast-import supports the specified feature, or abort if\ndiff --git a/fast-import.c b/fast-import.c\nindex 88547c6..33c6981 100644\n--- a/fast-import.c\n+++ b/fast-import.c\n@@ -55,8 +55,6 @@ Format of STDIN stream:\n     ('from' sp committish lf)?\n     lf?;\n \n-  cat_blob ::= 'cat-blob' sp (hexsha1 | idnum) lf;\n-\n   checkpoint ::= 'checkpoint' lf\n     lf?;\n \n@@ -134,14 +132,17 @@ Format of STDIN stream:\n   ts    ::= # time since the epoch in seconds, ascii base10 notation;\n   tz    ::= # GIT style timezone;\n \n-     # note: comments may appear anywhere in the input, except\n-     # within a data command.  Any form of the data command\n-     # always escapes the related input from comment processing.\n+     # note: comments and cat requests may appear anywhere\n+     # in the input, except within a data command.  Any form\n+     # of the data command always escapes the related input\n+     # from comment processing.\n      #\n      # In case it is not clear, the '#' that starts the comment\n      # must be the first character on that line (an lf\n      # preceded it).\n      #\n+  cat_blob ::= 'cat-blob' sp (hexsha1 | idnum) lf;\n+\n   comment ::= '#' not_lf* lf;\n   not_lf  ::= # Any byte that is not ASCII newline (LF);\n */\n@@ -367,6 +368,7 @@ static int seen_data_command;\n static int cat_blob_fd = STDOUT_FILENO;\n \n static void parse_argv(void);\n+static void parse_cat_blob(void);\n \n static void write_branch_report(FILE *rpt, struct branch *b)\n {\n@@ -1791,7 +1793,6 @@ static void read_marks(void)\n \tfclose(f);\n }\n \n-\n static int read_next_command(void)\n {\n \tstatic int stdin_eof = 0;\n@@ -1801,7 +1802,7 @@ static int read_next_command(void)\n \t\treturn EOF;\n \t}\n \n-\tdo {\n+\tfor (;;) {\n \t\tif (unread_command_buf) {\n \t\t\tunread_command_buf = 0;\n \t\t} else {\n@@ -1834,9 +1835,14 @@ static int read_next_command(void)\n \t\t\trc->prev->next = rc;\n \t\t\tcmd_tail = rc;\n \t\t}\n-\t} while (command_buf.buf[0] == '#');\n-\n-\treturn 0;\n+\t\tif (!prefixcmp(command_buf.buf, \"cat-blob \")) {\n+\t\t\tparse_cat_blob();\n+\t\t\tcontinue;\n+\t\t}\n+\t\tif (command_buf.buf[0] == '#')\n+\t\t\tcontinue;\n+\t\treturn 0;\n+\t}\n }\n \n static void skip_optional_lf(void)\n@@ -3062,8 +3068,6 @@ int main(int argc, const char **argv)\n \t\t\tparse_new_tag();\n \t\telse if (!prefixcmp(command_buf.buf, \"reset \"))\n \t\t\tparse_reset_branch();\n-\t\telse if (!prefixcmp(command_buf.buf, \"cat-blob \"))\n-\t\t\tparse_cat_blob();\n \t\telse if (!strcmp(\"checkpoint\", command_buf.buf))\n \t\t\tparse_checkpoint();\n \t\telse if (!prefixcmp(command_buf.buf, \"progress \"))\ndiff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh\nindex 3e2741b..2a050c7 100755\n--- a/t/t9300-fast-import.sh\n+++ b/t/t9300-fast-import.sh\n@@ -1692,6 +1692,72 @@ test_expect_success PIPE 'R: copy using cat-file' '\n \ttest_cmp big actual\n '\n \n+test_expect_success PIPE 'R: print blob mid-commit' '\n+\trm -f blobs &&\n+\techo \"A blob from _before_ the commit.\" >expect &&\n+\tmkfifo blobs &&\n+\t(\n+\t\texec 3<blobs &&\n+\t\tcat <<-EOF &&\n+\t\tfeature cat-blob\n+\t\tblob\n+\t\tmark :1\n+\t\tdata <<BLOB\n+\t\tA blob from _before_ the commit.\n+\t\tBLOB\n+\t\tcommit refs/heads/temporary\n+\t\tcommitter $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE\n+\t\tdata <<COMMIT\n+\t\tEmpty commit\n+\t\tCOMMIT\n+\t\tcat-blob :1\n+\t\tEOF\n+\n+\t\tread blob_id type size <&3 &&\n+\t\tdd if=/dev/stdin of=actual bs=$size count=1 <&3 &&\n+\t\tread newline <&3 &&\n+\n+\t\techo\n+\t) |\n+\tgit fast-import --cat-blob-fd=3 3>blobs &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success PIPE 'R: print staged blob within commit' '\n+\trm -f blobs &&\n+\techo \"A blob from _within_ the commit.\" >expect &&\n+\tmkfifo blobs &&\n+\t(\n+\t\texec 3<blobs &&\n+\t\tcat <<-EOF &&\n+\t\tfeature cat-blob\n+\t\tcommit refs/heads/within\n+\t\tcommitter $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE\n+\t\tdata <<COMMIT\n+\t\tEmpty commit\n+\t\tCOMMIT\n+\t\tM 644 inline within\n+\t\tdata <<BLOB\n+\t\tA blob from _within_ the commit.\n+\t\tBLOB\n+\t\tEOF\n+\n+\t\tto_get=$(\n+\t\t\techo \"A blob from _within_ the commit.\" |\n+\t\t\tgit hash-object --stdin\n+\t\t) &&\n+\t\techo \"cat-blob $to_get\" &&\n+\n+\t\tread blob_id type size <&3 &&\n+\t\tdd if=/dev/stdin of=actual bs=$size count=1 <&3 &&\n+\t\tread newline <&3 &&\n+\n+\t\techo deleteall\n+\t) |\n+\tgit fast-import --cat-blob-fd=3 3>blobs &&\n+\ttest_cmp expect actual\n+'\n+\n cat >input << EOF\n option git quiet\n blob\n"},{"id":"156869","messageId":"1291074508-18926-1-git-send-email-david.barr@cordelta.com","threadId":"25454","inReplyTo":"20101128194501.GD19998@burratino","subject":"[PATCH] fixup! fast-import: let importers retrieve blobs","fromName":"David Barr","fromEmail":"david.barr@cordelta.com","sentAt":"2010-11-29T23:48:28Z","receivedAt":"2010-11-29T23:48:28Z","isPatch":true,"sender":{"key":"david.barr@cordelta.com","avatar":"https://avatars.githubusercontent.com/u/220594?v=4"},"body":"Signed-off-by: David Barr <david.barr@cordelta.com>\n---\n fast-import.c |    2 ++\n 1 files changed, 2 insertions(+), 0 deletions(-)\n\ndiff --git a/fast-import.c b/fast-import.c\nindex 4dfea07..aa8f260 100644\n--- a/fast-import.c\n+++ b/fast-import.c\n@@ -2752,6 +2752,7 @@ static void cat_blob(struct object_entry *oe, unsigned char sha1[20])\n \t\tstrbuf_reset(&line);\n \t\tstrbuf_addf(&line, \"%s missing\\n\", sha1_to_hex(sha1));\n \t\tcat_blob_write(line.buf, line.len);\n+\t\tstrbuf_release(&line);\n \t\tfree(buf);\n \t\treturn;\n \t}\n@@ -2764,6 +2765,7 @@ static void cat_blob(struct object_entry *oe, unsigned char sha1[20])\n \tstrbuf_addf(&line, \"%s %s %lu\\n\", sha1_to_hex(sha1),\n \t\t\t\t\t\ttypename(type), size);\n \tcat_blob_write(line.buf, line.len);\n+\tstrbuf_release(&line);\n \tcat_blob_write(buf, size);\n \tcat_blob_write(\"\\n\", 1);\n \tfree(buf);\n-- \n1.7.3.2.846.gf4b062\n"},{"id":"156872","messageId":"201011301116.30288.david.barr@cordelta.com","threadId":"25454","inReplyTo":"1291074508-18926-1-git-send-email-david.barr@cordelta.com","subject":"Re: [PATCH] fixup! fast-import: let importers retrieve blobs","fromName":"David Barr","fromEmail":"david.barr@cordelta.com","sentAt":"2010-11-30T00:16:30Z","receivedAt":"2010-11-30T00:16:30Z","isPatch":true,"sender":{"key":"david.barr@cordelta.com","avatar":"https://avatars.githubusercontent.com/u/220594?v=4"},"body":"> David Barr wrote:\n> \n> [plug two memory leaks in \"[PATCH 3/4] fast-import: let importers retr...\"]\n> >\n> > Signed-off-by: David Barr <david.barr@cordelta.com>\n> \n> Good eyes, thanks!\n> \n> Acked-by: Jonathan Nieder <jrnieder@gmail.com>\n\nI only caught it because I was copying and adapting the same bit of code for \nmy 'ls' implementation. The svndiff0 implementation taught me to feel nervous\nwherever I see STRBUF_INIT ;)\n\n--\nDavid Barr\n"},{"id":"156868","messageId":"7vzksrwrnt.fsf@alter.siamese.dyndns.org","threadId":"25454","inReplyTo":"20101128194246.GB19998@burratino","subject":"Re: [PATCH 1/4] fast-import: stricter parsing of integer options","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-11-30T01:01:42Z","receivedAt":"2010-11-30T01:01:42Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder <jrnieder@gmail.com> writes:\n\n> +static unsigned long ulong_arg(const char *option, const char *arg)\n> +{\n> +\tchar *endptr;\n> +\tunsigned long rv = strtoul(arg, &endptr, 0);\n> +\tif (strchr(arg, '-') || endptr == arg || *endptr)\n> +\t\tdie(\"%s: argument must be an unsigned integer\", option);\n\nMicronit.\n\nIt probably is Ok for the target audience, but it might be more proper to\ncall it \"non-negative integer\" (\"unsigned integer\" is a container to hold\nsuch quantity).\n"},{"id":"156870","messageId":"20101130012214.GA12515@burratino","threadId":"25454","inReplyTo":"1291074508-18926-1-git-send-email-david.barr@cordelta.com","subject":"Re: [PATCH] fixup! fast-import: let importers retrieve blobs","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-11-30T01:22:14Z","receivedAt":"2010-11-30T01:22:14Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"David Barr wrote:\n\n[plug two memory leaks in \"[PATCH 3/4] fast-import: let importers retr...\"]\n>\n> Signed-off-by: David Barr <david.barr@cordelta.com>\n\nGood eyes, thanks!\n\nAcked-by: Jonathan Nieder <jrnieder@gmail.com>\n"},{"id":"157195","messageId":"201012031130.06008.trast@student.ethz.ch","threadId":"25454","inReplyTo":"20101128194501.GD19998@burratino","subject":"Re: [PATCH 3/4] fast-import: let importers retrieve blobs","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2010-12-03T10:30:05Z","receivedAt":"2010-12-03T10:30:05Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Jonathan Nieder wrote:\n> +test_expect_success PIPE 'R: copy using cat-file' '\n[...]\n> +\tdd if=/dev/stdin of=blob bs=$size count=1 <&3 &&\n\nThis breaks my automated tester, though I am not sure exactly why.  It\nruns RHEL5, and I have\n\n  $ ls -l /dev/std*\n  lrwxrwxrwx 1 root root 15 Sep  1 09:25 /dev/stderr -> /proc/self/fd/2\n  lrwxrwxrwx 1 root root 15 Sep  1 09:25 /dev/stdin -> /proc/self/fd/0\n  lrwxrwxrwx 1 root root 15 Sep  1 09:25 /dev/stdout -> /proc/self/fd/1\n\nBut from the tests I get back\n\n  dd: opening `/dev/stdin': No such file or directory\n  error: git-fast-import died of signal 13\n  not ok - 110 R: copy using cat-file\n\nIn any case I cannot see a reason to use this construct: 'dd' reads\nfrom stdin by default, so you could just leave away the option.\n\n-- \nThomas Rast\ntrast@{inf,student}.ethz.ch\n"},{"id":"157237","messageId":"20101203190643.GC14049@burratino","threadId":"25454","inReplyTo":"201012031130.06008.trast@student.ethz.ch","subject":"Re: [PATCH 3/4] fast-import: let importers retrieve blobs","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-12-03T19:06:43Z","receivedAt":"2010-12-03T19:06:43Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Thomas Rast wrote:\n\n> In any case I cannot see a reason to use this construct: 'dd' reads\n> from stdin by default, so you could just leave away the option.\n\nYes, sorry about that.  I had confused dd with 'tar' which defaults to\nrmt0 on some systems.  Fixed locally.\n"},{"id":"157252","messageId":"7vsjyeobka.fsf@alter.siamese.dyndns.org","threadId":"25454","inReplyTo":"201012031130.06008.trast@student.ethz.ch","subject":"Re: [PATCH 3/4] fast-import: let importers retrieve blobs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-12-03T20:17:57Z","receivedAt":"2010-12-03T20:17:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thomas Rast <trast@student.ethz.ch> writes:\n\n> But from the tests I get back\n>\n>   dd: opening `/dev/stdin': No such file or directory\n>   error: git-fast-import died of signal 13\n>   not ok - 110 R: copy using cat-file\n>\n> In any case I cannot see a reason to use this construct: 'dd' reads\n> from stdin by default, so you could just leave away the option.\n\nThanks for testing and reporting.\n-- >8 --\nSubject: t9300: remove unnecessary use of /dev/stdin\n\nWe really shouldn't be using these funny /dev/* files that did not exist\nin V7 UNIX in our tests when we do not have to.\n\nOutput from\n\n    $ git grep -n -e /dev/ --and --not -e /dev/null t/\n\ntells us that, aside from use of /dev/urandom in apache.conf used in http\ntests, \"dd if=/dev/stdin\" added recently to t/t9300-fast-import.sh are the\nonly offenders, so removing them should be straightforward.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n\n t/t9300-fast-import.sh |    6 +++---\n 1 files changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh\nindex d615d04..055ddc6 100755\n--- a/t/t9300-fast-import.sh\n+++ b/t/t9300-fast-import.sh\n@@ -1794,7 +1794,7 @@ test_expect_success PIPE 'R: copy using cat-file' '\n \n \tread blob_id type size <&3 &&\n \techo \"$blob_id $type $size\" >response &&\n-\tdd if=/dev/stdin of=blob bs=$size count=1 <&3 &&\n+\tdd of=blob bs=$size count=1 <&3 &&\n \tread newline <&3 &&\n \n \tcat <<EOF &&\n@@ -1845,7 +1845,7 @@ test_expect_success PIPE 'R: print blob mid-commit' '\n \t\tEOF\n \n \t\tread blob_id type size <&3 &&\n-\t\tdd if=/dev/stdin of=actual bs=$size count=1 <&3 &&\n+\t\tdd of=actual bs=$size count=1 <&3 &&\n \t\tread newline <&3 &&\n \n \t\techo\n@@ -1880,7 +1880,7 @@ test_expect_success PIPE 'R: print staged blob within commit' '\n \t\techo \"cat-blob $to_get\" &&\n \n \t\tread blob_id type size <&3 &&\n-\t\tdd if=/dev/stdin of=actual bs=$size count=1 <&3 &&\n+\t\tdd of=actual bs=$size count=1 <&3 &&\n \t\tread newline <&3 &&\n \n \t\techo deleteall\n"},{"id":"157255","messageId":"20101203202650.GA15517@burratino","threadId":"25454","inReplyTo":"7vsjyeobka.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 3/4] fast-import: let importers retrieve blobs","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-12-03T20:26:50Z","receivedAt":"2010-12-03T20:26:50Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Junio C Hamano wrote:\n\n> Output from\n> \n>     $ git grep -n -e /dev/ --and --not -e /dev/null t/\n\nFWIW\n\n\t$ git grep -e 'dd if='\n\nshows a few missed harmless examples.  Perhaps\n\n\t$ git grep -e '/dev/[^n]'\n\nwould have been the simplest way to catch them.\n"},{"id":"157291","messageId":"20101204023515.GA18735@burratino","threadId":"25454","inReplyTo":"20101128194501.GD19998@burratino","subject":"Re: [PATCH 3/4] fast-import: let importers retrieve blobs","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-12-04T02:35:15Z","receivedAt":"2010-12-04T02:35:15Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Jonathan Nieder wrote:\n>                                                              The value\n> for cat-blob-fd cannot be specified in the stream because that would\n> be a layering violation: the decision of where to direct a stream has\n> to be made when fast-import is started anyway, so we might as well\n> make the stream format is independent of that detail.\n\nUngrammatical.  I think I meant:\n\n There is no POSIX facility to open a file descriptor from outside\n after a process has already started; therefore, the frontend has to\n prepare a file descriptor for writing blobs before executing\n git fast-import.  The --cat-blob-fd command line option indicates\n which file descriptor that is, defaulting to 1.\n\n It does not make sense to wait until the stream starts to specify\n which fd so it is not allowed, avoiding a potential layering\n violation.  Other fast-import backends might provide other ways to\n specify where the blob stream should be written.\n\n> +++ b/fast-import.c\n> @@ -2824,6 +2910,8 @@ static int parse_one_feature(const char *feature, int from_stream)\n>  \t\toption_import_marks(feature + 13, from_stream);\n>  \t} else if (!prefixcmp(feature, \"export-marks=\")) {\n>  \t\toption_export_marks(feature + 13);\n> +\t} else if (!strcmp(feature, \"cat-blob\")) {\n> +\t\t; /* Don't die - this feature is supported */\n\nImplies support for a \"--cat-blob\" command line option\nthat checks for cat-blob support.  Is this wanted?\n\n(If so, it should be documented.  If not, the condition should be \n\"from_stream && !strcmp(...)\".)\n\n> @@ -2918,6 +3006,11 @@ static void parse_argv(void)\n>  \t\tif (parse_one_feature(a + 2, 0))\n>  \t\t\tcontinue;\n>  \n> +\t\tif (!prefixcmp(a + 2, \"cat-blob-fd=\")) {\n> +\t\t\toption_cat_blob_fd(a + 2 + strlen(\"cat-blob-fd=\"));\n> +\t\t\tcontinue;\n> +\t\t}\n> +\n\nWould be simpler and more explicit to put in parse_one_feature:\n\n\t} else if (!from_stream && !prefixcmp(feature, \"cat-blob-fd=\")) {\n\nSorry this is taking so long to get right. :-/\nJonathan\n"},{"id":"157302","messageId":"201012041424.33146.trast@student.ethz.ch","threadId":"25454","inReplyTo":"201012031130.06008.trast@student.ethz.ch","subject":"Re: [PATCH 3/4] fast-import: let importers retrieve blobs","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2010-12-04T13:24:32Z","receivedAt":"2010-12-04T13:24:32Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Thomas Rast wrote:\n> Jonathan Nieder wrote:\n> > +test_expect_success PIPE 'R: copy using cat-file' '\n> [...]\n> > +\tdd if=/dev/stdin of=blob bs=$size count=1 <&3 &&\n> \n> This breaks my automated tester, though I am not sure exactly why.  It\n> runs RHEL5, and I have\n> \n>   lrwxrwxrwx 1 root root 15 Sep  1 09:25 /dev/stdin -> /proc/self/fd/0\n\nAh, answering my own question: on my normal box, strace'ing dd[1] in\nsuch an invocation uses\n\n  open(\"/dev/stdin\", O_RDONLY)            = 3\n  dup2(3, 0)                              = 0\n  close(3)                                = 0\n\nOTOH on RHEL5[2] it tries a different order:\n\n  close(0)                                = 0\n  open(\"/dev/stdin\", O_RDONLY)            = -1 ENOENT (No such file or directory)\n\nOops.\n\n\n[1] dd --version says: dd (coreutils) 7.1\n[2] dd (coreutils) 5.97\n\n-- \nThomas Rast\ntrast@{inf,student}.ethz.ch\n"},{"id":"159537","messageId":"20110116021605.GA28307@burratino","threadId":"25454","inReplyTo":"20101128194501.GD19998@burratino","subject":"[PATCH] Documentation/fast-import: capitalize beginning of sentence","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-01-16T02:16:05Z","receivedAt":"2011-01-16T02:16:05Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>\n---\nNoticed by skimming through the v1.7.3..v1.7.4-rc2 diff.\n\n Documentation/git-fast-import.txt |    2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/Documentation/git-fast-import.txt b/Documentation/git-fast-import.txt\nindex e2a46a5..43d2174 100644\n--- a/Documentation/git-fast-import.txt\n+++ b/Documentation/git-fast-import.txt\n@@ -905,7 +905,7 @@ The `<dataref>` can be either a mark reference (`:<idnum>`)\n set previously or a full 40-byte SHA-1 of a Git blob, preexisting or\n ready to be written.\n \n-output uses the same format as `git cat-file --batch`:\n+Output uses the same format as `git cat-file --batch`:\n \n ====\n \t<sha1> SP 'blob' SP <size> LF\n-- \n1.7.4.rc2\n"}]}