On Thu, Apr 10, 2025 at 06:19:32PM +0200, Stefan Sperling wrote:
> I noticed this while investigating the tog issue reported by xs on IRC.
> 
> Opening a bunch of files in /tmp for pack temp files and opening the
> repository just to validate the work tree's base-commit ID is overkill.
> Simply parse the ID directly. If the commit object no longer exists we
> will run into another error sooner or later. The commit is already
> protected by the work tree reference. If it is missing then something
> has badly gone wrong.
> 
> ok?

Here is an alternative fix which obtains the hash algorithm from the
Git config file in the repository.  Omar pointed out on IRC that this
might be better than guessing an algorithm based on strlen().

I don't mind either way, both approaches work for me.

The strlen() approach makes 'got status' on my got work tree a bit
faster (0.05 seconds vs 0.10 seconds). But I do not actually care
about saving time at such a small scale.


M  lib/worktree_open.c  |  16+  15-

1 file changed, 16 insertions(+), 15 deletions(-)

commit - 288f62bc8681fd4d88d574527fe45052a7d641b7
commit + 05215130761193aab0b03ffe941ad82ed9585e7e
blob - 74e2affe2cf926ca05691a6b59ec0592908b3f12
blob + 24c199dd19ba921a3ada632314946180a9803fb0
--- lib/worktree_open.c
+++ lib/worktree_open.c
@@ -41,6 +41,7 @@
 
 #include "got_lib_worktree.h"
 #include "got_lib_gotconfig.h"
+#include "got_lib_hash.h"
 
 #ifndef nitems
 #define nitems(_a)	(sizeof((_a)) / sizeof((_a)[0]))
@@ -124,14 +125,18 @@ open_worktree(struct got_worktree **worktree, const ch
 	char *uuidstr = NULL;
 	char *path_lock = NULL;
 	char *base_commit_id_str = NULL;
+	struct got_object_id *base_commit_id = NULL;
 	int version, fd = -1;
 	const char *errstr;
 	struct got_repository *repo = NULL;
-	int *pack_fds = NULL;
 	uint32_t uuid_status;
 
 	*worktree = NULL;
 
+	base_commit_id = calloc(1, sizeof(*base_commit_id));
+	if (base_commit_id == NULL)
+		return got_error_from_errno("calloc");
+
 	if (asprintf(&path_meta, "%s/%s", path, meta_dir) == -1) {
 		err = got_error_from_errno("asprintf");
 		path_meta = NULL;
@@ -204,18 +209,19 @@ open_worktree(struct got_worktree **worktree, const ch
 		goto done;
 	}
 
-	err = got_repo_pack_fds_open(&pack_fds);
+	err = got_repo_open(&repo, (*worktree)->repo_path, NULL, NULL);
 	if (err)
 		goto done;
 
-	err = got_repo_open(&repo, (*worktree)->repo_path, NULL, pack_fds);
-	if (err)
-		goto done;
+	if (!got_parse_object_id(base_commit_id, base_commit_id_str,
+	    got_repo_get_object_format(repo))) {
+		err = got_error_fmt(GOT_ERR_BAD_OBJ_ID_STR,
+		    "%s/%s", meta_dir, GOT_WORKTREE_BASE_COMMIT);
+ 		goto done;
+	}
 
-	err = got_object_resolve_id_str(&(*worktree)->base_commit_id, repo,
-	    base_commit_id_str);
-	if (err)
-		goto done;
+	(*worktree)->base_commit_id = base_commit_id;
+	base_commit_id = NULL;
 
 	err = read_meta_file(&(*worktree)->head_ref_name, path_meta,
 	    GOT_WORKTREE_HEAD_REF);
@@ -246,15 +252,10 @@ done:
 		if (err == NULL)
 			err = close_err;
 	}
-	if (pack_fds) {
-		const struct got_error *pack_err =
-		    got_repo_pack_fds_close(pack_fds);
-		if (err == NULL)
-			err = pack_err;
-	}
 	free(path_meta);
 	free(path_lock);
 	free(base_commit_id_str);
+	free(base_commit_id);
 	free(uuidstr);
 	free(formatstr);
 	if (err) {