* [PATCH/RFC] fast-import: add 'ls' command @ 2010-12-02 10:40 David Barr 2010-12-02 10:40 ` [PATCH] " David Barr 2011-01-03 8:01 ` [PATCH/RFC v2 0/3] " Jonathan Nieder 0 siblings, 2 replies; 24+ messages in thread From: David Barr @ 2010-12-02 10:40 UTC (permalink / raw) To: Git Mailing List; +Cc: Ramkumar Ramachandra, Sverre Rabbelier, Shawn O. Pearce This patch is by no means complete - I still need to consider the edge cases. It does achieve the basic requirements for simplifying svn-fe. The vcs-svn library currently maintains an in-memory index of all paths in all revisions. Introducing an `ls` command to fast-import allows this responsibility to be delegated. Most importantly, it will allow access to the tree data on demand which is needed for incremental imports. The two features that svn-fe will need are access the the current in-flight commit and to previous commits by mark. fast-import.c | 127 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++- 1 files changed, 125 insertions(+), 2 deletions(-) ^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH] fast-import: add 'ls' command 2010-12-02 10:40 [PATCH/RFC] fast-import: add 'ls' command David Barr @ 2010-12-02 10:40 ` David Barr 2010-12-02 12:37 ` Sverre Rabbelier 2011-01-03 8:01 ` [PATCH/RFC v2 0/3] " Jonathan Nieder 1 sibling, 1 reply; 24+ messages in thread From: David Barr @ 2010-12-02 10:40 UTC (permalink / raw) To: Git Mailing List Cc: Ramkumar Ramachandra, Sverre Rabbelier, Shawn O. Pearce, David Barr There are two forms of the 'ls' command, one that takes a tree-ish and one relative to the index. Allow the tree-ish variant to be used anywhere a comment is allowed. Allow the index variant to be used within a commit where file change commands would be used. The syntax is as such: 'ls' SP <dataref> SP path LF and 'ls' SP 'index' SP path LF Based-on-patch-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: David Barr <david.barr@cordelta.com> --- fast-import.c | 127 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++- 1 files changed, 125 insertions(+), 2 deletions(-) diff --git a/fast-import.c b/fast-import.c index fbc70cd..854398a 100644 --- a/fast-import.c +++ b/fast-import.c @@ -24,10 +24,12 @@ Format of STDIN stream: commit_msg ('from' sp committish lf)? ('merge' sp committish lf)* - file_change* + (file_change | ls)* lf?; commit_msg ::= data; + ls ::= 'ls' sp 'index' sp path_str lf; + file_change ::= file_clr | file_del | file_rnm @@ -132,7 +134,7 @@ Format of STDIN stream: ts ::= # time since the epoch in seconds, ascii base10 notation; tz ::= # GIT style timezone; - # note: comments and cat requests may appear anywhere + # note: comments, ls and cat requests may appear anywhere # in the input, except within a data command. Any form # of the data command always escapes the related input # from comment processing. @@ -141,7 +143,9 @@ Format of STDIN stream: # must be the first character on that line (an lf # preceded it). # + cat_blob ::= 'cat-blob' sp (hexsha1 | idnum) lf; + ls_tree ::= 'ls' sp (hexsha1 | idnum) sp path_str lf; comment ::= '#' not_lf* lf; not_lf ::= # Any byte that is not ASCII newline (LF); @@ -369,6 +373,7 @@ static int cat_blob_fd = STDOUT_FILENO; static void parse_argv(void); static void parse_cat_blob(void); +static void parse_ls(struct branch *b); /* Signal handling */ static volatile sig_atomic_t checkpoint_requested; @@ -2607,6 +2612,8 @@ static void parse_new_commit(void) note_change_n(b, prev_fanout); else if (!strcmp("deleteall", command_buf.buf)) file_change_deleteall(b); + else if (!prefixcmp(command_buf.buf, "ls ")) + parse_ls(b); else { unread_command_buf = 1; break; @@ -2830,6 +2837,120 @@ static void parse_cat_blob(void) cat_blob(oe, sha1); } +static struct object_entry *parse_treeish_dataref(const char** r) +{ + unsigned char sha1[20]; + struct object_entry *e; + + if (**r == ':') { + char *x; + e = find_mark(strtoumax(*r + 1, &x, 10)); + if (x == *r + 1) + die("Invalid mark: %s", command_buf.buf); + if (!e) + die("Unknown mark: %s", command_buf.buf); + *r = x; + hashcpy(sha1, e->idx.sha1); + } else { + if (get_sha1_hex(*r, sha1)) + die("Invalid SHA1: %s", command_buf.buf); + e = find_object(sha1); + *r += 40; + } + + for (;;) { + unsigned long size; + void *buf; + if (!e) { + enum object_type type = sha1_object_info(sha1, NULL); + if (type < 0) + die("object not found: %s", sha1_to_hex(sha1)); + e = insert_object(sha1); + e->type = type; + e->pack_id = MAX_PACK_ID; + e->idx.offset = 1; + } + if (e->type == OBJ_TREE) + break; + + if (e->type != OBJ_COMMIT && e->type != OBJ_TAG) + die("Not a treeish: %s", command_buf.buf); + + if (e->pack_id != MAX_PACK_ID) { + buf = gfi_unpack_entry(e, &size); + } else { + enum object_type type; + buf = read_sha1_file(sha1, &type, &size); + } + if (!buf) + die("Can't load object %s", sha1_to_hex(sha1)); + + if (e->type == OBJ_COMMIT) { + if (size < 40 + strlen("tree ") || + get_sha1_hex(buf + strlen("tree "), sha1)) + die("Invalid SHA1 in commit: %s", command_buf.buf); + } else { + if (size < 40 + strlen("object ") || + get_sha1_hex(buf + strlen("object "), sha1)) + die("Invalid SHA1 in tag: %s", command_buf.buf); + } + free(buf); + e = find_object(sha1); + } + + return e; +} + +static void print_ls(int mode, unsigned char *sha1, char *path) +{ + enum object_type type; + struct strbuf line = STRBUF_INIT; + type = sha1_object_info(sha1, NULL); + /* mode SP type SP object_name TAB path LF */ + strbuf_addf(&line, "%o %s %s\t%s\n", + mode, typename(type), sha1_to_hex(sha1), path); + cat_blob_write(line.buf, line.len); + strbuf_release(&line); +} + +static void parse_ls(struct branch *b) +{ + const char *p; + struct strbuf uq = STRBUF_INIT; + struct tree_entry *root = NULL; + struct tree_entry leaf = {0}; + + /* ls SP <treeish> SP <path> */ + p = command_buf.buf + strlen("ls "); + if(!prefixcmp(p, "index")) { + p += strlen("index"); + if (!b) + die("Not in a commit: %s", command_buf.buf); + root = &b->branch_tree; + } else { + struct object_entry *e = parse_treeish_dataref(&p); + root = new_tree_entry(); + hashcpy(root->versions[1].sha1, e->idx.sha1); + load_tree(root); + } + if (*p++ != ' ') + die("Missing space after SHA1: %s", command_buf.buf); + if (unquote_c_style(&uq, p, &p)) + die("Invalid path: %s", command_buf.buf); + if (*p) + die("Garbage after path: %s", command_buf.buf); + tree_content_get(root, uq.buf, &leaf); + if (!leaf.versions[1].mode) + die("Path %s not in branch", uq.buf); + /* Allow new trees to be listed. */ + if (S_ISDIR(leaf.versions[1].mode)) + store_tree(&leaf); + print_ls(leaf.versions[1].mode, leaf.versions[1].sha1, uq.buf); + strbuf_release(&uq); + if (!b || root != &b->branch_tree) + release_tree_entry(root); +} + static void checkpoint(void) { checkpoint_requested = 0; @@ -3128,6 +3249,8 @@ int main(int argc, const char **argv) while (read_next_command() != EOF) { if (!strcmp("blob", command_buf.buf)) parse_new_blob(); + else if (!prefixcmp(command_buf.buf, "ls ")) + parse_ls(NULL); else if (!prefixcmp(command_buf.buf, "commit ")) parse_new_commit(); else if (!prefixcmp(command_buf.buf, "tag ")) -- 1.7.3 ^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH] fast-import: add 'ls' command 2010-12-02 10:40 ` [PATCH] " David Barr @ 2010-12-02 12:37 ` Sverre Rabbelier 2010-12-02 12:57 ` David Michael Barr 2010-12-02 17:37 ` Jonathan Nieder 0 siblings, 2 replies; 24+ messages in thread From: Sverre Rabbelier @ 2010-12-02 12:37 UTC (permalink / raw) To: David Barr; +Cc: Git Mailing List, Ramkumar Ramachandra, Shawn O. Pearce Heya, On Thu, Dec 2, 2010 at 11:40, David Barr <david.barr@cordelta.com> wrote: > There are two forms of the 'ls' command, one that takes a tree-ish and > one relative to the index. Allow the tree-ish variant to be used anywhere > a comment is allowed. Allow the index variant to be used within a commit > where file change commands would be used. The commit message doesn't explain why the index variant isn't allowed anywhere a comment is allowed. I assume that's because there's a half-constructed index if you're in the middle of a modify operation or such? -- Cheers, Sverre Rabbelier ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] fast-import: add 'ls' command 2010-12-02 12:37 ` Sverre Rabbelier @ 2010-12-02 12:57 ` David Michael Barr 2010-12-02 17:37 ` Jonathan Nieder 1 sibling, 0 replies; 24+ messages in thread From: David Michael Barr @ 2010-12-02 12:57 UTC (permalink / raw) To: Sverre Rabbelier Cc: Git Mailing List, Ramkumar Ramachandra, Shawn O. Pearce, Jonathan Nieder Hi Sverre, >> There are two forms of the 'ls' command, one that takes a tree-ish and >> one relative to the index. Allow the tree-ish variant to be used anywhere >> a comment is allowed. Allow the index variant to be used within a commit >> where file change commands would be used. > > The commit message doesn't explain why the index variant isn't allowed > anywhere a comment is allowed. I assume that's because there's a > half-constructed index if you're in the middle of a modify operation > or such? The index variant is allowed between file change commands, at which the index should be consistent albeit deferred for hashing. I believe this is the only place where there's naturally an implicit active tree. Simply, "relative to the index" only makes sense when describing a commit. Unfortunately, the list server dropped my summary email :( The patch was supposed to be marked as a RFC and I gave an outline of the intended use. The key features I need for svn-fe are access to the index for the current commit and access to the content of older marked commits. NB: This version doesn't support unquoted paths. -- David Barr ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] fast-import: add 'ls' command 2010-12-02 12:37 ` Sverre Rabbelier 2010-12-02 12:57 ` David Michael Barr @ 2010-12-02 17:37 ` Jonathan Nieder 2010-12-02 19:20 ` Junio C Hamano 1 sibling, 1 reply; 24+ messages in thread From: Jonathan Nieder @ 2010-12-02 17:37 UTC (permalink / raw) To: Sverre Rabbelier Cc: David Barr, Git Mailing List, Ramkumar Ramachandra, Shawn O. Pearce Sverre Rabbelier wrote: > On Thu, Dec 2, 2010 at 11:40, David Barr <david.barr@cordelta.com> wrote: >> There are two forms of the 'ls' command, one that takes a tree-ish and >> one relative to the index. Allow the tree-ish variant to be used anywhere >> a comment is allowed. Allow the index variant to be used within a commit >> where file change commands would be used. > > The commit message doesn't explain why the index variant isn't allowed > anywhere a comment is allowed. I assume that's because there's a > half-constructed index if you're in the middle of a modify operation > or such? I somewhat agree. Actually I would go further: the word "index" brings to mind .git/index and its in-core counterpart, so at the same time as documenting it better, we might look into making it more self- explanatory. Maybe a syntax like ls current "path/to/entry" would make it clearer that this is about directory entries in fast-import's active commit and not necessarily the usual index file? I am not sure what syntax other vcs-es use for tree-ishes. To avoid name clashes (what if 'current' is the low-level name of a tree-ish?), an alternative might be ls-tree :11 "path/to/historical/entry" ls "path/to/current/entry" Hm (just musing). Jonathan ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] fast-import: add 'ls' command 2010-12-02 17:37 ` Jonathan Nieder @ 2010-12-02 19:20 ` Junio C Hamano 2010-12-02 22:51 ` David Barr 0 siblings, 1 reply; 24+ messages in thread From: Junio C Hamano @ 2010-12-02 19:20 UTC (permalink / raw) To: Jonathan Nieder Cc: Sverre Rabbelier, David Barr, Git Mailing List, Ramkumar Ramachandra, Shawn O. Pearce Jonathan Nieder <jrnieder@gmail.com> writes: > I somewhat agree. Actually I would go further: the word "index" brings > to mind .git/index and its in-core counterpart, so at the same time as > documenting it better, we might look into making it more self- > explanatory. Maybe a syntax like > > ls current "path/to/entry" > > would make it clearer that this is about directory entries in > fast-import's active commit and not necessarily the usual index file? I think that explains the feature better. I was wondering if the stream somehow wanted to access the state the index of the repository happens to be. > I am not sure what syntax other vcs-es use for tree-ishes. To avoid > name clashes (what if 'current' is the low-level name of a tree-ish?), > an alternative might be > > ls-tree :11 "path/to/historical/entry" > ls "path/to/current/entry" Is it an option to use "ls" for both cases and treat one-arg and two-arg cases differently? ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] fast-import: add 'ls' command 2010-12-02 19:20 ` Junio C Hamano @ 2010-12-02 22:51 ` David Barr 0 siblings, 0 replies; 24+ messages in thread From: David Barr @ 2010-12-02 22:51 UTC (permalink / raw) To: Junio C Hamano Cc: Jonathan Nieder, Sverre Rabbelier, Git Mailing List, Ramkumar Ramachandra, Shawn O. Pearce > > I somewhat agree. Actually I would go further: the word "index" brings > > to mind .git/index and its in-core counterpart, so at the same time as > > documenting it better, we might look into making it more self- > > explanatory. Maybe a syntax like > > > > ls current "path/to/entry" > > > > would make it clearer that this is about directory entries in > > fast-import's active commit and not necessarily the usual index file? > > I think that explains the feature better. I was wondering if the stream > somehow wanted to access the state the index of the repository happens to > be. > > > I am not sure what syntax other vcs-es use for tree-ishes. To avoid > > name clashes (what if 'current' is the low-level name of a tree-ish?), > > an alternative might be > > > > ls-tree :11 "path/to/historical/entry" > > ls "path/to/current/entry" > > Is it an option to use "ls" for both cases and treat one-arg and two-arg > cases differently? I like that idea, its not hard to implement if we keep quoting mandatory. -- David Barr. --- diff --git a/fast-import.c b/fast-import.c index 854398a..0b0f2a1 100644 --- a/fast-import.c +++ b/fast-import.c @@ -2922,8 +2922,7 @@ static void parse_ls(struct branch *b) /* ls SP <treeish> SP <path> */ p = command_buf.buf + strlen("ls "); - if(!prefixcmp(p, "index")) { - p += strlen("index"); + if(*p == '"') { if (!b) die("Not in a commit: %s", command_buf.buf); root = &b->branch_tree; @@ -2932,9 +2931,9 @@ static void parse_ls(struct branch *b) root = new_tree_entry(); hashcpy(root->versions[1].sha1, e->idx.sha1); load_tree(root); + if (*p++ != ' ') + die("Missing space after tree-ish: %s", command_buf.buf); } - if (*p++ != ' ') - die("Missing space after SHA1: %s", command_buf.buf); if (unquote_c_style(&uq, p, &p)) die("Invalid path: %s", command_buf.buf); if (*p) ^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH/RFC v2 0/3] fast-import: add 'ls' command 2010-12-02 10:40 [PATCH/RFC] fast-import: add 'ls' command David Barr 2010-12-02 10:40 ` [PATCH] " David Barr @ 2011-01-03 8:01 ` Jonathan Nieder 2011-01-03 8:22 ` [PATCH 1/3] fast-import: clarify handling of cat-blob feature Jonathan Nieder ` (3 more replies) 1 sibling, 4 replies; 24+ messages in thread From: Jonathan Nieder @ 2011-01-03 8:01 UTC (permalink / raw) To: David Barr Cc: Git Mailing List, Ramkumar Ramachandra, Sverre Rabbelier, Shawn O. Pearce David Barr wrote: > This patch is by no means complete - I still need to consider the edge cases. > It does achieve the basic requirements for simplifying svn-fe. It really does do that. About time for a reroll. Patches 1 and 2 are nearby fixes noticed while hacking at this. Changes in patch 3 from v1 will be mentioned in the same message as the patch. Thoughts, improvements, especially tests welcome. Let's get this feature ready for wide use. David Barr (1): fast-import: add 'ls' command Jonathan Nieder (2): fast-import: clarify handling of cat-blob feature fast-import: treat filemodify with empty tree as delete Documentation/git-fast-import.txt | 49 ++++++++++- fast-import.c | 181 +++++++++++++++++++++++++++++++++++-- t/t9300-fast-import.sh | 158 ++++++++++++++++++++++++++++++-- 3 files changed, 371 insertions(+), 17 deletions(-) -- 1.7.4.rc0 ^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 1/3] fast-import: clarify handling of cat-blob feature 2011-01-03 8:01 ` [PATCH/RFC v2 0/3] " Jonathan Nieder @ 2011-01-03 8:22 ` Jonathan Nieder 2011-01-03 8:24 ` [PATCH 2/3] fast-import: treat filemodify with empty tree as delete Jonathan Nieder ` (2 subsequent siblings) 3 siblings, 0 replies; 24+ messages in thread From: Jonathan Nieder @ 2011-01-03 8:22 UTC (permalink / raw) To: David Barr Cc: Git Mailing List, Ramkumar Ramachandra, Sverre Rabbelier, Shawn O. Pearce Date: Thu Dec 9 14:45:21 2010 -0600 Remove the undocumented --cat-blob command line option. It used to be a no-op. While at it, move parsing of --cat-blob-fd to parse_one_feature; this makes the parse_argv loop a little easier to read and puts the code implementing 'feature cat-blob' and --cat-blob-fd closer to each other. Signed-off-by: Jonathan Nieder <jrnieder@gmail.com> --- Forgot to mention: these are based against v1.7.4-rc0~24 (t9300: use perl "head -c" clone in place of "dd bs=1 count=16000" kluge, 2010-12-13) but I wouldn't be surprised if they apply cleanly to other commits, too. ;-) fast-import.c | 9 +++------ t/t9300-fast-import.sh | 9 +++++++++ 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/fast-import.c b/fast-import.c index 7857760..a5cea45 100644 --- a/fast-import.c +++ b/fast-import.c @@ -2977,8 +2977,10 @@ static int parse_one_feature(const char *feature, int from_stream) option_import_marks(feature + 13, from_stream); } else if (!prefixcmp(feature, "export-marks=")) { option_export_marks(feature + 13); - } else if (!strcmp(feature, "cat-blob")) { + } else if (from_stream && !strcmp(feature, "cat-blob")) { ; /* Don't die - this feature is supported */ + } else if (!from_stream && !prefixcmp(feature, "cat-blob-fd=")) { + option_cat_blob_fd(feature + strlen("cat-blob-fd=")); } else if (!prefixcmp(feature, "relative-marks")) { relative_marks_paths = 1; } else if (!prefixcmp(feature, "no-relative-marks")) { @@ -3073,11 +3075,6 @@ static void parse_argv(void) if (parse_one_feature(a + 2, 0)) continue; - if (!prefixcmp(a + 2, "cat-blob-fd=")) { - option_cat_blob_fd(a + 2 + strlen("cat-blob-fd=")); - continue; - } - die("unknown option %s", a); } if (i != global_argc) diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh index 222d105..53aad51 100755 --- a/t/t9300-fast-import.sh +++ b/t/t9300-fast-import.sh @@ -1769,10 +1769,19 @@ test_expect_success 'R: feature cat-blob supported' ' git fast-import ' +test_expect_success 'R: no command line option for cat-blob feature' ' + test_must_fail git fast-import --cat-blob <empty +' + test_expect_success 'R: cat-blob-fd must be a nonnegative integer' ' test_must_fail git fast-import --cat-blob-fd=-1 </dev/null ' +test_expect_success 'R: cat-blob-fd cannot be specified in stream' ' + echo "feature cat-blob-fd=1" | + test_must_fail git fast-import +' + test_expect_success 'R: print old blob' ' blob=$(echo "yes it can" | git hash-object -w --stdin) && cat >expect <<-EOF && -- 1.7.4.rc0.580.g89dc.dirty ^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 2/3] fast-import: treat filemodify with empty tree as delete 2011-01-03 8:01 ` [PATCH/RFC v2 0/3] " Jonathan Nieder 2011-01-03 8:22 ` [PATCH 1/3] fast-import: clarify handling of cat-blob feature Jonathan Nieder @ 2011-01-03 8:24 ` Jonathan Nieder 2011-01-26 22:41 ` [PATCH v2] " Jonathan Nieder 2011-01-03 8:37 ` [PATCH 3/3] fast-import: add 'ls' command Jonathan Nieder 2011-01-26 21:39 ` [RFC] fast-import: 'cat-blob' and 'ls' commands Jonathan Nieder 3 siblings, 1 reply; 24+ messages in thread From: Jonathan Nieder @ 2011-01-03 8:24 UTC (permalink / raw) To: David Barr Cc: Git Mailing List, Ramkumar Ramachandra, Sverre Rabbelier, Shawn O. Pearce Date: Sat, 11 Dec 2010 16:42:28 -0600 Traditionally, git trees do not contain entries for empty subdirectories. Generally speaking, subtrees are not created or destroyed explicitly; instead, they automatically appear when needed to hold regular files, symlinks, and submodules. v1.7.3-rc0~75^2 (Teach fast-import to import subtrees named by tree id, 2010-06-30) changed that, by allowing an empty subtree to be included in a fast-import stream explicitly: M 040000 4b825dc642cb6eb9a060e54bf8d69288fbee4904 subdir That was unintentional. Better and more closely analogous to "git read-tree --prefix" to treat such an input line as a request to delete ("to empty") subdir. Noticed-by: David Barr <david.barr@cordelta.com> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com> --- If this seems like a good idea it might be a candidate for v1.7.4.x. Perhaps fsck.c should learn a "no empty trees" rule, too. fast-import.c | 10 ++++++++ t/t9300-fast-import.sh | 58 +++++++++++++++++++++++++++++++++++++++++------ 2 files changed, 60 insertions(+), 8 deletions(-) diff --git a/fast-import.c b/fast-import.c index a5cea45..385d12d 100644 --- a/fast-import.c +++ b/fast-import.c @@ -2231,6 +2231,16 @@ static void file_change_m(struct branch *b) p = uq.buf; } + /* + * Git does not track empty, non-toplevel directories. + */ + if (S_ISDIR(mode) && + !memcmp(sha1, (const unsigned char *) EMPTY_TREE_SHA1_BIN, 20) && + *p) { + tree_content_remove(&b->branch_tree, p, NULL); + return; + } + if (S_ISGITLINK(mode)) { if (inline_data) die("Git links cannot be specified 'inline': %s", diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh index 53aad51..b9aa3f0 100755 --- a/t/t9300-fast-import.sh +++ b/t/t9300-fast-import.sh @@ -42,6 +42,14 @@ echo "$@"' >empty +test_expect_success 'setup: have pipes?' ' + rm -f frob && + if mkfifo frob + then + test_set_prereq PIPE + fi +' + ### ### series A ### @@ -899,6 +907,48 @@ test_expect_success \ compare_diff_raw expect actual' test_expect_success \ + 'N: delete directory by copying' \ + 'cat >expect <<-\EOF && + OBJID + :100644 000000 OBJID OBJID D foo/bar/qux + OBJID + :000000 100644 OBJID OBJID A foo/bar/baz + :000000 100644 OBJID OBJID A foo/bar/qux + EOF + empty_tree=$(git mktree </dev/null) && + cat >input <<-INPUT_END && + commit refs/heads/N-delete + committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE + data <<COMMIT + collect data to be deleted + COMMIT + + deleteall + M 100644 inline foo/bar/baz + data <<DATA_END + hello + DATA_END + C "foo/bar/baz" "foo/bar/qux" + C "foo/bar/baz" "foo/bar/quux/1" + C "foo/bar/baz" "foo/bar/quuux" + M 040000 $empty_tree foo/bar/quux + M 040000 $empty_tree foo/bar/quuux + + commit refs/heads/N-delete + committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE + data <<COMMIT + delete subdirectory + COMMIT + + M 040000 $empty_tree foo/bar/qux + INPUT_END + git fast-import <input && + git rev-list N-delete | + git diff-tree -r --stdin --root --always | + sed -e "s/$_x40/OBJID/g" >actual && + test_cmp expect actual' + +test_expect_success \ 'N: copy root directory by tree hash' \ 'cat >expect <<-\EOF && :100755 000000 f1fb5da718392694d0076d677d6d0e364c79b0bc 0000000000000000000000000000000000000000 D file3/newf @@ -1898,14 +1948,6 @@ test_expect_success 'R: print two blobs to stdout' ' test_cmp expect actual ' -test_expect_success 'setup: have pipes?' ' - rm -f frob && - if mkfifo frob - then - test_set_prereq PIPE - fi -' - test_expect_success PIPE 'R: copy using cat-file' ' expect_id=$(git hash-object big) && expect_len=$(wc -c <big) && -- 1.7.4.rc0.580.g89dc.dirty ^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v2] fast-import: treat filemodify with empty tree as delete 2011-01-03 8:24 ` [PATCH 2/3] fast-import: treat filemodify with empty tree as delete Jonathan Nieder @ 2011-01-26 22:41 ` Jonathan Nieder 2011-01-26 22:45 ` Sverre Rabbelier 0 siblings, 1 reply; 24+ messages in thread From: Jonathan Nieder @ 2011-01-26 22:41 UTC (permalink / raw) To: Junio C Hamano Cc: Git Mailing List, Ramkumar Ramachandra, Sverre Rabbelier, Shawn O. Pearce, David Barr Date: Sat, 11 Dec 2010 16:42:28 -0600 Normal git processes do not allow one to build a tree with an empty subtree entry without trying hard at it. This is in keeping with the general UI philosophy: git tracks content, not empty directories. v1.7.3-rc0~75^2 (2010-06-30) changed that by making it easy to include an empty subtree in fast-import's active commit: M 040000 4b825dc642cb6eb9a060e54bf8d69288fbee4904 subdir It is easy to trigger this by accident by reading an empty tree (for example, the tree corresponding to an empty root commit) and trying to move it to a subtree. It is better and more closely analogous to "git read-tree --prefix" to treat such commands as a request to remove ("to empty") the subdir. Noticed-by: David Barr <david.barr@cordelta.com> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com> --- Change since v1: commit message. Resubmitting this fix separately from the 3-part series it came from. Seems to work okay. :) fast-import.c | 10 ++++++++ t/t9300-fast-import.sh | 58 +++++++++++++++++++++++++++++++++++++++++------ 2 files changed, 60 insertions(+), 8 deletions(-) diff --git a/fast-import.c b/fast-import.c index 7857760..8b19d87 100644 --- a/fast-import.c +++ b/fast-import.c @@ -2231,6 +2231,16 @@ static void file_change_m(struct branch *b) p = uq.buf; } + /* + * Git does not track empty, non-toplevel directories. + */ + if (S_ISDIR(mode) && + !memcmp(sha1, (const unsigned char *) EMPTY_TREE_SHA1_BIN, 20) && + *p) { + tree_content_remove(&b->branch_tree, p, NULL); + return; + } + if (S_ISGITLINK(mode)) { if (inline_data) die("Git links cannot be specified 'inline': %s", diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh index 222d105..80ddfe0 100755 --- a/t/t9300-fast-import.sh +++ b/t/t9300-fast-import.sh @@ -42,6 +42,14 @@ echo "$@"' >empty +test_expect_success 'setup: have pipes?' ' + rm -f frob && + if mkfifo frob + then + test_set_prereq PIPE + fi +' + ### ### series A ### @@ -899,6 +907,48 @@ test_expect_success \ compare_diff_raw expect actual' +test_expect_success \ + 'N: delete directory by copying' \ + 'cat >expect <<-\EOF && + OBJID + :100644 000000 OBJID OBJID D foo/bar/qux + OBJID + :000000 100644 OBJID OBJID A foo/bar/baz + :000000 100644 OBJID OBJID A foo/bar/qux + EOF + empty_tree=$(git mktree </dev/null) && + cat >input <<-INPUT_END && + commit refs/heads/N-delete + committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE + data <<COMMIT + collect data to be deleted + COMMIT + + deleteall + M 100644 inline foo/bar/baz + data <<DATA_END + hello + DATA_END + C "foo/bar/baz" "foo/bar/qux" + C "foo/bar/baz" "foo/bar/quux/1" + C "foo/bar/baz" "foo/bar/quuux" + M 040000 $empty_tree foo/bar/quux + M 040000 $empty_tree foo/bar/quuux + + commit refs/heads/N-delete + committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE + data <<COMMIT + delete subdirectory + COMMIT + + M 040000 $empty_tree foo/bar/qux + INPUT_END + git fast-import <input && + git rev-list N-delete | + git diff-tree -r --stdin --root --always | + sed -e "s/$_x40/OBJID/g" >actual && + test_cmp expect actual' + test_expect_success \ 'N: copy root directory by tree hash' \ 'cat >expect <<-\EOF && :100755 000000 f1fb5da718392694d0076d677d6d0e364c79b0bc 0000000000000000000000000000000000000000 D file3/newf @@ -1889,14 +1939,6 @@ test_expect_success 'R: print two blobs to stdout' ' test_cmp expect actual ' -test_expect_success 'setup: have pipes?' ' - rm -f frob && - if mkfifo frob - then - test_set_prereq PIPE - fi -' - test_expect_success PIPE 'R: copy using cat-file' ' expect_id=$(git hash-object big) && expect_len=$(wc -c <big) && -- 1.7.4.rc3 ^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH v2] fast-import: treat filemodify with empty tree as delete 2011-01-26 22:41 ` [PATCH v2] " Jonathan Nieder @ 2011-01-26 22:45 ` Sverre Rabbelier 2011-01-26 23:06 ` [PATCH jn/fast-import-fix v3] " Jonathan Nieder 0 siblings, 1 reply; 24+ messages in thread From: Sverre Rabbelier @ 2011-01-26 22:45 UTC (permalink / raw) To: Jonathan Nieder Cc: Junio C Hamano, Git Mailing List, Ramkumar Ramachandra, Shawn O. Pearce, David Barr Heya, On Wed, Jan 26, 2011 at 23:41, Jonathan Nieder <jrnieder@gmail.com> wrote: > Seems to work okay. :) Should it go on maint now that it's factored out, since it shipped in 1.7.3, or just master? -- Cheers, Sverre Rabbelier ^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH jn/fast-import-fix v3] fast-import: treat filemodify with empty tree as delete 2011-01-26 22:45 ` Sverre Rabbelier @ 2011-01-26 23:06 ` Jonathan Nieder 2011-01-27 0:04 ` Junio C Hamano 0 siblings, 1 reply; 24+ messages in thread From: Jonathan Nieder @ 2011-01-26 23:06 UTC (permalink / raw) To: Sverre Rabbelier Cc: Junio C Hamano, Git Mailing List, Ramkumar Ramachandra, Shawn O. Pearce, David Barr Date: Sat, 11 Dec 2010 16:42:28 -0600 Normal git processes do not allow one to build a tree with an empty subtree entry without trying hard at it. This is in keeping with the general UI philosophy: git tracks content, not empty directories. Unfortunately, v1.7.3-rc0~75^2 (2010-06-30) changed that by making it easy to include an empty subtree in fast-import's active commit: M 040000 4b825dc642cb6eb9a060e54bf8d69288fbee4904 subdir It is easy to trigger this by accident by reading an empty tree (for example, the tree corresponding to an empty root commit) and trying to move it to a subtree. It would be better and more closely analogous to "git read-tree --prefix" to treat such commands as a request to remove ("to empty") the subdir. Noticed-by: David Barr <david.barr@cordelta.com> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com> --- Sverre Rabbelier wrote: > Should it go on maint now that it's factored out, since it shipped in > 1.7.3, or just master? Hmm. I suppose on top of b2124125 (jn/fast-import-fix). While applying it there I noticed that the change to t9300 includes an unrelated change (residue of an old rebase). Here's a fixed version. fast-import.c | 10 ++++++++++ t/t9300-fast-import.sh | 42 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 52 insertions(+), 0 deletions(-) diff --git a/fast-import.c b/fast-import.c index d881630..9cf26f1 100644 --- a/fast-import.c +++ b/fast-import.c @@ -2194,6 +2194,16 @@ static void file_change_m(struct branch *b) p = uq.buf; } + /* + * Git does not track empty, non-toplevel directories. + */ + if (S_ISDIR(mode) && + !memcmp(sha1, (const unsigned char *) EMPTY_TREE_SHA1_BIN, 20) && + *p) { + tree_content_remove(&b->branch_tree, p, NULL); + return; + } + if (S_ISGITLINK(mode)) { if (inline_data) die("Git links cannot be specified 'inline': %s", diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh index dd90a09..ef3a347 100755 --- a/t/t9300-fast-import.sh +++ b/t/t9300-fast-import.sh @@ -875,6 +875,48 @@ test_expect_success \ compare_diff_raw expect actual' +test_expect_success \ + 'N: delete directory by copying' \ + 'cat >expect <<-\EOF && + OBJID + :100644 000000 OBJID OBJID D foo/bar/qux + OBJID + :000000 100644 OBJID OBJID A foo/bar/baz + :000000 100644 OBJID OBJID A foo/bar/qux + EOF + empty_tree=$(git mktree </dev/null) && + cat >input <<-INPUT_END && + commit refs/heads/N-delete + committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE + data <<COMMIT + collect data to be deleted + COMMIT + + deleteall + M 100644 inline foo/bar/baz + data <<DATA_END + hello + DATA_END + C "foo/bar/baz" "foo/bar/qux" + C "foo/bar/baz" "foo/bar/quux/1" + C "foo/bar/baz" "foo/bar/quuux" + M 040000 $empty_tree foo/bar/quux + M 040000 $empty_tree foo/bar/quuux + + commit refs/heads/N-delete + committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE + data <<COMMIT + delete subdirectory + COMMIT + + M 040000 $empty_tree foo/bar/qux + INPUT_END + git fast-import <input && + git rev-list N-delete | + git diff-tree -r --stdin --root --always | + sed -e "s/$_x40/OBJID/g" >actual && + test_cmp expect actual' + test_expect_success \ 'N: copy root directory by tree hash' \ 'cat >expect <<-\EOF && :100755 000000 f1fb5da718392694d0076d677d6d0e364c79b0bc 0000000000000000000000000000000000000000 D file3/newf -- 1.7.4.rc3 ^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH jn/fast-import-fix v3] fast-import: treat filemodify with empty tree as delete 2011-01-26 23:06 ` [PATCH jn/fast-import-fix v3] " Jonathan Nieder @ 2011-01-27 0:04 ` Junio C Hamano 2011-01-27 0:26 ` Jonathan Nieder 2011-01-27 6:07 ` [PATCH v4] " Jonathan Nieder 0 siblings, 2 replies; 24+ messages in thread From: Junio C Hamano @ 2011-01-27 0:04 UTC (permalink / raw) To: Jonathan Nieder Cc: Sverre Rabbelier, Junio C Hamano, Git Mailing List, Ramkumar Ramachandra, Shawn O. Pearce, David Barr Jonathan Nieder <jrnieder@gmail.com> writes: > Sverre Rabbelier wrote: > >> Should it go on maint now that it's factored out, since it shipped in >> 1.7.3, or just master? > > Hmm. I suppose on top of b2124125 (jn/fast-import-fix). Hmm, why not on top of v1.7.3-rc0~75^2 aka 334fba6 (Teach fast-import to import subtrees named by tree id, 2010-06-30) then? > While applying it there I noticed that the change to t9300 includes an > unrelated change (residue of an old rebase). Here's a fixed version. > diff --git a/fast-import.c b/fast-import.c > index d881630..9cf26f1 100644 > --- a/fast-import.c > +++ b/fast-import.c > @@ -2194,6 +2194,16 @@ static void file_change_m(struct branch *b) > p = uq.buf; > } > > + /* > + * Git does not track empty, non-toplevel directories. > + */ > + if (S_ISDIR(mode) && > + !memcmp(sha1, (const unsigned char *) EMPTY_TREE_SHA1_BIN, 20) && Do you need this cast? ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH jn/fast-import-fix v3] fast-import: treat filemodify with empty tree as delete 2011-01-27 0:04 ` Junio C Hamano @ 2011-01-27 0:26 ` Jonathan Nieder 2011-01-27 6:07 ` [PATCH v4] " Jonathan Nieder 1 sibling, 0 replies; 24+ messages in thread From: Jonathan Nieder @ 2011-01-27 0:26 UTC (permalink / raw) To: Junio C Hamano Cc: Sverre Rabbelier, Git Mailing List, Ramkumar Ramachandra, Shawn O. Pearce, David Barr Junio C Hamano wrote: > Jonathan Nieder <jrnieder@gmail.com> writes: >> Hmm. I suppose on top of b2124125 (jn/fast-import-fix). > > Hmm, why not on top of v1.7.3-rc0~75^2 aka 334fba6 (Teach fast-import to > import subtrees named by tree id, 2010-06-30) then? That could work, too. ;-) I was too lazy to check if the test case happens to work in the absence of the fixes from the fast-import-fix branch. >> +++ b/fast-import.c >> @@ -2194,6 +2194,16 @@ static void file_change_m(struct branch *b) >> p = uq.buf; >> } >> >> + /* >> + * Git does not track empty, non-toplevel directories. >> + */ >> + if (S_ISDIR(mode) && >> + !memcmp(sha1, (const unsigned char *) EMPTY_TREE_SHA1_BIN, 20) && > > Do you need this cast? No, it's not needed. (EMPTY_TREE_SHA1_BIN is a string constant, originally intended for use in initializers like static const unsigned char empty_tree_sha1[20] = EMPTY_TREE_SHA1_BIN; memcmp does not care about such considerations.) ^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v4] fast-import: treat filemodify with empty tree as delete 2011-01-27 0:04 ` Junio C Hamano 2011-01-27 0:26 ` Jonathan Nieder @ 2011-01-27 6:07 ` Jonathan Nieder 2011-01-27 19:33 ` Peter Baumann 1 sibling, 1 reply; 24+ messages in thread From: Jonathan Nieder @ 2011-01-27 6:07 UTC (permalink / raw) To: Junio C Hamano Cc: Sverre Rabbelier, Git Mailing List, Ramkumar Ramachandra, Shawn O. Pearce, David Barr Normal git processes do not allow one to build a tree with an empty subtree entry without trying hard at it. This is in keeping with the general UI philosophy: git tracks content, not empty directories. v1.7.3-rc0~75^2 (2010-06-30) changed that by making it easy to include an empty subtree in fast-import's active commit: M 040000 4b825dc642cb6eb9a060e54bf8d69288fbee4904 subdir One can trigger this by reading an empty tree (for example, the tree corresponding to an empty root commit) and trying to move it to a subtree. It is better and more closely analogous to 'git read-tree --prefix' to treat such commands as requests to remove the subtree. Noticed-by: David Barr <david.barr@cordelta.com> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com> --- Junio C Hamano wrote: > Hmm, why not on top of v1.7.3-rc0~75^2 aka 334fba6 (Teach fast-import to > import subtrees named by tree id, 2010-06-30) then? Okay, I found time to try it. Some other small simplifications while at it. fast-import.c | 6 ++++++ t/t9300-fast-import.sh | 42 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+), 0 deletions(-) diff --git a/fast-import.c b/fast-import.c index ad6843a..cd9310d 100644 --- a/fast-import.c +++ b/fast-import.c @@ -2163,6 +2163,12 @@ static void file_change_m(struct branch *b) p = uq.buf; } + /* Git does not track empty, non-toplevel directories. */ + if (S_ISDIR(mode) && !memcmp(sha1, EMPTY_TREE_SHA1_BIN, 20) && *p) { + tree_content_remove(&b->branch_tree, p, NULL); + return; + } + if (S_ISGITLINK(mode)) { if (inline_data) die("Git links cannot be specified 'inline': %s", diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh index 50d5913..8487734 100755 --- a/t/t9300-fast-import.sh +++ b/t/t9300-fast-import.sh @@ -818,6 +818,48 @@ test_expect_success \ compare_diff_raw expect actual' +test_expect_success \ + 'N: delete directory by copying' \ + 'cat >expect <<-\EOF && + OBJID + :100644 000000 OBJID OBJID D foo/bar/qux + OBJID + :000000 100644 OBJID OBJID A foo/bar/baz + :000000 100644 OBJID OBJID A foo/bar/qux + EOF + empty_tree=$(git mktree </dev/null) && + cat >input <<-INPUT_END && + commit refs/heads/N-delete + committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE + data <<COMMIT + collect data to be deleted + COMMIT + + deleteall + M 100644 inline foo/bar/baz + data <<DATA_END + hello + DATA_END + C "foo/bar/baz" "foo/bar/qux" + C "foo/bar/baz" "foo/bar/quux/1" + C "foo/bar/baz" "foo/bar/quuux" + M 040000 $empty_tree foo/bar/quux + M 040000 $empty_tree foo/bar/quuux + + commit refs/heads/N-delete + committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE + data <<COMMIT + delete subdirectory + COMMIT + + M 040000 $empty_tree foo/bar/qux + INPUT_END + git fast-import <input && + git rev-list N-delete | + git diff-tree -r --stdin --root --always | + sed -e "s/$_x40/OBJID/g" >actual && + test_cmp expect actual' + test_expect_success \ 'N: modify copied tree' \ 'cat >expect <<-\EOF && :100644 100644 fcf778cda181eaa1cbc9e9ce3a2e15ee9f9fe791 fcf778cda181eaa1cbc9e9ce3a2e15ee9f9fe791 C100 newdir/interesting file3/file5 -- 1.7.4.rc3 ^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH v4] fast-import: treat filemodify with empty tree as delete 2011-01-27 6:07 ` [PATCH v4] " Jonathan Nieder @ 2011-01-27 19:33 ` Peter Baumann 2011-01-27 19:48 ` Jonathan Nieder 0 siblings, 1 reply; 24+ messages in thread From: Peter Baumann @ 2011-01-27 19:33 UTC (permalink / raw) To: Jonathan Nieder Cc: Junio C Hamano, Sverre Rabbelier, Git Mailing List, Ramkumar Ramachandra, Shawn O. Pearce, David Barr On Thu, Jan 27, 2011 at 12:07:49AM -0600, Jonathan Nieder wrote: > diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh > index 50d5913..8487734 100755 > --- a/t/t9300-fast-import.sh > +++ b/t/t9300-fast-import.sh > @@ -818,6 +818,48 @@ test_expect_success \ > compare_diff_raw expect actual' > > +test_expect_success \ > + 'N: delete directory by copying' \ > + 'cat >expect <<-\EOF && > + OBJID > + :100644 000000 OBJID OBJID D foo/bar/qux > + OBJID > + :000000 100644 OBJID OBJID A foo/bar/baz > + :000000 100644 OBJID OBJID A foo/bar/qux > + EOF > + empty_tree=$(git mktree </dev/null) && [ Feel free to ignore me ... ] Just a (stupid?) suggestion: Why not put a $EMPTY_TREE definiton in test-lib.sh (or any other global file sourced in the tests) so if another caller needs this definition it won't waste cpu cycles doing the calculation via mktree < /dev/null again? ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v4] fast-import: treat filemodify with empty tree as delete 2011-01-27 19:33 ` Peter Baumann @ 2011-01-27 19:48 ` Jonathan Nieder 2011-01-27 20:46 ` Peter Baumann 0 siblings, 1 reply; 24+ messages in thread From: Jonathan Nieder @ 2011-01-27 19:48 UTC (permalink / raw) To: Peter Baumann Cc: Junio C Hamano, Sverre Rabbelier, Git Mailing List, Ramkumar Ramachandra, Shawn O. Pearce, David Barr Peter Baumann wrote: > On Thu, Jan 27, 2011 at 12:07:49AM -0600, Jonathan Nieder wrote: >> +++ b/t/t9300-fast-import.sh >> @@ -818,6 +818,48 @@ test_expect_success \ >> compare_diff_raw expect actual' >> >> +test_expect_success \ >> + 'N: delete directory by copying' \ >> + 'cat >expect <<-\EOF && >> + OBJID >> + :100644 000000 OBJID OBJID D foo/bar/qux >> + OBJID >> + :000000 100644 OBJID OBJID A foo/bar/baz >> + :000000 100644 OBJID OBJID A foo/bar/qux >> + EOF >> + empty_tree=$(git mktree </dev/null) && > > [ Feel free to ignore me ... ] > > Just a (stupid?) suggestion: Why not put a $EMPTY_TREE definiton in test-lib.sh > (or any other global file sourced in the tests) so if another caller needs this > definition it won't waste cpu cycles doing the calculation via mktree < /dev/null > again? Might be a good idea. Note, though, that that would mean more cpu cycles used rather than less, unless we hardcode the object name (which I prefer not to do). One possibility would be a lib-object-names.sh defining EMPTY_BLOB and EMPTY_TREE to be sourced by tests that need it. ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v4] fast-import: treat filemodify with empty tree as delete 2011-01-27 19:48 ` Jonathan Nieder @ 2011-01-27 20:46 ` Peter Baumann 2011-01-27 20:48 ` Peter Baumann 2011-01-28 17:13 ` Jonathan Nieder 0 siblings, 2 replies; 24+ messages in thread From: Peter Baumann @ 2011-01-27 20:46 UTC (permalink / raw) To: Jonathan Nieder Cc: Junio C Hamano, Sverre Rabbelier, Git Mailing List, Ramkumar Ramachandra, Shawn O. Pearce, David Barr On Thu, Jan 27, 2011 at 01:48:45PM -0600, Jonathan Nieder wrote: > Peter Baumann wrote: > > On Thu, Jan 27, 2011 at 12:07:49AM -0600, Jonathan Nieder wrote: > > >> +++ b/t/t9300-fast-import.sh > >> @@ -818,6 +818,48 @@ test_expect_success \ > >> compare_diff_raw expect actual' > >> > >> +test_expect_success \ > >> + 'N: delete directory by copying' \ > >> + 'cat >expect <<-\EOF && > >> + OBJID > >> + :100644 000000 OBJID OBJID D foo/bar/qux > >> + OBJID > >> + :000000 100644 OBJID OBJID A foo/bar/baz > >> + :000000 100644 OBJID OBJID A foo/bar/qux > >> + EOF > >> + empty_tree=$(git mktree </dev/null) && > > > > [ Feel free to ignore me ... ] > > > > Just a (stupid?) suggestion: Why not put a $EMPTY_TREE definiton in test-lib.sh > > (or any other global file sourced in the tests) so if another caller needs this > > definition it won't waste cpu cycles doing the calculation via mktree < /dev/null > > again? > > Might be a good idea. Note, though, that that would mean more cpu > cycles used rather than less, unless we hardcode the object name > (which I prefer not to do). > Wny not? It *is* already hardcoded in the GIT source code (see grep -a1 cache.h output). > One possibility would be a lib-object-names.sh defining EMPTY_BLOB and > EMPTY_TREE to be sourced by tests that need it. Hm. Might be a possibility, but if this file only contains 2 hardcoded variables I would prefer putting it into test-list.sh instead of an extra file. ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v4] fast-import: treat filemodify with empty tree as delete 2011-01-27 20:46 ` Peter Baumann @ 2011-01-27 20:48 ` Peter Baumann 2011-01-28 17:13 ` Jonathan Nieder 1 sibling, 0 replies; 24+ messages in thread From: Peter Baumann @ 2011-01-27 20:48 UTC (permalink / raw) To: Jonathan Nieder Cc: Junio C Hamano, Sverre Rabbelier, Git Mailing List, Ramkumar Ramachandra, Shawn O. Pearce, David Barr On Thu, Jan 27, 2011 at 09:46:50PM +0100, Peter Baumann wrote: > On Thu, Jan 27, 2011 at 01:48:45PM -0600, Jonathan Nieder wrote: > > Peter Baumann wrote: > > > On Thu, Jan 27, 2011 at 12:07:49AM -0600, Jonathan Nieder wrote: > > > > >> +++ b/t/t9300-fast-import.sh > > >> @@ -818,6 +818,48 @@ test_expect_success \ > > >> compare_diff_raw expect actual' > > >> > > >> +test_expect_success \ > > >> + 'N: delete directory by copying' \ > > >> + 'cat >expect <<-\EOF && > > >> + OBJID > > >> + :100644 000000 OBJID OBJID D foo/bar/qux > > >> + OBJID > > >> + :000000 100644 OBJID OBJID A foo/bar/baz > > >> + :000000 100644 OBJID OBJID A foo/bar/qux > > >> + EOF > > >> + empty_tree=$(git mktree </dev/null) && > > > > > > [ Feel free to ignore me ... ] > > > > > > Just a (stupid?) suggestion: Why not put a $EMPTY_TREE definiton in test-lib.sh > > > (or any other global file sourced in the tests) so if another caller needs this > > > definition it won't waste cpu cycles doing the calculation via mktree < /dev/null > > > again? > > > > Might be a good idea. Note, though, that that would mean more cpu > > cycles used rather than less, unless we hardcode the object name > > (which I prefer not to do). > > > > Wny not? It *is* already hardcoded in the GIT source code (see grep -a1 cache.h ^ EMPTY > output). > > > One possibility would be a lib-object-names.sh defining EMPTY_BLOB and > > EMPTY_TREE to be sourced by tests that need it. > > Hm. Might be a possibility, but if this file only contains 2 hardcoded variables > I would prefer putting it into test-list.sh instead of an extra file. > -- > To unsubscribe from this list: send the line "unsubscribe git" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v4] fast-import: treat filemodify with empty tree as delete 2011-01-27 20:46 ` Peter Baumann 2011-01-27 20:48 ` Peter Baumann @ 2011-01-28 17:13 ` Jonathan Nieder 1 sibling, 0 replies; 24+ messages in thread From: Jonathan Nieder @ 2011-01-28 17:13 UTC (permalink / raw) To: Peter Baumann Cc: Junio C Hamano, Sverre Rabbelier, Git Mailing List, Ramkumar Ramachandra, Shawn O. Pearce, David Barr Peter Baumann wrote: > On Thu, Jan 27, 2011 at 01:48:45PM -0600, Jonathan Nieder wrote: >>> On Thu, Jan 27, 2011 at 12:07:49AM -0600, Jonathan Nieder wrote: >>>> + empty_tree=$(git mktree </dev/null) && [...] >> unless we hardcode the object name >> (which I prefer not to do). > > Wny not? It *is* already hardcoded in the GIT source code (see > grep -a1 EMPTY cache.h output). I think it is okay for the git implementation to rely on an implementation detail. ;-) Likewise, t0000 checks that the empty tree has id 4b825dc6. Meanwhile I would like to see people's scripts and other tests using the $(git mktree </dev/null) form, since it is more self-explanatory and avoids hardcoding an implementation detail. Of course this is not an absolute thing. Hope that helps, Jonathan Further reading: t0000-basic.h --help: Note that this test *deliberately* hard-codes many expected object IDs. When object ID computation changes, like in the previous case of swapping compression and hashing order, the person who is making the modification *should* take notice and update the test vectors here. "Tips for Writing Tests" in t/README: However, other tests that simply rely on basic parts of the core GIT working properly should not have that level of intimate knowledge of the core GIT internals. If all the test scripts hardcoded the object IDs like t0000-basic.sh does, that defeats the purpose of t0000-basic.sh, which is to isolate that level of validation in one place. Your test also ends up needing updating when such a change to the internal happens, so do _not_ do it and leave the low level of validation to t0000-basic.sh. ^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 3/3] fast-import: add 'ls' command 2011-01-03 8:01 ` [PATCH/RFC v2 0/3] " Jonathan Nieder 2011-01-03 8:22 ` [PATCH 1/3] fast-import: clarify handling of cat-blob feature Jonathan Nieder 2011-01-03 8:24 ` [PATCH 2/3] fast-import: treat filemodify with empty tree as delete Jonathan Nieder @ 2011-01-03 8:37 ` Jonathan Nieder 2011-01-26 21:39 ` [RFC] fast-import: 'cat-blob' and 'ls' commands Jonathan Nieder 3 siblings, 0 replies; 24+ messages in thread From: Jonathan Nieder @ 2011-01-03 8:37 UTC (permalink / raw) To: David Barr Cc: Git Mailing List, Ramkumar Ramachandra, Sverre Rabbelier, Shawn O. Pearce, Tomas Carnecky From: David Barr <david.barr@cordelta.com> Date: Thu, 2 Dec 2010 21:40:20 +1100 The vcs-svn library currently maintains an in-core index of all paths in all revisions. Introducing an `ls` command to fast-import would allow this responsibility to be delegated; and reading this information from the target repository instead of an in-core data structure would result in support for resuming an import partway through (i.e., incremental imports) for free. There are two forms of the 'ls' command: the two-argument form prints the entry at <path> for the tree underlying the tree, commit, or tag named by <dataref>: 'ls' SP <dataref> SP <path> LF The one-argument form prints the entry at <path> in fast-import's active commit. 'ls' SP <path> LF Output uses ls-tree format. Dirty hack: missing paths are assumed to represent the empty subtree and are printed as 040000 tree 4b825dc642cb6eb9a060e54bf8d69288fbee4904 path/to/nowhere to avoid confusing frontends that inserted such a path before. But frontends should also be prepared to accept missing path/to/nowhere from backends that (unlike git) distinguish between empty subtrees and nonentities. Signed-off-by: David Barr <david.barr@cordelta.com> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com> --- The empty tree handling is an evil hack. One of the tests illustrates the kind of operation this is meant to support. It would be easy to convince me that some other evil hack is better. This doesn't have tests for the basic functionality. Maybe there should be a new t9302-fast-import-bidi.sh so there is less to read to get started? No new "feature" for this. Frontends can easily make a feature test for themselves if they need it. ;-) And I still have plans for "feature command ls" et al, as part of a series including Tomas's simplified command dispatch. Only compile tested. (Something similar to this is very well tested but that is not enough to prevent accidents.) Changes from v1: - new documentation and demo (tests) - refactored peel-to-tree routines - mode is always 6 digits - path output uses quoting (especially important for filenames with \n [though that wouldn't come up in the svn-fe case]) - persistent buffers to avoid allocation overhead - the empty tree hackery - mode is based on type, not based on extracting the object itself - path after <dataref> does not have to be quoted - no-<dataref> form is 'ls "<path>"' instead of 'ls index "<path>"' Thanks for the original patch and a lot of help improving it go to David. 'night, Jonathan Documentation/git-fast-import.txt | 49 +++++++++++- fast-import.c | 162 ++++++++++++++++++++++++++++++++++++- t/t9300-fast-import.sh | 91 +++++++++++++++++++++ 3 files changed, 299 insertions(+), 3 deletions(-) diff --git a/Documentation/git-fast-import.txt b/Documentation/git-fast-import.txt index f56dfca..3957f70 100644 --- a/Documentation/git-fast-import.txt +++ b/Documentation/git-fast-import.txt @@ -192,7 +192,8 @@ especially when a higher level language such as Perl, Python or Ruby is being used. fast-import is very strict about its input. Where we say SP below we mean -*exactly* one space. Likewise LF means one (and only one) linefeed. +*exactly* one space. Likewise LF means one (and only one) linefeed +and HT one (and only one) horizontal tab. Supplying additional whitespace characters will cause unexpected results, such as branch names or file names with leading or trailing spaces in their name, or early termination of fast-import when it encounters @@ -330,6 +331,11 @@ and control the current import process. More detailed discussion format to the file descriptor set with `--cat-blob-fd` or `stdout` if unspecified. +`ls`:: + Causes fast-import to print a directory entry in 'ls-tree' + format to the file descriptor set with `--cat-blob-fd` or + `stdout` if unspecified. + `feature`:: Require that fast-import supports the specified feature, or abort if it does not. @@ -916,6 +922,47 @@ This command can be used anywhere in the stream that comments are accepted. In particular, the `cat-blob` command can be used in the middle of a commit but not in the middle of a `data` command. +`ls` +~~~~ +Prints a directory entry to a file descriptor previously arranged with +the `--cat-blob-fd` argument. In the current implementation, if that +entry represents a subdirectory in the current commit, it will be +stored in the object database, but it is not advisable to rely on this +detail since it maybe change. + +.... + 'ls' (SP <dataref>)? SP <path> LF +.... + +The `<dataref>` can be either a mark reference (`:<idnum>`) or a full +40-byte SHA-1 of a Git tag, commit, or tree object, preexisting or +waiting to be written. The directory entry printed is that named by +the path, relative to the top level of that tree. + +The `ls` command can be used anywhere in the stream that comments are +accepted, including the middle of a commit. + +In the middle of a `commit`, the `<dataref>` part of the command can +be omitted, in which case the path names a directory entry within +fast-import's active commit. The path must be quoted in this case. + +Output uses the same format as `git ls-tree <tree> -- <path>`: + +==== + <mode> SP ('blob' | 'tree') SP <dataref> HT <path> LF +==== + +Since git repositories do not distinguish between missing paths and +empty subtrees, if a path is not found it will be reported as an +empty tree. Backends for version control systems that do have a +notion of empty trees may write + +==== + missing SP <path> LF +==== + +for paths that do not correspond to a blob or subtree. + `feature` ~~~~~~~~~ Require that fast-import supports the specified feature, or abort if diff --git a/fast-import.c b/fast-import.c index 385d12d..21cb109 100644 --- a/fast-import.c +++ b/fast-import.c @@ -24,10 +24,12 @@ Format of STDIN stream: commit_msg ('from' sp committish lf)? ('merge' sp committish lf)* - file_change* + (file_change | ls)* lf?; commit_msg ::= data; + ls ::= 'ls' sp '"' quoted(path) '"' lf; + file_change ::= file_clr | file_del | file_rnm @@ -132,7 +134,7 @@ Format of STDIN stream: ts ::= # time since the epoch in seconds, ascii base10 notation; tz ::= # GIT style timezone; - # note: comments and cat requests may appear anywhere + # note: comments, ls and cat requests may appear anywhere # in the input, except within a data command. Any form # of the data command always escapes the related input # from comment processing. @@ -141,7 +143,9 @@ Format of STDIN stream: # must be the first character on that line (an lf # preceded it). # + cat_blob ::= 'cat-blob' sp (hexsha1 | idnum) lf; + ls_tree ::= 'ls' sp (hexsha1 | idnum) sp path_str lf; comment ::= '#' not_lf* lf; not_lf ::= # Any byte that is not ASCII newline (LF); @@ -373,6 +377,7 @@ static int cat_blob_fd = STDOUT_FILENO; static void parse_argv(void); static void parse_cat_blob(void); +static void parse_ls(struct branch *b); static void write_branch_report(FILE *rpt, struct branch *b) { @@ -2613,6 +2618,8 @@ static void parse_new_commit(void) note_change_n(b, prev_fanout); else if (!strcmp("deleteall", command_buf.buf)) file_change_deleteall(b); + else if (!prefixcmp(command_buf.buf, "ls ")) + parse_ls(b); else { unread_command_buf = 1; break; @@ -2836,6 +2843,155 @@ static void parse_cat_blob(void) cat_blob(oe, sha1); } +static struct object_entry *dereference(struct object_entry *oe, + unsigned char sha1[20]) +{ + unsigned long size; + void *buf = NULL; + if (!oe) { + enum object_type type = sha1_object_info(sha1, NULL); + if (type < 0) + die("object not found: %s", sha1_to_hex(sha1)); + /* cache it! */ + oe = insert_object(sha1); + oe->type = type; + oe->pack_id = MAX_PACK_ID; + oe->idx.offset = 1; + } + switch (oe->type) { + case OBJ_TREE: /* easy case. */ + return oe; + case OBJ_COMMIT: + case OBJ_TAG: + break; + default: + die("Not a treeish: %s", command_buf.buf); + } + + if (oe->pack_id != MAX_PACK_ID) { /* in a pack being written */ + buf = gfi_unpack_entry(oe, &size); + } else { + enum object_type unused; + buf = read_sha1_file(sha1, &unused, &size); + } + if (!buf) + die("Can't load object %s", sha1_to_hex(sha1)); + + /* Peel one layer. */ + switch (oe->type) { + case OBJ_TAG: + if (size < 40 + strlen("object ") || + get_sha1_hex(buf + strlen("object "), sha1)) + die("Invalid SHA1 in tag: %s", command_buf.buf); + break; + case OBJ_COMMIT: + if (size < 40 + strlen("tree ") || + get_sha1_hex(buf + strlen("tree "), sha1)) + die("Invalid SHA1 in commit: %s", command_buf.buf); + } + + free(buf); + return find_object(sha1); +} + +static struct object_entry *parse_treeish_dataref(const char **p) +{ + unsigned char sha1[20]; + struct object_entry *e; + + if (**p == ':') { /* <mark> */ + char *endptr; + e = find_mark(strtoumax(*p + 1, &endptr, 10)); + if (endptr == *p + 1) + die("Invalid mark: %s", command_buf.buf); + if (!e) + die("Unknown mark: %s", command_buf.buf); + *p = endptr; + hashcpy(sha1, e->idx.sha1); + } else { /* <sha1> */ + if (get_sha1_hex(*p, sha1)) + die("Invalid SHA1: %s", command_buf.buf); + e = find_object(sha1); + *p += 40; + } + + while (!e || e->type != OBJ_TREE) + e = dereference(e, sha1); + return e; +} + +static void print_ls(int mode, const unsigned char *sha1, const char *path) +{ + static struct strbuf line = STRBUF_INIT; + + /* See show_tree(). */ + const char *type = + S_ISGITLINK(mode) ? commit_type : + S_ISDIR(mode) ? tree_type : + blob_type; + + /* mode SP type SP object_name TAB path LF */ + strbuf_reset(&line); + strbuf_addf(&line, "%06o %s %s\t", + mode, type, sha1_to_hex(sha1)); + quote_c_style(path, &line, NULL, 0); + strbuf_addch(&line, '\n'); + cat_blob_write(line.buf, line.len); +} + +static void parse_ls(struct branch *b) +{ + const char *p; + struct tree_entry *root = NULL; + struct tree_entry leaf = {0}; + + /* ls SP (<treeish> SP)? <path> */ + p = command_buf.buf + strlen("ls "); + if (*p == '"') { + if (!b) + die("Not in a commit: %s", command_buf.buf); + root = &b->branch_tree; + } else { + struct object_entry *e = parse_treeish_dataref(&p); + root = new_tree_entry(); + hashcpy(root->versions[1].sha1, e->idx.sha1); + load_tree(root); + if (*p++ != ' ') + die("Missing space after tree-ish: %s", command_buf.buf); + } + if (*p == '"') { + static struct strbuf uq = STRBUF_INIT; + const char *endp; + strbuf_reset(&uq); + if (unquote_c_style(&uq, p, &endp)) + die("Invalid path: %s", command_buf.buf); + if (*endp) + die("Garbage after path in: %s", command_buf.buf); + p = uq.buf; + } + tree_content_get(root, p, &leaf); + if (!leaf.versions[1].mode) { + /* + * Missing path? Must be an empty subtree! + * + * When git learns to track empty directories, we can report + * this by saying 'missing "path/to/directory"' instead. + */ + print_ls(S_IFDIR, (const unsigned char *) EMPTY_TREE_SHA1_BIN, p); + } else { + /* + * A directory in preparation would have a sha1 of zero + * until it is saved. Save, for simplicity. + */ + if (S_ISDIR(leaf.versions[1].mode)) + store_tree(&leaf); + + print_ls(leaf.versions[1].mode, leaf.versions[1].sha1, p); + } + if (!b || root != &b->branch_tree) + release_tree_entry(root); +} + static void checkpoint(void) { checkpoint_requested = 0; @@ -3131,6 +3287,8 @@ int main(int argc, const char **argv) while (read_next_command() != EOF) { if (!strcmp("blob", command_buf.buf)) parse_new_blob(); + else if (!prefixcmp(command_buf.buf, "ls ")) + parse_ls(NULL); else if (!prefixcmp(command_buf.buf, "commit ")) parse_new_commit(); else if (!prefixcmp(command_buf.buf, "tag ")) diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh index b9aa3f0..6842b1f 100755 --- a/t/t9300-fast-import.sh +++ b/t/t9300-fast-import.sh @@ -906,6 +906,97 @@ test_expect_success \ git diff-tree -C --find-copies-harder -r N4^ N4 >actual && compare_diff_raw expect actual' +test_expect_success PIPE 'N: read and copy directory' ' + cat >expect <<-\EOF + :100755 100755 f1fb5da718392694d0076d677d6d0e364c79b0bc f1fb5da718392694d0076d677d6d0e364c79b0bc C100 file2/newf file3/newf + :100644 100644 7123f7f44e39be127c5eb701e5968176ee9d78b1 7123f7f44e39be127c5eb701e5968176ee9d78b1 C100 file2/oldf file3/oldf + EOF + git update-ref -d refs/heads/N4 && + rm -f backflow && + mkfifo backflow && + ( + exec <backflow && + cat <<-EOF && + commit refs/heads/N4 + committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE + data <<COMMIT + copy by tree hash, part 2 + COMMIT + + from refs/heads/branch^0 + ls "file2" + EOF + read mode type tree filename && + echo "M 040000 $tree file3" + ) | + git fast-import --cat-blob-fd=3 3>backflow && + git diff-tree -C --find-copies-harder -r N4^ N4 >actual && + compare_diff_raw expect actual +' + +test_expect_success PIPE 'N: read and copy "empty" directory' ' + cat <<-\EOF >expect && + OBJNAME + :000000 100644 OBJNAME OBJNAME A greeting + OBJNAME + :100644 000000 OBJNAME OBJNAME D unrelated + OBJNAME + :000000 100644 OBJNAME OBJNAME A unrelated + EOF + git update-ref -d refs/heads/copy-empty && + rm -f backflow && + mkfifo backflow && + ( + exec <backflow && + cat <<-EOF && + commit refs/heads/copy-empty + committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE + data <<COMMIT + copy "empty" (missing) directory + COMMIT + + M 100644 inline src/greeting + data <<BLOB + hello + BLOB + C src/greeting dst1/non-greeting + C src/greeting unrelated + # leave behind "empty" src directory + D src/greeting + ls "src" + EOF + read mode type tree filename && + sed -e "s/X\$//" <<-EOF + M $mode $tree dst1 + M $mode $tree dst2 + + commit refs/heads/copy-empty + committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE + data <<COMMIT + copy empty directory to root + COMMIT + + M $mode $tree X + + commit refs/heads/copy-empty + committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE + data <<COMMIT + add another file + COMMIT + + M 100644 inline greeting + data <<BLOB + hello + BLOB + EOF + ) | + git fast-import --cat-blob-fd=3 3>backflow && + git rev-list copy-empty | + git diff-tree -r --root --stdin | + sed "s/$_x40/OBJNAME/g" >actual && + test_cmp expect actual +' + test_expect_success \ 'N: delete directory by copying' \ 'cat >expect <<-\EOF && -- 1.7.4.rc0.580.g89dc.dirty ^ permalink raw reply related [flat|nested] 24+ messages in thread
* [RFC] fast-import: 'cat-blob' and 'ls' commands 2011-01-03 8:01 ` [PATCH/RFC v2 0/3] " Jonathan Nieder ` (2 preceding siblings ...) 2011-01-03 8:37 ` [PATCH 3/3] fast-import: add 'ls' command Jonathan Nieder @ 2011-01-26 21:39 ` Jonathan Nieder 2011-01-26 23:46 ` Sam Vilain 3 siblings, 1 reply; 24+ messages in thread From: Jonathan Nieder @ 2011-01-26 21:39 UTC (permalink / raw) To: vcs-fast-import-devs Cc: David Barr, Git Mailing List, Ramkumar Ramachandra, Sverre Rabbelier, Shawn O. Pearce, Tomas Carnecky, Sam Vilain Hi fast importers, I would like your thoughts on a few developments in fast-import protocol (thanks to David, Ram, Sverre, Tomas, and Sam for work so far). If they seem good, I'd be happy to help make patches to other backends so these can be implemented widely. Contents: cat-blob command, filemodify (M) with trees, ls command. cat-blob command ---------------- fast-import 1.7.4-rc0 added a new "cat-blob" feature. It is meant to allow exporters that receiving changes in delta form to avoid having to remember the full text of blobs already exported or re-retrieve them from the source repository. It works like this: 1. Out of band, the fast-import frontend and backend negotiate a channel for the backend to send replies to the frontend. In git fast-import, this is a file descriptor, defaulting to stdout. So you can do: mkfifo replies && $frontend <replies | git fast-import --cat-blob-fd=3 3>replies The intent is that stdin would typically be a socket and this file descriptor would point to that. 2. The frontend (optionally) declares use of this feature by putting feature cat-blob at the beginning of the stream. 3. When the frontend needs a previously exported blob to use as delta preimage, it uses the cat-blob command. cat-blob :3 The backend replies with something like 7c8987a987ca98c blob 6 hello More precisely, the output format is <dataref> SP 'blob' SP <length> LF <full text of blob> LF The <dataref> can be any text not including whitespace. The frontend can rely on a little buffering if it wants to print a command after the "cat-blob", but it must read the reply in its entirety if it expects the backend to act on later commands. In other words, the cat-blob command is not guaranteed to be asynchronous. This protocol is used by the svn-fe[1] tool to handle Subversion dump files in version 3 (--deltas) format and seems to work ok. Does this look sane or does it need tweaking or more detailed specification to be widely useful? Even once git 1.7.4 is out, it should be possible to make improvements using a new "feature" name. filemodify (M) with trees ------------------------- fast-import 1.7.3-rc0 introduced the ability for a filemodify (M) command to place a tree named by mark or other <dataref> at a given path, replacing whatever was there before. The implementation had some kinks, which fast-import 1.7.4-rc0 ironed out. Without some way to specify marks or learn tree names out of band, it is not very useful. With some way to learn tree names, it can be used, for example, to rewrite revision metadata while reusing the old tree data: commit refs/heads/master mark :11 committer A U Thor <author@example.com> Wed, 26 Jan 2011 15:14:11 -0600 data <<EOF New change description EOF M 040000 4b825dc642cb6eb9a060e54bf8d69288fbee4904 "" There is no "feature" name for this. Corner case: a command to replace a path with the empty tree is interpreted[2] as meaning to remove that file or subtree, because git does not track empty directories. Do the semantics seem reasonable? Should this get a corresponding "feature"? ls command ---------- A patch in flight[3] introduces an "ls" command to read directory entries from the active commit or a named commit. This allows printing a blob from the active commit or copying a blob or tree from a previous commit for use in the current one. It works like so: 1. Frontend writes 'ls' SP <path> LF or 'ls' SP <dataref> SP <path> LF In the first form, the <path> _must_ be surrounded in quotes and quoted C-style. In the second form, the <dataref> can refer to a tag, commit, or tree. 2. Backend replies through the cat-blob channel: <mode> SP <type> SP <dataref> HT <path> LF <mode> is a 6-digit octal mode: 040000, 100644, 100755, 120000, or 160000 for a directory, regular file, executable file, symlink, or submodule, respectively. <type> is 'blob', 'tree', or 'commit'. <dataref> represents the corresponding blob, tree, or commit object. <path> is the path in question. It can be quoted C-style and must be if the path starts with '"' or contains a newline. 3. Frontend reads the reply. The frontend might use that <dataref> in a later filemodify (M) and cat-blob command. Proposed updates to svn-fe[1] use this heavily and work well. One ugly corner case: although it is intended to allow "missing <path>" as a reply when the path is missing, the proposed patch makes git fast-import use an empty tree to signal that case, to ensure that, for example, ls "" M <mode> <dataref> "" is always a non-operation. No "feature" name yet. Even better, it's not part of git yet so I invite to nitpick to your heart's content. Maybe you'd rather the command be called "ls-tree" instead of "ls"? Ask away. :) Thoughts welcome, as always. Jonathan [1] http://repo.or.cz/w/git/jrn.git/blob/refs/heads/vcs-svn-pu:/vcs-svn/svndump.c [2] Or rather, is not interpreted but ought to be, or else fast-import will make it too easy to produce invalid commits. One of the patches in series [3] fixes it. [3] http://thread.gmane.org/gmane.comp.version-control.git/162698/focus=164448 ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [RFC] fast-import: 'cat-blob' and 'ls' commands 2011-01-26 21:39 ` [RFC] fast-import: 'cat-blob' and 'ls' commands Jonathan Nieder @ 2011-01-26 23:46 ` Sam Vilain 0 siblings, 0 replies; 24+ messages in thread From: Sam Vilain @ 2011-01-26 23:46 UTC (permalink / raw) To: Jonathan Nieder Cc: vcs-fast-import-devs, David Barr, Git Mailing List, Ramkumar Ramachandra, Sverre Rabbelier, Shawn O. Pearce, Tomas Carnecky On 27/01/11 07:39, Jonathan Nieder wrote: > Hi fast importers, > > I would like your thoughts on a few developments in fast-import > protocol (thanks to David, Ram, Sverre, Tomas, and Sam for work so > far). If they seem good, I'd be happy to help make patches to other > backends so these can be implemented widely. > > Contents: cat-blob command, filemodify (M) with trees, ls command. Ok. My first thoughts here are to be careful about the design: this fast-import protocol is fast becoming close to getting an RFC, having multiple interoperable implementations available, so do consider whether all syntax will be cleanly extensible to eventually support full basic plumbing requirements. ie, using the command 'cat-blob' instead of a 'cat' command with 'blob' as an argument as git cat-file currently works seems to be an inflexibility and may eventually be considered legacy. Otherwise it looks fine, seems to support all the file types etc. Thanks for keeping the work up! Cheers, Sam ^ permalink raw reply [flat|nested] 24+ messages in thread
end of thread, other threads:[~2011-01-28 17:14 UTC | newest] Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2010-12-02 10:40 [PATCH/RFC] fast-import: add 'ls' command David Barr 2010-12-02 10:40 ` [PATCH] " David Barr 2010-12-02 12:37 ` Sverre Rabbelier 2010-12-02 12:57 ` David Michael Barr 2010-12-02 17:37 ` Jonathan Nieder 2010-12-02 19:20 ` Junio C Hamano 2010-12-02 22:51 ` David Barr 2011-01-03 8:01 ` [PATCH/RFC v2 0/3] " Jonathan Nieder 2011-01-03 8:22 ` [PATCH 1/3] fast-import: clarify handling of cat-blob feature Jonathan Nieder 2011-01-03 8:24 ` [PATCH 2/3] fast-import: treat filemodify with empty tree as delete Jonathan Nieder 2011-01-26 22:41 ` [PATCH v2] " Jonathan Nieder 2011-01-26 22:45 ` Sverre Rabbelier 2011-01-26 23:06 ` [PATCH jn/fast-import-fix v3] " Jonathan Nieder 2011-01-27 0:04 ` Junio C Hamano 2011-01-27 0:26 ` Jonathan Nieder 2011-01-27 6:07 ` [PATCH v4] " Jonathan Nieder 2011-01-27 19:33 ` Peter Baumann 2011-01-27 19:48 ` Jonathan Nieder 2011-01-27 20:46 ` Peter Baumann 2011-01-27 20:48 ` Peter Baumann 2011-01-28 17:13 ` Jonathan Nieder 2011-01-03 8:37 ` [PATCH 3/3] fast-import: add 'ls' command Jonathan Nieder 2011-01-26 21:39 ` [RFC] fast-import: 'cat-blob' and 'ls' commands Jonathan Nieder 2011-01-26 23:46 ` Sam Vilain
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.