Re: [PATCH 2/3] fast-export: do not refer to non-existing marks
- From
Jonathan Nieder <jrnieder@gmail.com>
- Date
- Nov 6, 2011, 04:45 UTC
- Message-ID
- <20111106044514.GN27272@elie.hsd1.il.comcast.net>
- In-Reply-To
- <1320535407-4933-3-git-send-email-srabbelier@gmail.com>
Hi,
Sverre Rabbelier wrote:
> When calling `git fast-export a..a b` when a and b refer to the same
> commit, nothing would be exported, and an incorrect reset line would
> be printed for b ('from :0').Hm, seems problematic indeed.
> Extract a handle_reset function that deals with this, which can then > be re-used in a later commit.
So, does this patch drop the confusing behavior and add one that is more intuitive for remote helpers? It's not clear from this description what sort of deal the patch makes and whether it is a good or bad one.
[...]
Show 7 quoted lines
> --- a/builtin/fast-export.c > +++ b/builtin/fast-export.c > @@ -529,9 +529,20 @@ static void get_tags_and_duplicates(struct object_array *pending, > } > } > > +static void handle_reset(const char *name, struct object *object)
Nit: the other handle_* functions are about acting on objects encountered during revision traversal from the object store. In other words, the things being handled are the git objects.
By contrast, this function is about writing a "reset" command to the fast-import stream. I'd be tempted to call it reset_ref() or something like that.
Show 12 quoted lines
> +{
> + int mark = get_object_mark(object);
> +
> - commit = (struct commit *)object;
> - printf("reset %s\nfrom :%d\n\n", name,
> - get_object_mark(&commit->object));
> + if (mark)
> + printf("reset %s\nfrom :%d\n\n", name,
> + get_object_mark(object));
> + else
> + printf("reset %s\nfrom %s\n\n", name,
> + sha1_to_hex(object->sha1));Ah --- the functional change is to use a sha1 when there is no mark corresponding to the object.
Why is this codepath being run at all when b is excluded by the revision range (a..a a = ^a a a)? Is this the same bug tested for in patch 1/3 or something separate?