From: Omar Polo Subject: Re: pass commit object to blame callback To: Stefan Sperling Cc: gameoftrees@openbsd.org Date: Thu, 07 Apr 2022 15:19:52 +0200 Stefan Sperling wrote: > Pass an already open commit object to the blame callback. > This was written on top of my id_by_path diff I sent earlier. > > The idea is to avoid re-opening the same commit again in the blame callback > as implemented by got.c and gotweb.c. Otherwise, these callbacks rely on the > commit cache to avoid doing the same amount of work the caller already did. > > ok? it also makes the callbacks easier to read :) ok op > diff 6fd1a33c932d168a9aef98ca7760e8f63446bea1 43acb5b55df6240c091bc4ec11079eaee47d64a7 > blob - 7ac9d84ea1bc903f4fd7ab569799630bbaa48aaa > blob + 1b4824dc3a8b72c725ef07b0e5d942bb8ad41589 > --- got/got.c > +++ got/got.c > @@ -4876,14 +4876,14 @@ struct blame_cb_args { > }; > > static const struct got_error * > -blame_cb(void *arg, int nlines, int lineno, struct got_object_id *id) > +blame_cb(void *arg, int nlines, int lineno, > + struct got_commit_object *commit, struct got_object_id *id) > { > const struct got_error *err = NULL; > struct blame_cb_args *a = arg; > struct blame_line *bline; > char *line = NULL; > size_t linesize = 0; > - struct got_commit_object *commit = NULL; > off_t offset; > struct tm tm; > time_t committer_time; > @@ -4906,10 +4906,6 @@ blame_cb(void *arg, int nlines, int lineno, struct got > if (err) > return err; > > - err = got_object_open_as_commit(&commit, a->repo, id); > - if (err) > - goto done; > - > bline->committer = strdup(got_object_commit_get_committer(commit)); > if (bline->committer == NULL) { > err = got_error_from_errno("strdup"); > @@ -4968,8 +4964,6 @@ blame_cb(void *arg, int nlines, int lineno, struct got > bline = &a->lines[a->lineno_cur - 1]; > } > done: > - if (commit) > - got_object_commit_close(commit); > free(line); > return err; > } > blob - 70b31ef68737cdd0d46bd944fea3af2c33d3e89a > blob + 710788181588ea0542eb1b6312476ddaa579fc99 > --- gotweb/gotweb.c > +++ gotweb/gotweb.c > @@ -214,6 +214,7 @@ static const struct got_error *gw_get_commit(struct gw > struct got_object_id *); > static const struct got_error *gw_apply_unveil(const char *); > static const struct got_error *gw_blame_cb(void *, int, int, > + struct got_commit_object *, > struct got_object_id *); > static const struct got_error *gw_load_got_paths(struct gw_trans *); > static const struct got_error *gw_load_got_path(struct gw_trans *, > @@ -3862,14 +3863,14 @@ struct gw_blame_cb_args { > }; > > static const struct got_error * > -gw_blame_cb(void *arg, int nlines, int lineno, struct got_object_id *id) > +gw_blame_cb(void *arg, int nlines, int lineno, > + struct got_commit_object *commit, struct got_object_id *id) > { > const struct got_error *err = NULL; > struct gw_blame_cb_args *a = arg; > struct blame_line *bline; > char *line = NULL; > size_t linesize = 0; > - struct got_commit_object *commit = NULL; > off_t offset; > struct tm tm; > time_t committer_time; > @@ -3890,10 +3891,6 @@ gw_blame_cb(void *arg, int nlines, int lineno, struct > if (err) > return err; > > - err = got_object_open_as_commit(&commit, a->repo, id); > - if (err) > - goto done; > - > bline->committer = strdup(got_object_commit_get_committer(commit)); > if (bline->committer == NULL) { > err = got_error_from_errno("strdup"); > @@ -4030,8 +4027,6 @@ err: > free(href_diff); > } > done: > - if (commit) > - got_object_commit_close(commit); > free(line); > if (err == NULL && kerr != KCGI_OK) > err = gw_kcgi_error(kerr); > blob - c4c15ba2906056d4efdd0378fb21aa0e70cf83f7 > blob + e360955ca287d181a8ff24dbf66100470e96de18 > --- include/got_blame.h > +++ include/got_blame.h > @@ -14,6 +14,9 @@ > * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. > */ > > +typedef const struct got_error *(*got_blame_cb)(void *, int, int, > + struct got_commit_object *, struct got_object_id *); > + > /* > * Blame the blob at the specified path in the specified commit and invoke > * a callback whenever an annotation has been computed for a line. > @@ -33,5 +36,4 @@ > */ > const struct got_error *got_blame(const char *, > struct got_object_id *, struct got_repository *, > - const struct got_error *(*cb)(void *, int, int, struct got_object_id *), > - void *, got_cancel_cb, void *); > + got_blame_cb, void *, got_cancel_cb, void *); > blob - 4fb57b680144beb72b8bb8898b73e6d032ad15d8 > blob + 19208f0e07660099d682daaab42dfacd9c2c5724 > --- lib/blame.c > +++ lib/blame.c > @@ -89,9 +89,9 @@ struct got_blame { > }; > > static const struct got_error * > -annotate_line(struct got_blame *blame, int lineno, struct got_object_id *id, > - const struct got_error *(*cb)(void *, int, int, struct got_object_id *), > - void *arg) > +annotate_line(struct got_blame *blame, int lineno, > + struct got_commit_object *commit, struct got_object_id *id, > + got_blame_cb cb, void *arg) > { > const struct got_error *err = NULL; > struct got_blame_line *line; > @@ -107,15 +107,14 @@ annotate_line(struct got_blame *blame, int lineno, str > line->annotated = 1; > blame->nannotated++; > if (cb) > - err = cb(arg, blame->nlines, lineno + 1, id); > + err = cb(arg, blame->nlines, lineno + 1, commit, id); > return err; > } > > static const struct got_error * > blame_changes(struct got_blame *blame, struct diff_result *diff_result, > - struct got_object_id *commit_id, > - const struct got_error *(*cb)(void *, int, int, struct got_object_id *), > - void *arg) > + struct got_commit_object *commit, struct got_object_id *commit_id, > + got_blame_cb cb, void *arg) > { > const struct got_error *err = NULL; > int i; > @@ -153,7 +152,8 @@ blame_changes(struct got_blame *blame, struct diff_res > > for (j = 0; j < right_count; j++) { > int ln = blame->linemap2[idx2++]; > - err = annotate_line(blame, ln, commit_id, cb, arg); > + err = annotate_line(blame, ln, commit, commit_id, > + cb, arg); > if (err) > return err; > if (blame->nlines == blame->nannotated) > @@ -196,8 +196,7 @@ blame_prepare_file(FILE *f, unsigned char **p, off_t * > static const struct got_error * > blame_commit(struct got_blame *blame, struct got_object_id *id, > const char *path, struct got_repository *repo, > - const struct got_error *(*cb)(void *, int, int, struct got_object_id *), > - void *arg) > + got_blame_cb cb, void *arg) > { > const struct got_error *err = NULL; > struct got_commit_object *commit = NULL, *pcommit = NULL; > @@ -261,11 +260,11 @@ blame_commit(struct got_blame *blame, struct got_objec > goto done; > } > } > - err = blame_changes(blame, diff_result, id, cb, arg); > + err = blame_changes(blame, diff_result, commit, id, cb, arg); > if (err) > goto done; > } else if (cb) > - err = cb(arg, blame->nlines, -1, id); > + err = cb(arg, blame->nlines, -1, commit, id); > done: > if (diff_result) > diff_result_free(diff_result); > @@ -500,11 +499,10 @@ close_file2_and_reuse_file1(struct got_blame *blame) > static const struct got_error * > blame_open(struct got_blame **blamep, const char *path, > struct got_object_id *start_commit_id, struct got_repository *repo, > - const struct got_error *(*cb)(void *, int, int, struct got_object_id *), > - void *arg, got_cancel_cb cancel_cb, void *cancel_arg) > + got_blame_cb cb, 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_commit_object *start_commit = NULL, *last_commit = NULL; > struct got_object_id *obj_id = NULL; > struct got_blob_object *blob = NULL; > struct got_blame *blame = NULL; > @@ -619,8 +617,12 @@ blame_open(struct got_blame **blamep, const char *path > > if (id && blame->nannotated < blame->nlines) { > /* Annotate remaining non-annotated lines with last commit. */ > + err = got_object_open_as_commit(&last_commit, repo, id); > + if (err) > + goto done; > for (lineno = 0; lineno < blame->nlines; lineno++) { > - err = annotate_line(blame, lineno, id, cb, arg); > + err = annotate_line(blame, lineno, last_commit, id, > + cb, arg); > if (err) > goto done; > } > @@ -634,6 +636,8 @@ done: > got_object_blob_close(blob); > if (start_commit) > got_object_commit_close(start_commit); > + if (last_commit) > + got_object_commit_close(last_commit); > if (err) { > if (blame) > blame_close(blame); > @@ -645,9 +649,8 @@ done: > > const struct got_error * > got_blame(const char *path, struct got_object_id *commit_id, > - struct got_repository *repo, > - const struct got_error *(*cb)(void *, int, int, struct got_object_id *), > - void *arg, got_cancel_cb cancel_cb, void* cancel_arg) > + struct got_repository *repo, got_blame_cb cb, void *arg, > + got_cancel_cb cancel_cb, void* cancel_arg) > { > const struct got_error *err = NULL, *close_err = NULL; > struct got_blame *blame; > blob - cf640001b1d644e38bc4909d42362dd3ed7f348e > blob + 1235f3d15f5c154123999f3791ee35c3119cd238 > --- tog/tog.c > +++ tog/tog.c > @@ -4162,7 +4162,8 @@ draw_blame(struct tog_view *view) > } > > static const struct got_error * > -blame_cb(void *arg, int nlines, int lineno, struct got_object_id *id) > +blame_cb(void *arg, int nlines, int lineno, > + struct got_commit_object *commit, struct got_object_id *id) > { > const struct got_error *err = NULL; > struct tog_blame_cb_args *a = arg;