All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.