Re: [PATCH] fast-import: Add ability to copy a path from an arbitrary commit
From: Shawn O. Pearce <hidden>
Date: 2016-06-15 22:43:44
Julian Phillips [off-list ref] 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.
quoted hunk
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).
quoted hunk
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.