Git development
 help / color / mirror / Atom feed
* [PATCH] fast-import: fix lookup for new commits
@ 2026-10-10 17:24 Marc Becker via GitGitGadget
  2026-10-11 19:38 ` Aniket Gupta
  2026-10-12 22:50 ` [PATCH v2] fast-import: fix lookup for new commits and tags Marc Becker via GitGitGadget
  0 siblings, 2 replies; 6+ messages in thread
From: Marc Becker via GitGitGadget @ 2026-10-10 17:24 UTC (permalink / raw)
  To: git; +Cc: Marc Becker, Marc Becker

From: Marc Becker <becm@gmx.de>

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>

Getting file content for commit has no issue (working implementation):
    < ls <new-commit-oid> file-in-new-commit.txt
    > {file content}

During import, new objects may not (yet) be reachable via the ODB layer.
Try to resolve requested tag/commit via object cache first
to avoid crash and (eventually) improve lookup speed.

Signed-off-by: Marc Becker <becm@gmx.de>
---
    fast-import: fix lookup for new commits

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2252%2Fbecm%2Ffix-fast-import-commit-from-existing-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2252/becm/fix-fast-import-commit-from-existing-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/2252

 builtin/fast-import.c | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/builtin/fast-import.c b/builtin/fast-import.c
index fbd919982c..4b93148551 100644
--- a/builtin/fast-import.c
+++ b/builtin/fast-import.c
@@ -292,6 +292,8 @@ static void fast_import_state_init(struct fast_import_state *state,
 	state->option = option;
 }
 
+static struct object_entry *dereference(struct object_entry *oe, struct object_id *oid);
+
 static void parse_argv(struct fast_import_state *state);
 static void parse_get_mark(struct fast_import_state *state, const char *p);
 static void parse_cat_blob(struct fast_import_state *state, const char *p);
@@ -2658,12 +2660,20 @@ static void parse_from_existing(struct branch *b)
 		oidclr(&b->branch_tree.versions[1].oid, the_repository->hash_algo);
 	} else {
 		unsigned long size;
-		size_t size_st = 0;
 		char *buf;
+		struct object_entry *e = find_object(&b->oid);
 
-		buf = odb_read_object_peeled(the_repository->objects, &b->oid,
-					     OBJ_COMMIT, &size_st, &b->oid);
-		size = cast_size_t_to_ulong(size_st);
+		while (e && e->type == OBJ_TAG)
+			e = dereference(e, &b->oid);
+
+		if (e && e->type == OBJ_COMMIT && e->pack_id != MAX_PACK_ID) {
+			buf = gfi_unpack_entry(e, &size);
+		} else {
+			size_t size_st = 0;
+			buf = odb_read_object_peeled(the_repository->objects, &b->oid,
+						     OBJ_COMMIT, &size_st, &b->oid);
+			size = cast_size_t_to_ulong(size_st);
+		}
 		parse_from_commit(b, buf, size);
 		free(buf);
 	}

base-commit: 6de20f6092dcf9bdb1c8efe03db4b70c82b423dd
-- 
gitgitgadget

^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [PATCH] fast-import: fix lookup for new commits
@ 2026-10-11 19:22 Aniket Gupta
  0 siblings, 0 replies; 6+ messages in thread
From: Aniket Gupta @ 2026-10-11 19:22 UTC (permalink / raw)
  To: Marc Becker; +Cc: git

Hello Marc,

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

^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-10-13  1:40 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-10 17:24 [PATCH] fast-import: fix lookup for new commits Marc Becker via GitGitGadget
2026-10-11 19:38 ` Aniket Gupta
2026-10-12 22:50 ` [PATCH v2] fast-import: fix lookup for new commits and tags Marc Becker via GitGitGadget
2026-10-13  0:08   ` Junio C Hamano
2026-10-13  1:39   ` Junio C Hamano
  -- strict thread matches above, loose matches on Subject: below --
2026-10-11 19:22 [PATCH] fast-import: fix lookup for new commits Aniket Gupta

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox