Felipe Contreras <felipe.contreras@xxxxxxxxx> wrote:
> This is useful in certain SCMs like monotone, where each 'merge revision' has
> the changes of all the micro-branches merged. So it appears as duplicated commands.
>
> The delete command was ignoring the issue completely. The rename/copy commands
> where throwing a fatal exception.
Signed-off-by line? See Documentation/SubmittingPatches.
> diff --git a/fast-import.c b/fast-import.c
> index 7089e6f..3dd2ab6 100644
> --- a/fast-import.c
> +++ b/fast-import.c
> @@ -1952,7 +1953,13 @@ static void file_change_d(struct branch *b)
> die("Garbage after path in: %s", command_buf.buf);
> p = uq.buf;
> }
> - tree_content_remove(&b->branch_tree, p, NULL);
> + memset(&leaf, 0, sizeof(leaf));
> + tree_content_remove(&b->branch_tree, p, &leaf);
> + if (!leaf.versions[1].mode)
> + {
> + warning("Path %s not in branch", p);
> + return;
> + }
> }
>
> static void file_change_cr(struct branch *b, int rename)
This is going to leak memory unless you add this before the
if (..mode) condition:
if (leaf->tree)
release_tree_content_recursive(e->tree);
We didn't worry about deleting a path that doesn't exist because
the importer clearly wants it gone. If it wants it gone and it is
already gone then it should be fine to ignore the delete command.
But as I point out below some import front-ends should be accurate
enough that they should not send a 'D' command unless the path is
already in the tree. Thus this can be an error condition for some
types of frontends, but can be ignored for others.
> @@ -1994,7 +2001,10 @@ static void file_change_cr(struct branch *b, int rename)
> else
> tree_content_get(&b->branch_tree, s, &leaf);
> if (!leaf.versions[1].mode)
> - die("Path %s not in branch", s);
> + {
> + warning("Path %s not in branch", s);
> + return;
> + }
> tree_content_set(&b->branch_tree, d,
> leaf.versions[1].sha1,
> leaf.versions[1].mode,
Normally we consider invalid paths to be an error. I wonder if this
should still be an error, unless the front-end passes an option on
the command line. Then monotone based importers can make these
warnings, but other importers that don't have this problem can
still treat them what they are, which is a fatal error.
Did you run the test suite (t/t9300-fast-import.sh) after your patch?
I would have thought a few of the bad path errors should be caught
there.
--
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
