Re: [PATCH 3/3] fast-export: output reset command for commandline revs

[Felipe Contreras <felipe.contreras@gmail.com>]

On Sun, Nov 6, 2011 at 12:23 AM, Sverre Rabbelier <srabbelier@gmail.com> wrote:
> When a revision is specified on the commandline we explicitly output
> a 'reset' command for it if it was not handled already. This allows
> for example the remote-helper protocol to use fast-export to create
> branches that point to a commit that has already been exported.

This simpler patch does the same, doesn't it?

diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index 12220ad..3b4c2d6 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -523,10 +523,13 @@ static void get_tags_and_duplicates(struct
object_array *pending,
                                typename(e->item->type));
                        continue;
                }
-               if (commit->util)
-                       /* more than one name for the same object */
+               /*
+                * This ref will not be updated through a commit, lets make
+                * sure it gets properly updated eventually.
+                */
+               if (commit->util || commit->object.flags & SHOWN)
                        string_list_append(extra_refs,
full_name)->util = commit;
-               else
+               if (!commit->util)
                        commit->util = full_name;
        }
 }

> Initial-patch-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
> Signed-off-by: Sverre Rabbelier <srabbelier@gmail.com>
> ---
>
>   Most of the hard work for this patch was done by Dscho. The rest of
>   it was basically me applying the technique used by jch in c3502fa
>   (25-08-2011 do not include sibling history in --ancestry-path).
>
>   The if statement dealing with tag_of_filtered_mode is not as
>   elegant as either me or Dscho would have liked, but we couldn't
>   find a better way to determine if a ref is a tag at this point
>   in the code.

Which is needed why?

Right now if I do:
% git fast-export --{im,ex}port-marks=/tmp/marks foo1 tag-to-foo1

Where tag-to-foo1 is a tag that that points to foo1, I get a reset for that.

>   Additionally, the elem->whence != REV_CMD_RIGHT case should really
>   check if REV_CMD_RIGHT_REF, but as this is not provided by the
>   ref_info structure this is left as is. A result of this is that
>   incorrect input will result in incorrect output, rather than an
>   error message. That is: `git fast-export a..<sha1>` will
>   incorrectly generate a `reset <sha1>` statement in the fast-export
>   stream.

I don't see the point of this.

Besides, you can check the return value of dwim_ref, if it's not 1,
then you shouldn't generate a reset.

>   The dwim_ref bit is a double work (it has already been done by the
>   caller of this function), but I decided it would be more work to
>   pass this information along than to recompute it for the few
>   commandline refs that were relevant.

It's already stored in commit->util, you don't need to do that.

As I said, I think the patch above does the trick, and it even has the
advantage of not having the above a..<SHA-1> issues.

Cheers.

-- 
Felipe Contreras