commit 5191b70b5b2e123aadd88aeafe2e2cfc0958c327 from: Mark Jamsek date: Sat Jan 07 11:33:28 2023 UTC got: implement diffstat for got log and tog diff view Add new got_diff_blob_cb() implementation to compute added/removed line metrics for a given diff. This enables displaying a diffstat with 'got log -d'. As per suggestion from stsp, change tog diff view to display the diffstat by default. ok stsp@ commit - 17c726049a0c49099ef0a44190264fa4b6b037fa commit + 5191b70b5b2e123aadd88aeafe2e2cfc0958c327 blob - e754777a9202afca9a7b80dfd634b3c605d8a5cd blob + d7f14e7606560bbef80113d081dca440bc8ef372 --- got/got.1 +++ got/got.1 @@ -829,6 +829,11 @@ An abbreviated hash argument will be expanded to a ful automatically, provided the abbreviation is unique. If this option is not specified, default to the work tree's current branch if invoked in a work tree, or to the repository's HEAD reference. +.It Fl d +Display diffstat of changes introduced in the commit. +Cannot be used with the +.Fl s +option. .It Fl l Ar N Limit history traversal to a given number of commits. If this option is not specified, a default limit value of zero is used, blob - ad0fec63ec6030f40937e7bf6836769359bb55ba blob + aaf34c7a1dabde267171e1e19cd7f49b72525be1 --- got/got.c +++ got/got.c @@ -3753,13 +3753,42 @@ done: static const struct got_error * get_changed_paths(struct got_pathlist_head *paths, - struct got_commit_object *commit, struct got_repository *repo) + struct got_commit_object *commit, struct got_repository *repo, + struct got_diffstat_cb_arg *dsa) { const struct got_error *err = NULL; struct got_object_id *tree_id1 = NULL, *tree_id2 = NULL; struct got_tree_object *tree1 = NULL, *tree2 = NULL; struct got_object_qid *qid; + got_diff_blob_cb cb = got_diff_tree_collect_changed_paths; + FILE *f1 = NULL, *f2 = NULL; + int fd1 = -1, fd2 = -1; + if (dsa) { + cb = got_diff_tree_compute_diffstat; + + f1 = got_opentemp(); + if (f1 == NULL) { + err = got_error_from_errno("got_opentemp"); + goto done; + } + f2 = got_opentemp(); + if (f2 == NULL) { + err = got_error_from_errno("got_opentemp"); + goto done; + } + fd1 = got_opentempfd(); + if (fd1 == -1) { + err = got_error_from_errno("got_opentempfd"); + goto done; + } + fd2 = got_opentempfd(); + if (fd2 == -1) { + err = got_error_from_errno("got_opentempfd"); + goto done; + } + } + qid = STAILQ_FIRST(got_object_commit_get_parent_ids(commit)); if (qid != NULL) { struct got_commit_object *pcommit; @@ -3789,13 +3818,21 @@ get_changed_paths(struct got_pathlist_head *paths, if (err) goto done; - err = got_diff_tree(tree1, tree2, NULL, NULL, -1, -1, "", "", repo, - got_diff_tree_collect_changed_paths, paths, 0); + err = got_diff_tree(tree1, tree2, f1, f2, fd1, fd2, "", "", repo, + cb, dsa ? (void *)dsa : paths, dsa ? 1 : 0); done: if (tree1) got_object_tree_close(tree1); if (tree2) got_object_tree_close(tree2); + if (fd1 != -1 && close(fd1) == -1 && err == NULL) + err = got_error_from_errno("close"); + if (fd2 != -1 && close(fd2) == -1 && err == NULL) + err = got_error_from_errno("close"); + if (f1 && fclose(f1) == EOF && err == NULL) + err = got_error_from_errno("fclose"); + if (f2 && fclose(f2) == EOF && err == NULL) + err = got_error_from_errno("fclose"); free(tree_id1); return err; } @@ -4129,9 +4166,9 @@ done: static const struct got_error * print_commit(struct got_commit_object *commit, struct got_object_id *id, struct got_repository *repo, const char *path, - struct got_pathlist_head *changed_paths, int show_patch, - int diff_context, struct got_reflist_object_id_map *refs_idmap, - const char *custom_refs_str) + struct got_pathlist_head *changed_paths, struct got_diffstat_cb_arg *dsa, + int show_patch, int diff_context, + struct got_reflist_object_id_map *refs_idmap, const char *custom_refs_str) { const struct got_error *err = NULL; char *id_str, *datestr, *logmsg0, *logmsg, *line; @@ -4202,10 +4239,29 @@ print_commit(struct got_commit_object *commit, struct if (changed_paths) { struct got_pathlist_entry *pe; + TAILQ_FOREACH(pe, changed_paths, entry) { struct got_diff_changed_path *cp = pe->data; - printf(" %c %s\n", cp->status, pe->path); + char *stat = NULL; + + if (dsa) { + int pad = dsa->max_path_len - pe->path_len + 1; + + if (asprintf(&stat, "%*c | %*d+ %*d-", + pad, ' ', dsa->add_cols + 1, cp->add, + dsa->rm_cols + 1, cp->rm) == -1) { + err = got_error_from_errno("asprintf"); + goto done; + } + } + printf(" %c %s%s\n", cp->status, pe->path, + stat ? stat : ""); + free(stat); } + if (dsa) + printf("\n%d file%s changed, %d insertions(+), " + "%d deletions(-)\n", dsa->nfiles, + dsa->nfiles > 1 ? "s" : "", dsa->ins, dsa->del); printf("\n"); } if (show_patch) { @@ -4225,8 +4281,8 @@ done: static const struct got_error * print_commits(struct got_object_id *root_id, struct got_object_id *end_id, struct got_repository *repo, const char *path, int show_changed_paths, - int show_patch, const char *search_pattern, int diff_context, int limit, - int log_branches, int reverse_display_order, + int show_diffstat, int show_patch, const char *search_pattern, + int diff_context, int limit, int log_branches, int reverse_display_order, struct got_reflist_object_id_map *refs_idmap, int one_line, FILE *tmpfile) { @@ -4256,6 +4312,8 @@ print_commits(struct got_object_id *root_id, struct go goto done; for (;;) { struct got_object_id id; + struct got_diffstat_cb_arg dsa = { 0, 0, 0, 0, 0, 0, + &changed_paths, 0, 0, GOT_DIFF_ALGORITHM_PATIENCE }; if (sigint_received || sigpipe_received) break; @@ -4272,8 +4330,10 @@ print_commits(struct got_object_id *root_id, struct go if (err) break; - if (show_changed_paths && !reverse_display_order) { - err = get_changed_paths(&changed_paths, commit, repo); + if ((show_changed_paths || show_diffstat) && + !reverse_display_order) { + err = get_changed_paths(&changed_paths, commit, repo, + show_diffstat ? &dsa : NULL); if (err) break; } @@ -4317,8 +4377,10 @@ print_commits(struct got_object_id *root_id, struct go repo, refs_idmap); else err = print_commit(commit, &id, repo, path, - show_changed_paths ? &changed_paths : NULL, - show_patch, diff_context, refs_idmap, NULL); + (show_changed_paths || show_diffstat) ? + &changed_paths : NULL, + show_diffstat ? &dsa : NULL, show_patch, + diff_context, refs_idmap, NULL); got_object_commit_close(commit); if (err) break; @@ -4335,13 +4397,16 @@ print_commits(struct got_object_id *root_id, struct go } if (reverse_display_order) { STAILQ_FOREACH(qid, &reversed_commits, entry) { + struct got_diffstat_cb_arg dsa = { 0, 0, 0, 0, 0, 0, + &changed_paths, 0, 0, GOT_DIFF_ALGORITHM_PATIENCE }; + err = got_object_open_as_commit(&commit, repo, &qid->id); if (err) break; - if (show_changed_paths) { + if (show_changed_paths || show_diffstat) { err = get_changed_paths(&changed_paths, - commit, repo); + commit, repo, show_diffstat ? &dsa : NULL); if (err) break; } @@ -4350,8 +4415,10 @@ print_commits(struct got_object_id *root_id, struct go repo, refs_idmap); else err = print_commit(commit, &qid->id, repo, path, - show_changed_paths ? &changed_paths : NULL, - show_patch, diff_context, refs_idmap, NULL); + (show_changed_paths || show_diffstat) ? + &changed_paths : NULL, + show_diffstat ? &dsa : NULL, show_patch, + diff_context, refs_idmap, NULL); got_object_commit_close(commit); if (err) break; @@ -4416,7 +4483,7 @@ cmd_log(int argc, char *argv[]) const char *search_pattern = NULL; int diff_context = -1, ch; int show_changed_paths = 0, show_patch = 0, limit = 0, log_branches = 0; - int reverse_display_order = 0, one_line = 0; + int show_diffstat = 0, reverse_display_order = 0, one_line = 0; const char *errstr; struct got_reflist_head refs; struct got_reflist_object_id_map *refs_idmap = NULL; @@ -4434,7 +4501,7 @@ cmd_log(int argc, char *argv[]) limit = get_default_log_limit(); - while ((ch = getopt(argc, argv, "bC:c:l:PpRr:S:sx:")) != -1) { + while ((ch = getopt(argc, argv, "bC:c:dl:PpRr:S:sx:")) != -1) { switch (ch) { case 'b': log_branches = 1; @@ -4446,6 +4513,9 @@ cmd_log(int argc, char *argv[]) errx(1, "number of context lines is %s: %s", errstr, optarg); break; + case 'd': + show_diffstat = 1; + break; case 'c': start_commit = optarg; break; @@ -4494,8 +4564,8 @@ cmd_log(int argc, char *argv[]) else if (!show_patch) errx(1, "-C requires -p"); - if (one_line && (show_patch || show_changed_paths)) - errx(1, "cannot use -s with -p or -P"); + if (one_line && (show_patch || show_changed_paths || show_diffstat)) + errx(1, "cannot use -s with -d, -p or -P"); cwd = getcwd(NULL, 0); if (cwd == NULL) { @@ -4626,9 +4696,9 @@ cmd_log(int argc, char *argv[]) } error = print_commits(start_id, end_id, repo, path ? path : "", - show_changed_paths, show_patch, search_pattern, diff_context, - limit, log_branches, reverse_display_order, refs_idmap, one_line, - tmpfile); + show_changed_paths, show_diffstat, show_patch, search_pattern, + diff_context, limit, log_branches, reverse_display_order, + refs_idmap, one_line, tmpfile); done: free(path); free(repo_path); @@ -9873,7 +9943,7 @@ print_backup_ref(const char *branch_name, const char * if (asprintf(&custom_refs_str, "formerly %s", branch_name) == -1) return got_error_from_errno("asprintf"); - err = print_commit(old_commit, old_commit_id, repo, NULL, NULL, + err = print_commit(old_commit, old_commit_id, repo, NULL, NULL, NULL, 0, 0, refs_idmap, custom_refs_str); if (err) goto done; blob - 26617c2f087a790bbb5f19d32d57f83246fcb532 blob + 18fe6a19ccc9b5b2450e313f81d9246ce51b630d --- include/got_diff.h +++ include/got_diff.h @@ -146,8 +146,10 @@ const struct got_error *got_diff_tree(struct got_tree_ struct got_repository *, got_diff_blob_cb cb, void *cb_arg, int); /* - * A pre-defined implementation of got_diff_blob_cb() which collects a list - * of file paths that differ between two trees. + * Pre-defined implementations of got_diff_blob_cb(): the first of which + * collects a list of file paths that differ between two trees; the second + * also computes a diffstat of added/removed lines for each collected path + * and requires passing an initialized got_diffstat_cb_arg argument. * The caller must allocate and initialize a got_pathlist_head * argument. * Data pointers of entries added to the path list will point to a struct * got_diff_changed_path object. @@ -155,6 +157,8 @@ const struct got_error *got_diff_tree(struct got_tree_ * entries on the path list. */ struct got_diff_changed_path { + uint32_t add; /* number of lines added */ + uint32_t rm; /* number of lines removed */ /* * The modification status of this path. It can be GOT_STATUS_ADD, * GOT_STATUS_DELETE, GOT_STATUS_MODIFY, or GOT_STATUS_MODE_CHANGE. @@ -166,6 +170,23 @@ const struct got_error *got_diff_tree_collect_changed_ struct got_object_id *, struct got_object_id *, const char *, const char *, mode_t, mode_t, struct got_repository *); +struct got_diffstat_cb_arg { + size_t max_path_len; + uint32_t ins; + uint32_t del; + int add_cols; + int rm_cols; + int nfiles; + struct got_pathlist_head *paths; + int ignore_ws; + int force_text; + enum got_diff_algorithm diff_algo; +}; +const struct got_error *got_diff_tree_compute_diffstat(void *, + struct got_blob_object *, struct got_blob_object *, FILE *, FILE *, + struct got_object_id *, struct got_object_id *, const char *, const char *, + mode_t, mode_t, struct got_repository *); + /* * Diff two objects, assuming both objects are blobs. Two const char * diff * header labels may be provided which will be used to identify each blob in blob - 58baf2baa359d6bb49f1d7c1b7bd6af3f15bbb36 blob + 2007e46b23e16bc18f223174ce22d2e157bad96e --- lib/diff.c +++ lib/diff.c @@ -37,6 +37,10 @@ #include "got_lib_delta.h" #include "got_lib_inflate.h" #include "got_lib_object.h" + +#ifndef MAX +#define MAX(_a,_b) ((_a) > (_b) ? (_a) : (_b)) +#endif static const struct got_error * add_line_metadata(struct got_diff_line **lines, size_t *nlines, @@ -605,8 +609,139 @@ diff_entry_new_old(struct got_tree_entry *te2, return cb(cb_arg, NULL, NULL, NULL, NULL, NULL, &te2->id, NULL, label2, 0, te2->mode, repo); } + +static void +diffstat_field_width(size_t *maxlen, int *add_cols, int *rm_cols, size_t len, + uint32_t add, uint32_t rm) +{ + int d1 = 1, d2 = 1; + *maxlen = MAX(*maxlen, len); + + while (add /= 10) + ++d1; + *add_cols = MAX(*add_cols, d1); + + while (rm /= 10) + ++d2; + *rm_cols = MAX(*rm_cols, d2); +} + const struct got_error * +got_diff_tree_compute_diffstat(void *arg, struct got_blob_object *blob1, + struct got_blob_object *blob2, FILE *f1, FILE *f2, + struct got_object_id *id1, struct got_object_id *id2, + const char *label1, const char *label2, + mode_t mode1, mode_t mode2, struct got_repository *repo) +{ + const struct got_error *err = NULL; + struct got_diffreg_result *result = NULL; + struct diff_result *r; + struct got_diff_changed_path *change = NULL; + struct got_diffstat_cb_arg *a = arg; + struct got_pathlist_entry *pe; + char *path = NULL; + int i; + + path = strdup(label2 ? label2 : label1); + if (path == NULL) + return got_error_from_errno("malloc"); + + change = malloc(sizeof(*change)); + if (change == NULL) { + err = got_error_from_errno("malloc"); + goto done; + } + + change->add = 0; + change->rm = 0; + change->status = GOT_STATUS_NO_CHANGE; + if (id1 == NULL) + change->status = GOT_STATUS_ADD; + else if (id2 == NULL) + change->status = GOT_STATUS_DELETE; + else { + if (got_object_id_cmp(id1, id2) != 0) + change->status = GOT_STATUS_MODIFY; + else if (mode1 != mode2) + change->status = GOT_STATUS_MODE_CHANGE; + } + + if (f1) { + err = got_opentemp_truncate(f1); + if (err) + goto done; + } + if (f2) { + err = got_opentemp_truncate(f2); + if (err) + goto done; + } + + if (blob1) { + err = got_object_blob_dump_to_file(NULL, NULL, NULL, f1, + blob1); + if (err) + goto done; + } + if (blob2) { + err = got_object_blob_dump_to_file(NULL, NULL, NULL, f2, + blob2); + if (err) + goto done; + } + + err = got_diffreg(&result, f1, f2, a->diff_algo, a->ignore_ws, + a->force_text); + if (err) + goto done; + + for (i = 0, r = result->result; i < r->chunks.len; ++i) { + int flags = (r->left->atomizer_flags | r->right->atomizer_flags); + int isbin = (flags & DIFF_ATOMIZER_FOUND_BINARY_DATA); + + if (!isbin || a->force_text) { + struct diff_chunk *c; + int clc, crc; + + c = diff_chunk_get(r, i); + clc = diff_chunk_get_left_count(c); + crc = diff_chunk_get_right_count(c); + + if (clc && !crc) + change->rm += clc; + else if (crc && !clc) + change->add += crc; + } + } + + err = got_pathlist_append(a->paths, path, change); + if (err) + goto done; + + pe = TAILQ_LAST(a->paths, got_pathlist_head); + diffstat_field_width(&a->max_path_len, &a->add_cols, &a->rm_cols, + pe->path_len, change->add, change->rm); + a->ins += change->add; + a->del += change->rm; + ++a->nfiles; + +done: + if (result) { + const struct got_error *free_err; + + free_err = got_diffreg_result_free(result); + if (free_err && err == NULL) + err = free_err; + } + if (err) { + free(path); + free(change); + } + return err; +} + +const struct got_error * got_diff_tree_collect_changed_paths(void *arg, struct got_blob_object *blob1, struct got_blob_object *blob2, FILE *f1, FILE *f2, struct got_object_id *id1, struct got_object_id *id2, blob - 2f1436265914a5af2afa20029647cb7236df2288 blob + c032241608751301e7be7c2f440255dec505fecb --- tog/tog.c +++ tog/tog.c @@ -4457,13 +4457,38 @@ get_datestr(time_t *time, char *datebuf) static const struct got_error * get_changed_paths(struct got_pathlist_head *paths, - struct got_commit_object *commit, struct got_repository *repo) + struct got_commit_object *commit, struct got_repository *repo, + struct got_diffstat_cb_arg *dsa) { const struct got_error *err = NULL; struct got_object_id *tree_id1 = NULL, *tree_id2 = NULL; struct got_tree_object *tree1 = NULL, *tree2 = NULL; struct got_object_qid *qid; + FILE *f1 = NULL, *f2 = NULL; + int fd1 = -1, fd2 = -1; + f1 = got_opentemp(); + if (f1 == NULL) { + err = got_error_from_errno("got_opentemp"); + goto done; + } + f2 = got_opentemp(); + if (f2 == NULL) { + err = got_error_from_errno("got_opentemp"); + goto done; + } + + fd1 = got_opentempfd(); + if (fd1 == -1) { + err = got_error_from_errno("got_opentempfd"); + goto done; + } + fd2 = got_opentempfd(); + if (fd2 == -1) { + err = got_error_from_errno("got_opentempfd"); + goto done; + } + qid = STAILQ_FIRST(got_object_commit_get_parent_ids(commit)); if (qid != NULL) { struct got_commit_object *pcommit; @@ -4493,13 +4518,21 @@ get_changed_paths(struct got_pathlist_head *paths, if (err) goto done; - err = got_diff_tree(tree1, tree2, NULL, NULL, -1, -1, "", "", repo, - got_diff_tree_collect_changed_paths, paths, 0); + err = got_diff_tree(tree1, tree2, f1, f2, fd1, fd2, "", "", repo, + got_diff_tree_compute_diffstat, dsa, 1); done: if (tree1) got_object_tree_close(tree1); if (tree2) got_object_tree_close(tree2); + if (fd1 != -1 && close(fd1) == -1 && err == NULL) + err = got_error_from_errno("close"); + if (fd2 != -1 && close(fd2) == -1 && err == NULL) + err = got_error_from_errno("close"); + if (f1 && fclose(f1) == EOF && err == NULL) + err = got_error_from_errno("fclose"); + if (f2 && fclose(f2) == EOF && err == NULL) + err = got_error_from_errno("fclose"); free(tree_id1); return err; } @@ -4524,7 +4557,8 @@ add_line_metadata(struct got_diff_line **lines, size_t static const struct got_error * write_commit_info(struct got_diff_line **lines, size_t *nlines, struct got_object_id *commit_id, struct got_reflist_head *refs, - struct got_repository *repo, FILE *outfile) + struct got_repository *repo, int ignore_ws, int force_text_diff, + FILE *outfile) { const struct got_error *err = NULL; char datebuf[26], *datestr; @@ -4535,6 +4569,8 @@ write_commit_info(struct got_diff_line **lines, size_t char *refs_str = NULL; struct got_pathlist_head changed_paths; struct got_pathlist_entry *pe; + struct got_diffstat_cb_arg dsa = { 0, 0, 0, 0, 0, 0, &changed_paths, + ignore_ws, force_text_diff, tog_diff_algo }; off_t outoff = 0; int n; @@ -4651,12 +4687,17 @@ write_commit_info(struct got_diff_line **lines, size_t goto done; } - err = get_changed_paths(&changed_paths, commit, repo); + err = get_changed_paths(&changed_paths, commit, repo, &dsa); if (err) goto done; + TAILQ_FOREACH(pe, &changed_paths, entry) { struct got_diff_changed_path *cp = pe->data; - n = fprintf(outfile, "%c %s\n", cp->status, pe->path); + int pad = dsa.max_path_len - pe->path_len + 1; + + n = fprintf(outfile, "%c %s%*c | %*d+ %*d-\n", cp->status, + pe->path, pad, ' ', dsa.add_cols + 1, cp->add, + dsa.rm_cols + 1, cp->rm); if (n < 0) { err = got_error_from_errno("fprintf"); goto done; @@ -4673,6 +4714,24 @@ write_commit_info(struct got_diff_line **lines, size_t fputc('\n', outfile); outoff++; err = add_line_metadata(lines, nlines, outoff, GOT_DIFF_LINE_NONE); + if (err) + goto done; + + n = fprintf(outfile, + "%d file%s changed, %d insertions(+), %d deletions(-)\n", + dsa.nfiles, dsa.nfiles > 1 ? "s" : "", dsa.ins, dsa.del); + if (n < 0) { + err = got_error_from_errno("fprintf"); + goto done; + } + outoff += n; + err = add_line_metadata(lines, nlines, outoff, GOT_DIFF_LINE_NONE); + if (err) + goto done; + + fputc('\n', outfile); + outoff++; + err = add_line_metadata(lines, nlines, outoff, GOT_DIFF_LINE_NONE); done: got_pathlist_free(&changed_paths); free(id_str); @@ -4744,7 +4803,8 @@ create_diff(struct tog_diff_view_state *s) /* Show commit info if we're diffing to a parent/root commit. */ if (s->id1 == NULL) { err = write_commit_info(&s->lines, &s->nlines, s->id2, - refs, s->repo, s->f); + refs, s->repo, s->ignore_whitespace, + s->force_text_diff, s->f); if (err) goto done; } else { @@ -4753,7 +4813,8 @@ create_diff(struct tog_diff_view_state *s) if (got_object_id_cmp(s->id1, &pid->id) == 0) { err = write_commit_info(&s->lines, &s->nlines, s->id2, refs, s->repo, - s->f); + s->ignore_whitespace, + s->force_text_diff, s->f); if (err) goto done; break; @@ -5213,7 +5274,7 @@ input_diff_view(struct tog_view **new_view, struct tog case 'w': if (ch == 'a') s->force_text_diff = !s->force_text_diff; - if (ch == 'w') + else if (ch == 'w') s->ignore_whitespace = !s->ignore_whitespace; err = reset_diff_view(view); break;