Julian Phillips <julian@xxxxxxxxxxxxxxxxx> wrote: > This adds the ability to copy a path from an arbitrary commit instead > of just from the current commit. This is done using the 'K' version > of the filecopy sub-command. So the reason I didn't apply this earlier when I was playing the role of "interim maintainer" was four fold: - There's no test cases for this new 'K' command. Everything else in fast-import has at least one test case for it, this should as well. - There's a huge memory leak every time you use the 'K' command. See below in the code for where. - The documentation doesn't talk about the special "/" source path. - I was really busy and just forgot to reply to this email. Other than that I think this is good. I've just been too busy to do it myself, and I'm glad that you took it upon yourself to remedy that. > diff --git a/Documentation/git-fast-import.txt b/Documentation/git-fast-import.txt > index d511967..89aefea 100644 > --- a/Documentation/git-fast-import.txt > +++ b/Documentation/git-fast-import.txt > @@ -534,6 +534,17 @@ location has been copied to the destination any future commands > applied to the source location will not impact the destination of > the copy. > > +It is also possible to copy a file or directory from a commit other > +than the current one. > + > +.... > + 'K' <dataref> SP <path> SP <path> LF > +.... > + > +where `<dataref>` is a reference (see `filemodify` above) to the > +commit that the first (source) `<path>` is located in, the second > +`<path>` is the destination. > + This should talk about the special case of the first (src) <path> being able to be "/" to copy the entire tree. If I follow your implementation right a src_path of "/" also causes the destination path to be ignored entirely. Which means you cannot copy another branch into a subdirectory of this branch (aka one direction of the subtree merge strategy). > diff --git a/fast-import.c b/fast-import.c > index e9c80be..9537c63 100644 > --- a/fast-import.c > +++ b/fast-import.c ... > +static void copy_from_commit( > + unsigned char *src_commit, const char *src_path, > + struct branch *dest_br, const char *dest_path) ... > + load_tree(&src_br.branch_tree); This is a huge memory leak. You never unload this tree and return its data onto the freelists so we'll leak everything that wasn't moved into the destination tree. -- Shawn. - To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
