Download raw body.
got_object_id_by_path() vs. commit object cache
Stefan Sperling <stsp@stsp.name> wrote:
> On Tue, Apr 05, 2022 at 06:12:59PM +0200, Stefan Sperling wrote:
> > Stop relying on the commit object cache for reasoable performance
> > of got_object_id_by_path(). This function is called over and over
> > on the same commit by commands like 'got tree -R'.
> >
> > Instead of internally opening and closing the same commit object over and
> > over again, require callers to pass an open commit object in. Avoids an
> > inherent dependency on the commit object cache for reasonable performance
> > of this function.
> >
> > My end goal is to eventually get rid of the commit object cache altogether
> > because this cache seems to be slowing down operations like 'gotadmin pack'.
> > The cache is only useful when commits are accessed repeatedly and very often,
> > which is rarely the case.
> > Usually, operations open one commit as an anchor to start from, or they open
> > commits in sequence and do not process a given commit more than once.
> > In such cases the object cache management overhead provides no benefit and
> > just slows things down.
> >
> > ok?
>
> New version which includes changes to gotweb.c I forgot in the previous diff.
> The previous version broke the build in 'make web'.
I don't know much about the commit object cache, but agree on the idea:
commits are traversed in sequence and caching them when they won't be
re-used is just a waste. The diff reads fine to me.
> diff 720c84593b989629674c0c2d566b6fcb246578fd 6fd1a33c932d168a9aef98ca7760e8f63446bea1
> blob - f3608d970c5ae77fee89a4e9ca8fdb89b015c5f8
> blob + 7ac9d84ea1bc903f4fd7ab569799630bbaa48aaa
> --- got/got.c
> +++ got/got.c
> @@ -3609,7 +3609,7 @@ print_patch(struct got_commit_object *commit, struct g
>
> if (path && path[0] != '\0') {
> int obj_type;
> - err = got_object_id_by_path(&obj_id2, repo, id, path);
> + err = got_object_id_by_path(&obj_id2, repo, commit, path);
> if (err)
> goto done;
> err = got_object_id_str(&id_str2, obj_id2);
> @@ -3619,7 +3619,7 @@ print_patch(struct got_commit_object *commit, struct g
> }
> if (pcommit) {
> err = got_object_id_by_path(&obj_id1, repo,
> - qid->id, path);
> + pcommit, path);
> if (err) {
> if (err->code != GOT_ERR_NO_TREE_ENTRY) {
> free(obj_id2);
> @@ -4984,6 +4984,7 @@ cmd_blame(int argc, char *argv[])
> char *link_target = NULL;
> struct got_object_id *obj_id = NULL;
> struct got_object_id *commit_id = NULL;
> + struct got_commit_object *commit = NULL;
> struct got_blob_object *blob = NULL;
> char *commit_id_str = NULL;
> struct blame_cb_args bca;
> @@ -5111,12 +5112,16 @@ cmd_blame(int argc, char *argv[])
> worktree = NULL;
> }
>
> + error = got_object_open_as_commit(&commit, repo, commit_id);
> + if (error)
> + goto done;
> +
> error = got_object_resolve_symlinks(&link_target, in_repo_path,
> - commit_id, repo);
> + commit, repo);
> if (error)
> goto done;
>
> - error = got_object_id_by_path(&obj_id, repo, commit_id,
> + error = got_object_id_by_path(&obj_id, repo, commit,
> link_target ? link_target : in_repo_path);
> if (error)
> goto done;
> @@ -5171,6 +5176,8 @@ done:
> free(cwd);
> free(commit_id);
> free(obj_id);
> + if (commit)
> + got_object_commit_close(commit);
> if (blob)
> got_object_blob_close(blob);
> if (worktree)
> @@ -5246,7 +5253,7 @@ print_entry(struct got_tree_entry *te, const char *id,
> }
>
> static const struct got_error *
> -print_tree(const char *path, struct got_object_id *commit_id,
> +print_tree(const char *path, struct got_commit_object *commit,
> int show_ids, int recurse, const char *root_path,
> struct got_repository *repo)
> {
> @@ -5255,7 +5262,7 @@ print_tree(const char *path, struct got_object_id *com
> struct got_tree_object *tree = NULL;
> int nentries, i;
>
> - err = got_object_id_by_path(&tree_id, repo, commit_id, path);
> + err = got_object_id_by_path(&tree_id, repo, commit, path);
> if (err)
> goto done;
>
> @@ -5297,7 +5304,7 @@ print_tree(const char *path, struct got_object_id *com
> err = got_error_from_errno("asprintf");
> goto done;
> }
> - err = print_tree(child_path, commit_id, show_ids, 1,
> + err = print_tree(child_path, commit, show_ids, 1,
> root_path, repo);
> free(child_path);
> if (err)
> @@ -5320,6 +5327,7 @@ cmd_tree(int argc, char *argv[])
> const char *path, *refname = NULL;
> char *cwd = NULL, *repo_path = NULL, *in_repo_path = NULL;
> struct got_object_id *commit_id = NULL;
> + struct got_commit_object *commit = NULL;
> char *commit_id_str = NULL;
> int show_ids = 0, recurse = 0;
> int ch;
> @@ -5459,13 +5467,19 @@ cmd_tree(int argc, char *argv[])
> worktree = NULL;
> }
>
> - error = print_tree(in_repo_path, commit_id, show_ids, recurse,
> + error = got_object_open_as_commit(&commit, repo, commit_id);
> + if (error)
> + goto done;
> +
> + error = print_tree(in_repo_path, commit, show_ids, recurse,
> in_repo_path, repo);
> done:
> free(in_repo_path);
> free(repo_path);
> free(cwd);
> free(commit_id);
> + if (commit)
> + got_object_commit_close(commit);
> if (worktree)
> got_worktree_close(worktree);
> if (repo) {
> @@ -11830,6 +11844,7 @@ cmd_cat(int argc, char *argv[])
> char *cwd = NULL, *repo_path = NULL, *label = NULL;
> const char *commit_id_str = NULL;
> struct got_object_id *id = NULL, *commit_id = NULL;
> + struct got_commit_object *commit = NULL;
> int ch, obj_type, i, force_path = 0;
> struct got_reflist_head refs;
>
> @@ -11914,9 +11929,13 @@ cmd_cat(int argc, char *argv[])
> if (error)
> goto done;
>
> + error = got_object_open_as_commit(&commit, repo, commit_id);
> + if (error)
> + goto done;
> +
> for (i = 0; i < argc; i++) {
> if (force_path) {
> - error = got_object_id_by_path(&id, repo, commit_id,
> + error = got_object_id_by_path(&id, repo, commit,
> argv[i]);
> if (error)
> break;
> @@ -11929,7 +11948,7 @@ cmd_cat(int argc, char *argv[])
> error->code != GOT_ERR_NOT_REF)
> break;
> error = got_object_id_by_path(&id, repo,
> - commit_id, argv[i]);
> + commit, argv[i]);
> if (error)
> break;
> }
> @@ -11967,6 +11986,8 @@ done:
> free(label);
> free(id);
> free(commit_id);
> + if (commit)
> + got_object_commit_close(commit);
> if (worktree)
> got_worktree_close(worktree);
> if (repo) {
> blob - a0286f156c9551904a9dfcbdfe00805bfe05bdf6
> blob + 70b31ef68737cdd0d46bd944fea3af2c33d3e89a
> --- gotweb/gotweb.c
> +++ gotweb/gotweb.c
> @@ -4044,6 +4044,7 @@ gw_output_file_blame(struct gw_trans *gw_trans, struct
> const struct got_error *error = NULL;
> struct got_object_id *obj_id = NULL;
> struct got_object_id *commit_id = NULL;
> + struct got_commit_object *commit = NULL;
> struct got_blob_object *blob = NULL;
> char *path = NULL, *in_repo_path = NULL;
> struct gw_blame_cb_args bca;
> @@ -4069,7 +4070,11 @@ gw_output_file_blame(struct gw_trans *gw_trans, struct
> if (error)
> goto done;
>
> - error = got_object_id_by_path(&obj_id, gw_trans->repo, commit_id,
> + error = got_object_open_as_commit(&commit, gw_trans->repo, commit_id);
> + if (error)
> + goto done;
> +
> + error = got_object_id_by_path(&obj_id, gw_trans->repo, commit,
> in_repo_path);
> if (error)
> goto done;
> @@ -4142,6 +4147,8 @@ done:
> }
> if (blob)
> got_object_blob_close(blob);
> + if (commit)
> + got_object_commit_close(commit);
> return error;
> }
>
> @@ -4151,6 +4158,7 @@ gw_output_blob_buf(struct gw_trans *gw_trans, struct g
> const struct got_error *error = NULL;
> struct got_object_id *obj_id = NULL;
> struct got_object_id *commit_id = NULL;
> + struct got_commit_object *commit = NULL;
> struct got_blob_object *blob = NULL;
> char *path = NULL, *in_repo_path = NULL;
> int obj_type, set_mime = 0;
> @@ -4175,7 +4183,11 @@ gw_output_blob_buf(struct gw_trans *gw_trans, struct g
> if (error)
> goto done;
>
> - error = got_object_id_by_path(&obj_id, gw_trans->repo, commit_id,
> + error = got_object_open_as_commit(&commit, gw_trans->repo, commit_id);
> + if (error)
> + goto done;
> +
> + error = got_object_id_by_path(&obj_id, gw_trans->repo, commit,
> in_repo_path);
> if (error)
> goto done;
> @@ -4232,6 +4244,8 @@ done:
> free(path);
> if (blob)
> got_object_blob_close(blob);
> + if (commit)
> + got_object_commit_close(commit);
> if (error == NULL && kerr != KCGI_OK)
> error = gw_kcgi_error(kerr);
> return error;
> @@ -4243,6 +4257,7 @@ gw_output_repo_tree(struct gw_trans *gw_trans, struct
> const struct got_error *error = NULL;
> struct got_object_id *tree_id = NULL, *commit_id = NULL;
> struct got_tree_object *tree = NULL;
> + struct got_commit_object *commit = NULL;
> char *path = NULL, *in_repo_path = NULL;
> char *id_str = NULL;
> char *build_folder = NULL;
> @@ -4294,7 +4309,11 @@ gw_output_repo_tree(struct gw_trans *gw_trans, struct
> goto done;
> }
>
> - error = got_object_id_by_path(&tree_id, gw_trans->repo, commit_id,
> + error = got_object_open_as_commit(&commit, gw_trans->repo, commit_id);
> + if (error)
> + goto done;
> +
> + error = got_object_id_by_path(&tree_id, gw_trans->repo, commit,
> path);
> if (error)
> goto done;
> @@ -4466,6 +4485,8 @@ gw_output_repo_tree(struct gw_trans *gw_trans, struct
> done:
> if (tree)
> got_object_tree_close(tree);
> + if (commit)
> + got_object_commit_close(commit);
> free(id_str);
> free(href_blob);
> free(href_blame);
> blob - 592e77655be88e831265856057c0f2f4d253128e
> blob + 9d6f362ad6242f2fdff94792bc113ede1710807d
> --- include/got_object.h
> +++ include/got_object.h
> @@ -106,7 +106,7 @@ const struct got_error *got_object_tree_find_path(stru
> * The caller should dispose of it with free(3).
> */
> const struct got_error *got_object_id_by_path(struct got_object_id **,
> - struct got_repository *, struct got_object_id *, const char *);
> + struct got_repository *, struct got_commit_object *, const char *);
>
> /*
> * Obtain the type of an object.
> @@ -241,7 +241,7 @@ int got_object_tree_entry_is_symlink(struct got_tree_e
> * target path. The caller must dispose of it with free(3).
> */
> const struct got_error *got_object_resolve_symlinks(char **, const char *,
> - struct got_object_id *, struct got_repository *);
> + struct got_commit_object *, struct got_repository *);
>
> /*
> * Compare two trees and indicate whether the entry at the specified path
> blob - d2db8c1e2cd6a46130821551a293b51fa2c841b9
> blob + 4fb57b680144beb72b8bb8898b73e6d032ad15d8
> --- lib/blame.c
> +++ lib/blame.c
> @@ -200,7 +200,7 @@ blame_commit(struct got_blame *blame, struct got_objec
> void *arg)
> {
> const struct got_error *err = NULL;
> - struct got_commit_object *commit = NULL;
> + struct got_commit_object *commit = NULL, *pcommit = NULL;
> struct got_object_qid *pid = NULL;
> struct got_object_id *pblob_id = NULL;
> struct got_blob_object *pblob = NULL;
> @@ -216,7 +216,11 @@ blame_commit(struct got_blame *blame, struct got_objec
> return NULL;
> }
>
> - err = got_object_id_by_path(&pblob_id, repo, pid->id, path);
> + err = got_object_open_as_commit(&pcommit, repo, pid->id);
> + if (err)
> + goto done;
> +
> + err = got_object_id_by_path(&pblob_id, repo, pcommit, path);
> if (err) {
> if (err->code == GOT_ERR_NO_TREE_ENTRY)
> err = NULL;
> @@ -267,6 +271,8 @@ done:
> diff_result_free(diff_result);
> if (commit)
> got_object_commit_close(commit);
> + if (pcommit)
> + got_object_commit_close(pcommit);
> free(pblob_id);
> if (pblob)
> got_object_blob_close(pblob);
> @@ -498,6 +504,7 @@ blame_open(struct got_blame **blamep, const char *path
> void *arg, got_cancel_cb cancel_cb, void *cancel_arg)
> {
> const struct got_error *err = NULL;
> + struct got_commit_object *start_commit = NULL;
> struct got_object_id *obj_id = NULL;
> struct got_blob_object *blob = NULL;
> struct got_blame *blame = NULL;
> @@ -507,10 +514,14 @@ blame_open(struct got_blame **blamep, const char *path
>
> *blamep = NULL;
>
> - err = got_object_id_by_path(&obj_id, repo, start_commit_id, path);
> + err = got_object_open_as_commit(&start_commit, repo, start_commit_id);
> if (err)
> goto done;
>
> + err = got_object_id_by_path(&obj_id, repo, start_commit, path);
> + if (err)
> + goto done;
> +
> err = got_object_open_as_blob(&blob, repo, obj_id, 8192);
> if (err)
> goto done;
> @@ -621,6 +632,8 @@ done:
> free(obj_id);
> if (blob)
> got_object_blob_close(blob);
> + if (start_commit)
> + got_object_commit_close(start_commit);
> if (err) {
> if (blame)
> blame_close(blame);
> blob - 55b4da29492815a14efed2a3680b55d68dddbfd0
> blob + 644fd4c564841be0c8142f649c9e68f26133deb7
> --- lib/commit_graph.c
> +++ lib/commit_graph.c
> @@ -112,7 +112,7 @@ detect_changed_path(int *changed, struct got_commit_ob
> pid = STAILQ_FIRST(&commit->parent_ids);
> if (pid == NULL) {
> struct got_object_id *obj_id;
> - err = got_object_id_by_path(&obj_id, repo, commit_id, path);
> + err = got_object_id_by_path(&obj_id, repo, commit, path);
> if (err) {
> if (err->code == GOT_ERR_NO_TREE_ENTRY)
> err = NULL;
> @@ -293,20 +293,30 @@ advance_branch(struct got_commit_graph *graph, struct
> struct got_object_id *merged_id, *prev_id = NULL;
> int branches_differ = 0;
>
> - err = got_object_id_by_path(&merged_id, repo, commit_id,
> + err = got_object_id_by_path(&merged_id, repo, commit,
> graph->path);
> if (err)
> return err;
>
> STAILQ_FOREACH(qid, &commit->parent_ids, entry) {
> - struct got_object_id *id;
> + struct got_object_id *id = NULL;
> + struct got_commit_object *pcommit = NULL;
>
> if (got_object_idset_contains(graph->open_branches,
> qid->id))
> continue;
>
> - err = got_object_id_by_path(&id, repo, qid->id,
> + err = got_object_open_as_commit(&pcommit, repo,
> + qid->id);
> + if (err) {
> + free(merged_id);
> + free(prev_id);
> + return err;
> + }
> + err = got_object_id_by_path(&id, repo, pcommit,
> graph->path);
> + got_object_commit_close(pcommit);
> + pcommit = NULL;
> if (err) {
> if (err->code == GOT_ERR_NO_TREE_ENTRY) {
> branches_differ = 1;
> blob - 8218b2c4c146e832c63f84d4a9f6a43f8b3283f8
> blob + f6a83e12b20b326cac3cf14c4fe93d71d56a2775
> --- lib/object.c
> +++ lib/object.c
> @@ -1998,18 +1998,13 @@ done:
> }
> const struct got_error *
> got_object_id_by_path(struct got_object_id **id, struct got_repository *repo,
> - struct got_object_id *commit_id, const char *path)
> + struct got_commit_object *commit, const char *path)
> {
> const struct got_error *err = NULL;
> - struct got_commit_object *commit = NULL;
> struct got_tree_object *tree = NULL;
>
> *id = NULL;
>
> - err = got_object_open_as_commit(&commit, repo, commit_id);
> - if (err)
> - goto done;
> -
> /* Handle opening of root of commit's tree. */
> if (got_path_is_root_dir(path)) {
> *id = got_object_id_dup(commit->tree_id);
> @@ -2022,8 +2017,6 @@ got_object_id_by_path(struct got_object_id **id, struc
> err = got_object_tree_find_path(id, NULL, repo, tree, path);
> }
> done:
> - if (commit)
> - got_object_commit_close(commit);
> if (tree)
> got_object_tree_close(tree);
> return err;
> @@ -2187,7 +2180,7 @@ got_object_tree_entry_is_symlink(struct got_tree_entry
>
> static const struct got_error *
> resolve_symlink(char **link_target, const char *path,
> - struct got_object_id *commit_id, struct got_repository *repo)
> + struct got_commit_object *commit, struct got_repository *repo)
> {
> const struct got_error *err = NULL;
> char buf[PATH_MAX];
> @@ -2209,7 +2202,7 @@ resolve_symlink(char **link_target, const char *path,
> if (err)
> return err;
>
> - err = got_object_id_by_path(&tree_obj_id, repo, commit_id,
> + err = got_object_id_by_path(&tree_obj_id, repo, commit,
> parent_path);
> if (err) {
> if (err->code == GOT_ERR_NO_TREE_ENTRY) {
> @@ -2265,7 +2258,7 @@ done:
>
> const struct got_error *
> got_object_resolve_symlinks(char **link_target, const char *path,
> - struct got_object_id *commit_id, struct got_repository *repo)
> + struct got_commit_object *commit, struct got_repository *repo)
> {
> const struct got_error *err = NULL;
> char *next_target = NULL;
> @@ -2275,7 +2268,7 @@ got_object_resolve_symlinks(char **link_target, const
>
> do {
> err = resolve_symlink(&next_target,
> - *link_target ? *link_target : path, commit_id, repo);
> + *link_target ? *link_target : path, commit, repo);
> if (err)
> break;
> if (next_target) {
> blob - 16eebc519859d846026d968e3e953c502035de48
> blob + bfebb46526cc0f3f0deda90d0868edad7c6946c9
> --- lib/worktree.c
> +++ lib/worktree.c
> @@ -2459,7 +2459,8 @@ done:
> static const struct got_error *
> find_tree_entry_for_checkout(int *entry_type, char **tree_relpath,
> struct got_object_id **tree_id, const char *wt_relpath,
> - struct got_worktree *worktree, struct got_repository *repo)
> + struct got_commit_object *base_commit, struct got_worktree *worktree,
> + struct got_repository *repo)
> {
> const struct got_error *err = NULL;
> struct got_object_id *id = NULL;
> @@ -2478,8 +2479,8 @@ find_tree_entry_for_checkout(int *entry_type, char **t
> err = got_error_from_errno("strdup");
> goto done;
> }
> - err = got_object_id_by_path(tree_id, repo,
> - worktree->base_commit_id, worktree->path_prefix);
> + err = got_object_id_by_path(tree_id, repo, base_commit,
> + worktree->path_prefix);
> if (err)
> goto done;
> return NULL;
> @@ -2493,8 +2494,7 @@ find_tree_entry_for_checkout(int *entry_type, char **t
> goto done;
> }
>
> - err = got_object_id_by_path(&id, repo, worktree->base_commit_id,
> - in_repo_path);
> + err = got_object_id_by_path(&id, repo, base_commit, in_repo_path);
> if (err)
> goto done;
>
> @@ -2532,7 +2532,7 @@ find_tree_entry_for_checkout(int *entry_type, char **t
> }
> }
> err = got_object_id_by_path(tree_id, repo,
> - worktree->base_commit_id, in_repo_path);
> + base_commit, in_repo_path);
> } else {
> /* Check out all files within a subdirectory. */
> *tree_id = got_object_id_dup(id);
> @@ -2644,6 +2644,11 @@ got_worktree_checkout_files(struct got_worktree *workt
> if (err)
> return err;
>
> + err = got_object_open_as_commit(&commit, repo,
> + worktree->base_commit_id);
> + if (err)
> + goto done;
> +
> /* Map all specified paths to in-repository trees. */
> TAILQ_FOREACH(pe, paths, entry) {
> tpd = malloc(sizeof(*tpd));
> @@ -2653,7 +2658,8 @@ got_worktree_checkout_files(struct got_worktree *workt
> }
>
> err = find_tree_entry_for_checkout(&tpd->entry_type,
> - &tpd->relpath, &tpd->tree_id, pe->path, worktree, repo);
> + &tpd->relpath, &tpd->tree_id, pe->path, commit,
> + worktree, repo);
> if (err) {
> free(tpd);
> goto done;
> @@ -3095,12 +3101,16 @@ merge_files(struct got_worktree *worktree, struct got_
> const struct got_error *err = NULL, *sync_err;
> struct got_object_id *tree_id1 = NULL, *tree_id2 = NULL;
> struct got_tree_object *tree1 = NULL, *tree2 = NULL;
> + struct got_commit_object *commit1 = NULL, *commit2 = NULL;
> struct check_merge_conflicts_arg cmc_arg;
> struct merge_file_cb_arg arg;
> char *label_orig = NULL;
>
> if (commit_id1) {
> - err = got_object_id_by_path(&tree_id1, repo, commit_id1,
> + err = got_object_open_as_commit(&commit1, repo, commit_id1);
> + if (err)
> + goto done;
> + err = got_object_id_by_path(&tree_id1, repo, commit1,
> worktree->path_prefix);
> if (err && err->code != GOT_ERR_NO_TREE_ENTRY)
> goto done;
> @@ -3125,7 +3135,11 @@ merge_files(struct got_worktree *worktree, struct got_
> free(id_str);
> }
>
> - err = got_object_id_by_path(&tree_id2, repo, commit_id2,
> + err = got_object_open_as_commit(&commit2, repo, commit_id2);
> + if (err)
> + goto done;
> +
> + err = got_object_id_by_path(&tree_id2, repo, commit2,
> worktree->path_prefix);
> if (err)
> goto done;
> @@ -3156,6 +3170,10 @@ merge_files(struct got_worktree *worktree, struct got_
> if (sync_err && err == NULL)
> err = sync_err;
> done:
> + if (commit1)
> + got_object_commit_close(commit1);
> + if (commit2)
> + got_object_commit_close(commit2);
> if (tree1)
> got_object_tree_close(tree1);
> if (tree2)
> @@ -4544,6 +4562,7 @@ revert_file(void *arg, unsigned char status, unsigned
> const struct got_error *err = NULL;
> char *parent_path = NULL;
> struct got_fileindex_entry *ie;
> + struct got_commit_object *base_commit = NULL;
> struct got_tree_object *tree = NULL;
> struct got_object_id *tree_id = NULL;
> const struct got_tree_entry *te = NULL;
> @@ -4597,8 +4616,12 @@ revert_file(void *arg, unsigned char status, unsigned
> }
> }
>
> - err = got_object_id_by_path(&tree_id, a->repo,
> - a->worktree->base_commit_id, tree_path);
> + err = got_object_open_as_commit(&base_commit, a->repo,
> + a->worktree->base_commit_id);
> + if (err)
> + goto done;
> +
> + err = got_object_id_by_path(&tree_id, a->repo, base_commit, tree_path);
> if (err) {
> if (!(err->code == GOT_ERR_NO_TREE_ENTRY &&
> (status == GOT_STATUS_ADD ||
> @@ -4756,6 +4779,8 @@ done:
> if (tree)
> got_object_tree_close(tree);
> free(tree_id);
> + if (base_commit)
> + got_object_commit_close(base_commit);
> return err;
> }
>
> @@ -5513,6 +5538,7 @@ check_out_of_date(const char *in_repo_path, unsigned c
> int ood_errcode)
> {
> const struct got_error *err = NULL;
> + struct got_commit_object *commit = NULL;
> struct got_object_id *id = NULL;
>
> if (status != GOT_STATUS_ADD && staged_status != GOT_STATUS_ADD) {
> @@ -5523,8 +5549,10 @@ check_out_of_date(const char *in_repo_path, unsigned c
> * Ensure file content which local changes were based
> * on matches file content in the branch head.
> */
> - err = got_object_id_by_path(&id, repo, head_commit_id,
> - in_repo_path);
> + err = got_object_open_as_commit(&commit, repo, head_commit_id);
> + if (err)
> + goto done;
> + err = got_object_id_by_path(&id, repo, commit, in_repo_path);
> if (err) {
> if (err->code == GOT_ERR_NO_TREE_ENTRY)
> err = got_error(ood_errcode);
> @@ -5533,14 +5561,18 @@ check_out_of_date(const char *in_repo_path, unsigned c
> err = got_error(ood_errcode);
> } else {
> /* Require that added files don't exist in the branch head. */
> - err = got_object_id_by_path(&id, repo, head_commit_id,
> - in_repo_path);
> + err = got_object_open_as_commit(&commit, repo, head_commit_id);
> + if (err)
> + goto done;
> + err = got_object_id_by_path(&id, repo, commit, in_repo_path);
> if (err && err->code != GOT_ERR_NO_TREE_ENTRY)
> goto done;
> err = id ? got_error(ood_errcode) : NULL;
> }
> done:
> free(id);
> + if (commit)
> + got_object_commit_close(commit);
> return err;
> }
>
> @@ -6692,6 +6724,7 @@ got_worktree_rebase_abort(struct got_worktree *worktre
> const struct got_error *err, *unlockerr, *sync_err;
> struct got_reference *resolved = NULL;
> struct got_object_id *commit_id = NULL;
> + struct got_commit_object *commit = NULL;
> char *fileindex_path = NULL;
> struct revert_file_args rfa;
> struct got_object_id *tree_id = NULL;
> @@ -6700,6 +6733,11 @@ got_worktree_rebase_abort(struct got_worktree *worktre
> if (err)
> return err;
>
> + err = got_object_open_as_commit(&commit, repo,
> + worktree->base_commit_id);
> + if (err)
> + goto done;
> +
> err = got_ref_open(&resolved, repo,
> got_ref_get_symref_target(new_base_branch), 0);
> if (err)
> @@ -6722,8 +6760,8 @@ got_worktree_rebase_abort(struct got_worktree *worktre
> if (err)
> goto done;
>
> - err = got_object_id_by_path(&tree_id, repo,
> - worktree->base_commit_id, worktree->path_prefix);
> + err = got_object_id_by_path(&tree_id, repo, commit,
> + worktree->path_prefix);
> if (err)
> goto done;
>
> @@ -6758,6 +6796,8 @@ done:
> got_ref_close(resolved);
> free(tree_id);
> free(commit_id);
> + if (commit)
> + got_object_commit_close(commit);
> if (fileindex)
> got_fileindex_free(fileindex);
> free(fileindex_path);
> @@ -7056,6 +7096,7 @@ got_worktree_histedit_abort(struct got_worktree *workt
> const struct got_error *err, *unlockerr, *sync_err;
> struct got_reference *resolved = NULL;
> char *fileindex_path = NULL;
> + struct got_commit_object *commit = NULL;
> struct got_object_id *tree_id = NULL;
> struct revert_file_args rfa;
>
> @@ -7063,6 +7104,11 @@ got_worktree_histedit_abort(struct got_worktree *workt
> if (err)
> return err;
>
> + err = got_object_open_as_commit(&commit, repo,
> + worktree->base_commit_id);
> + if (err)
> + goto done;
> +
> err = got_ref_open(&resolved, repo,
> got_ref_get_symref_target(branch), 0);
> if (err)
> @@ -7076,7 +7122,7 @@ got_worktree_histedit_abort(struct got_worktree *workt
> if (err)
> goto done;
>
> - err = got_object_id_by_path(&tree_id, repo, base_commit_id,
> + err = got_object_id_by_path(&tree_id, repo, commit,
> worktree->path_prefix);
> if (err)
> goto done;
> @@ -7269,6 +7315,7 @@ got_worktree_integrate_continue(struct got_worktree *w
> const struct got_error *err = NULL, *sync_err, *unlockerr;
> char *fileindex_path = NULL;
> struct got_object_id *tree_id = NULL, *commit_id = NULL;
> + struct got_commit_object *commit = NULL;
>
> err = get_fileindex_path(&fileindex_path, worktree);
> if (err)
> @@ -7278,7 +7325,11 @@ got_worktree_integrate_continue(struct got_worktree *w
> if (err)
> goto done;
>
> - err = got_object_id_by_path(&tree_id, repo, commit_id,
> + err = got_object_open_as_commit(&commit, repo, commit_id);
> + if (err)
> + goto done;
> +
> + err = got_object_id_by_path(&tree_id, repo, commit,
> worktree->path_prefix);
> if (err)
> goto done;
> @@ -7320,6 +7371,8 @@ done:
> got_fileindex_free(fileindex);
> free(fileindex_path);
> free(tree_id);
> + if (commit)
> + got_object_commit_close(commit);
>
> unlockerr = lock_worktree(worktree, LOCK_SH);
> if (unlockerr && err == NULL)
> @@ -7780,15 +7833,21 @@ got_worktree_merge_abort(struct got_worktree *worktree
> {
> const struct got_error *err, *unlockerr, *sync_err;
> struct got_object_id *commit_id = NULL;
> + struct got_commit_object *commit = NULL;
> char *fileindex_path = NULL;
> struct revert_file_args rfa;
> struct got_object_id *tree_id = NULL;
>
> - err = got_object_id_by_path(&tree_id, repo,
> - worktree->base_commit_id, worktree->path_prefix);
> + err = got_object_open_as_commit(&commit, repo,
> + worktree->base_commit_id);
> if (err)
> goto done;
>
> + err = got_object_id_by_path(&tree_id, repo, commit,
> + worktree->path_prefix);
> + if (err)
> + goto done;
> +
> err = delete_merge_refs(worktree, repo);
> if (err)
> goto done;
> @@ -7819,6 +7878,8 @@ sync:
> done:
> free(tree_id);
> free(commit_id);
> + if (commit)
> + got_object_commit_close(commit);
> if (fileindex)
> got_fileindex_free(fileindex);
> free(fileindex_path);
> blob - 162cb7c902a68e5ba42a938f863e0f9846e84b1a
> blob + cf640001b1d644e38bc4909d42362dd3ed7f348e
> --- tog/tog.c
> +++ tog/tog.c
> @@ -1910,7 +1910,7 @@ tree_view_visit_subtree(struct tog_tree_view_state *s,
>
> static const struct got_error *
> tree_view_walk_path(struct tog_tree_view_state *s,
> - struct got_object_id *commit_id, const char *path)
> + struct got_commit_object *commit, const char *path)
> {
> const struct got_error *err = NULL;
> struct got_tree_object *tree = NULL;
> @@ -1959,7 +1959,7 @@ tree_view_walk_path(struct tog_tree_view_state *s,
> break;
> }
>
> - err = got_object_id_by_path(&tree_id, s->repo, commit_id,
> + err = got_object_id_by_path(&tree_id, s->repo, commit,
> subpath);
> if (err)
> break;
> @@ -2008,7 +2008,7 @@ browse_commit_tree(struct tog_view **new_view, int beg
> if (got_path_is_root_dir(path))
> return NULL;
>
> - return tree_view_walk_path(s, entry->id, path);
> + return tree_view_walk_path(s, entry->commit, path);
> }
>
> static const struct got_error *
> @@ -4329,16 +4329,21 @@ run_blame(struct tog_view *view)
> struct tog_blame_view_state *s = &view->state.blame;
> struct tog_blame *blame = &s->blame;
> const struct got_error *err = NULL;
> + struct got_commit_object *commit = NULL;
> struct got_blob_object *blob = NULL;
> struct got_repository *thread_repo = NULL;
> struct got_object_id *obj_id = NULL;
> int obj_type;
>
> - err = got_object_id_by_path(&obj_id, s->repo, s->blamed_commit->id,
> - s->path);
> + err = got_object_open_as_commit(&commit, s->repo,
> + s->blamed_commit->id);
> if (err)
> return err;
>
> + err = got_object_id_by_path(&obj_id, s->repo, commit, s->path);
> + if (err)
> + goto done;
> +
> err = got_object_get_type(&obj_type, s->repo, obj_id);
> if (err)
> goto done;
> @@ -4405,6 +4410,8 @@ run_blame(struct tog_view *view)
> s->matched_line = 0;
>
> done:
> + if (commit)
> + got_object_commit_close(commit);
> if (blob)
> got_object_blob_close(blob);
> free(obj_id);
> @@ -4646,7 +4653,7 @@ input_blame_view(struct tog_view **new_view, struct to
> if (id == NULL)
> break;
> if (ch == 'p') {
> - struct got_commit_object *commit;
> + struct got_commit_object *commit, *pcommit;
> struct got_object_qid *pid;
> struct got_object_id *blob_id = NULL;
> int obj_type;
> @@ -4661,8 +4668,13 @@ input_blame_view(struct tog_view **new_view, struct to
> break;
> }
> /* Check if path history ends here. */
> + err = got_object_open_as_commit(&pcommit,
> + s->repo, pid->id);
> + if (err)
> + break;
> err = got_object_id_by_path(&blob_id, s->repo,
> - pid->id, s->path);
> + pcommit, s->path);
> + got_object_commit_close(pcommit);
> if (err) {
> if (err->code == GOT_ERR_NO_TREE_ENTRY)
> err = NULL;
> @@ -4807,6 +4819,7 @@ cmd_blame(int argc, char *argv[])
> char *cwd = NULL, *repo_path = NULL, *in_repo_path = NULL;
> char *link_target = NULL;
> struct got_object_id *commit_id = NULL;
> + struct got_commit_object *commit = NULL;
> char *commit_id_str = NULL;
> int ch;
> struct tog_view *view;
> @@ -4892,8 +4905,12 @@ cmd_blame(int argc, char *argv[])
> goto done;
> }
>
> + error = got_object_open_as_commit(&commit, repo, commit_id);
> + if (error)
> + goto done;
> +
> error = got_object_resolve_symlinks(&link_target, in_repo_path,
> - commit_id, repo);
> + commit, repo);
> if (error)
> goto done;
>
> @@ -4913,6 +4930,8 @@ done:
> free(link_target);
> free(cwd);
> free(commit_id);
> + if (commit)
> + got_object_commit_close(commit);
> if (worktree)
> got_worktree_close(worktree);
> if (repo) {
> @@ -5659,6 +5678,7 @@ cmd_tree(int argc, char *argv[])
> struct got_worktree *worktree = NULL;
> char *cwd = NULL, *repo_path = NULL, *in_repo_path = NULL;
> struct got_object_id *commit_id = NULL;
> + struct got_commit_object *commit = NULL;
> const char *commit_id_arg = NULL;
> char *label = NULL;
> struct got_reference *ref = NULL;
> @@ -5745,6 +5765,10 @@ cmd_tree(int argc, char *argv[])
> goto done;
> }
>
> + error = got_object_open_as_commit(&commit, repo, commit_id);
> + if (error)
> + goto done;
> +
> view = view_open(0, 0, 0, 0, TOG_VIEW_TREE);
> if (view == NULL) {
> error = got_error_from_errno("view_open");
> @@ -5754,7 +5778,7 @@ cmd_tree(int argc, char *argv[])
> if (error)
> goto done;
> if (!got_path_is_root_dir(in_repo_path)) {
> - error = tree_view_walk_path(&view->state.tree, commit_id,
> + error = tree_view_walk_path(&view->state.tree, commit,
> in_repo_path);
> if (error)
> goto done;
> @@ -6559,6 +6583,7 @@ tog_log_with_path(int argc, char *argv[])
> struct got_repository *repo = NULL;
> struct got_worktree *worktree = NULL;
> struct got_object_id *commit_id = NULL, *id = NULL;
> + struct got_commit_object *commit = NULL;
> char *cwd = NULL, *repo_path = NULL, *in_repo_path = NULL;
> char *commit_id_str = NULL, **cmd_argv = NULL;
>
> @@ -6602,7 +6627,11 @@ tog_log_with_path(int argc, char *argv[])
> worktree = NULL;
> }
>
> - error = got_object_id_by_path(&id, repo, commit_id, in_repo_path);
> + error = got_object_open_as_commit(&commit, repo, commit_id);
> + if (error)
> + goto done;
> +
> + error = got_object_id_by_path(&id, repo, commit, in_repo_path);
> if (error) {
> if (error->code != GOT_ERR_NO_TREE_ENTRY)
> goto done;
> @@ -6631,6 +6660,8 @@ done:
> if (error == NULL)
> error = close_err;
> }
> + if (commit)
> + got_object_commit_close(commit);
> if (worktree)
> got_worktree_close(worktree);
> free(id);
got_object_id_by_path() vs. commit object cache