Re: [PATCH] fast-import: fix lookup for new commits

[Aniket Gupta <aniketgupta3001@gmail.com>]

Hello Marc,

Sorry for the noise, my first mail lost its In-Reply-To header on the
way, so it is not in this thread. Resending the same review here.

I tried this on top of 6de20f609 with DEVELOPER=1 and did not get warnings.

> Queries for commits created during the same 'git fast-import'  session
> result in a crash:
>     < reset refs/heads/newbranch
>     < from <new-commit-oid>
>     > fatal: not a valid commit: <new-commit-oid>

I was able to repro this. With the patch fsck is clean and it works
nicely. Same goes for "commit" with "from <new-oid>", a tag made in
the same session, and a tag of a tag. Existing commits and tags and
"from <blob>" behave like before.

One small nitpick: there is a double space in "'git fast-import'  session".

> to avoid crash and (eventually) improve lookup speed.

Did you measure this? If not, maybe drop it or say it is only a
possible side effect.

> +static struct object_entry *dereference(struct object_entry *oe, struct object_id *oid);

This is over 80 columns, can it be wrapped like the definition?

>  builtin/fast-import.c | 18 ++++++++++++++----

I could not find a test for this. Can you add one to t9300? You can
get the new oid with get-mark and use it in a "reset ... from <oid>"
in the same stream.

Thanks,
Aniket