got diff: add support for multiple path arguments

On Thu, Oct 07, 2021 at 11:56:37PM +0200, Stefan Sperling wrote: > How about: > got diff -c 6.9 -c 7.0 sys/net sys/dev/pci sys/dev/ic ... > > Where -c expects an object ID or a reference which resolves to a > commit object, much like many other commands. > Use of -c would imply -P, i.e. all non-option arguments are paths. > > A single -c option would also be supported: > got diff -c 6.9 sys/net > > This could be the same as: > got diff -c first-parent-commit-of-6.9 -c 6.9 sys/net This patch implements the above idea. It keeps all existing use cases working, and adds the ability to filter diffs between two commits by path. It does not prevent the addition of a repository->worktree diff. We could add another option to cover that case later, such as: got diff -t 6.9 It takes a bit of fiddling to interpret all the possible combinations of arguments. I hope I have covered all the possibilities. It is a bit annoying and makes the code tricky. But the UI we get in return seems fairly intuitive to me, so I think the complexity is worth it. Or should we use a different approach instead? If so, please make concrete suggestions about a new UI which covers all existing use cases, or present a convincing case for why some existing functionality could be dropped in favour of your proposed new UI. Can anyone think of additional invocations that need testing? There is one test case I am skipping intentionally: If a blob object has been replaced by a tree object (or vice versa) between the two commits, we will error out. This case is also broken in the current tree object diff driver (see diff_kind_mismatch() in lib/diff.c) and can be fixed later. The patch below was generated with: got diff -c diff-c (Where the diff-c branch adds only one commit on top of the main branch.) diff 138e4f4798d5a09e2b97c9a18e49999fb8ff599b refs/heads/diff-c blob - cf8d6c0c7dfcb49db87bfb44ed436e7dc2e5a132 blob + f0a19759ff8cd6e32301a3956c93992b0687c7e2 --- got/got.1 +++ got/got.1 @@ -839,7 +839,7 @@ This option has no effect if the specified is never traversed. .El .Tg di -.It Cm diff Oo Fl a Oc Oo Fl C Ar number Oc Oo Fl r Ar repository-path Oc Oo Fl s Oc Oo Fl P Oc Oo Fl w Oc Op Ar object1 Ar object2 | Ar path ... +.It Cm diff Oo Fl a Oc Oo Fl c Ar commit Oc Oo Fl C Ar number Oc Oo Fl r Ar repository-path Oc Oo Fl s Oc Oo Fl P Oc Oo Fl w Oc Op Ar object1 Ar object2 | Ar path ... .Dl (alias: Cm di ) When invoked within a work tree without any arguments, display all local changes in the work tree. @@ -866,6 +866,30 @@ are as follows: .Bl -tag -width Ds .It Fl a Treat file contents as ASCII text even if binary data is detected. +.It Fl c Ar commit +Show differences between commits in the repository. +This options may be used up to two times. +When used only once, show differences between the specified +.Ar commit +and its first parent commit. +When used twice, show differences between the two specified commits. +.Pp +The expected argument is a commit ID SHA1 hash or an existing reference +or tag name which will be resolved to a commit ID. +An abbreviated hash argument will be expanded to a full SHA1 hash +automatically, provided the abbreviation is unique. +.Pp +If the +.Fl c +option is used, all non-option arguments will be interpreted as paths. +If one or more such +.Ar path +arguments are provided, only show differences for the specified paths. +.Pp +Cannot be used together with the +.Fl P +option. +.Pp .It Fl C Ar number Set the number of context lines shown in the diff. By default, 3 lines of context are shown. blob - 3454ef5b7d94ce34d945b2c25782176e12ddf8cc blob + 8449c3c11215dcdfed0b9895f61a62a523ab4aad --- got/got.c +++ got/got.c @@ -4276,7 +4276,8 @@ done: __dead static void usage_diff(void) { - fprintf(stderr, "usage: %s diff [-a] [-C number] [-r repository-path] " + fprintf(stderr, "usage: %s diff [-a] [-c commit1] [-c commit2] " + "[-C number] [-r repository-path] " "[-s] [-w] [-P] [object1 object2 | path ...]\n", getprogname()); exit(1); } @@ -4486,9 +4487,11 @@ cmd_diff(int argc, char *argv[]) struct got_repository *repo = NULL; struct got_worktree *worktree = NULL; char *cwd = NULL, *repo_path = NULL; + const char *commit_args[2] = { NULL, NULL }; + int ncommit_args = 0; struct got_object_id *ids[2] = { NULL, NULL }; char *labels[2] = { NULL, NULL }; - int type1, type2; + int type1 = GOT_OBJ_TYPE_ANY, type2 = GOT_OBJ_TYPE_ANY; int diff_context = 3, diff_staged = 0, ignore_whitespace = 0, ch, i; int force_text_diff = 0, force_path = 0, rflag = 0; const char *errstr; @@ -4505,11 +4508,16 @@ cmd_diff(int argc, char *argv[]) err(1, "pledge"); #endif - while ((ch = getopt(argc, argv, "aC:r:swP")) != -1) { + while ((ch = getopt(argc, argv, "ac:C:r:swP")) != -1) { switch (ch) { case 'a': force_text_diff = 1; break; + case 'c': + if (ncommit_args >= 2) + errx(1, "too many -c options used"); + commit_args[ncommit_args++] = optarg; + break; case 'C': diff_context = strtonum(optarg, 0, GOT_DIFF_MAX_CONTEXT, &errstr); @@ -4570,48 +4578,84 @@ cmd_diff(int argc, char *argv[]) } } - if (force_path && (rflag || worktree == NULL)) - errx(1, "-P option can only be used when diffing a work tree"); - error = got_repo_open(&repo, repo_path, NULL); free(repo_path); if (error != NULL) goto done; + if (rflag || worktree == NULL || ncommit_args > 0) { + if (force_path) { + error = got_error_msg(GOT_ERR_NOT_IMPL, + "-P option can only be used when diffing " + "a work tree"); + goto done; + } + if (diff_staged) { + error = got_error_msg(GOT_ERR_NOT_IMPL, + "-s option can only be used when diffing " + "a work tree"); + goto done; + } + } + error = apply_unveil(got_repo_get_path(repo), 1, worktree ? got_worktree_get_root_path(worktree) : NULL); if (error) goto done; - if (!force_path && argc == 2) { + if ((!force_path && argc == 2) || ncommit_args > 0) { + int obj_type = (ncommit_args > 0 ? + GOT_OBJ_TYPE_COMMIT : GOT_OBJ_TYPE_ANY); error = got_ref_list(&refs, repo, NULL, got_ref_cmp_by_name, NULL); if (error) goto done; - for (i = 0; i < argc; i++) { + for (i = 0; i < (ncommit_args > 0 ? ncommit_args : argc); i++) { + const char *arg; + if (ncommit_args > 0) + arg = commit_args[i]; + else + arg = argv[i]; error = got_repo_match_object_id(&ids[i], &labels[i], - argv[i], GOT_OBJ_TYPE_ANY, &refs, repo); + arg, obj_type, &refs, repo); if (error) { if (error->code != GOT_ERR_NOT_REF && error->code != GOT_ERR_NO_OBJ) goto done; + if (ncommit_args > 0) + goto done; error = NULL; break; } } } - if (worktree != NULL && (ids[0] == NULL || ids[1] == NULL)) { - error = get_worktree_paths_from_argv(&paths, - argc, argv, worktree); - if (error) - goto done; - } - - if (!TAILQ_EMPTY(&paths)) { + if (ncommit_args == 0 && (ids[0] == NULL || ids[1] == NULL)) { struct print_diff_arg arg; char *id_str; + if (worktree == NULL) { + if (argc == 2 && ids[0] == NULL) { + error = got_error_path(argv[0], GOT_ERR_NO_OBJ); + goto done; + } else if (argc == 2 && ids[1] == NULL) { + error = got_error_path(argv[1], GOT_ERR_NO_OBJ); + goto done; + } else if (argc > 0) { + error = got_error_fmt(GOT_ERR_NOT_WORKTREE, + "%s", "specified paths cannot be resolved"); + goto done; + } else { + error = got_error(GOT_ERR_NOT_WORKTREE); + goto done; + } + } + + error = get_worktree_paths_from_argv(&paths, argc, argv, + worktree); + if (error) + goto done; + error = got_object_id_str(&id_str, got_worktree_get_base_commit_id(worktree)); if (error) @@ -4631,34 +4675,115 @@ cmd_diff(int argc, char *argv[]) goto done; } - if (ids[0] == NULL || ids[1] == NULL) { - if (argc == 2) { - error = got_error_fmt(GOT_ERR_NO_OBJ, "%s", - ids[0] ? argv[1] : argv[0]); + if (ncommit_args == 1) { + struct got_commit_object *commit; + error = got_object_open_as_commit(&commit, repo, ids[0]); + if (error) goto done; - } if (worktree == NULL) { - error = got_error(GOT_ERR_NOT_WORKTREE); - goto done; - } else - usage_diff(); + + labels[1] = labels[0]; + ids[1] = ids[0]; + if (got_object_commit_get_nparents(commit) > 0) { + const struct got_object_id_queue *pids; + struct got_object_qid *pid; + pids = got_object_commit_get_parent_ids(commit); + pid = STAILQ_FIRST(pids); + ids[0] = got_object_id_dup(pid->id); + if (ids[0] == NULL) { + error = got_error_from_errno( + "got_object_id_dup"); + got_object_commit_close(commit); + goto done; + } + error = got_object_id_str(&labels[0], ids[0]); + if (error) { + got_object_commit_close(commit); + goto done; + } + } else { + ids[0] = NULL; + labels[0] = strdup("/dev/null"); + if (labels[0] == NULL) { + error = got_error_from_errno("strdup"); + got_object_commit_close(commit); + goto done; + } + } + + got_object_commit_close(commit); } - if (diff_staged) - errx(1, "-s option can't be used when diffing " - "objects in repository"); - error = got_object_get_type(&type1, repo, ids[0]); - if (error) + if (ncommit_args == 0 && argc > 2) { + error = got_error_msg(GOT_ERR_BAD_PATH, + "path arguments cannot be used when diffing two objects"); goto done; + } + if (ids[0]) { + error = got_object_get_type(&type1, repo, ids[0]); + if (error) + goto done; + } + error = got_object_get_type(&type2, repo, ids[1]); if (error) goto done; - if (type1 != type2) { + if (type1 != GOT_OBJ_TYPE_ANY && type1 != type2) { error = got_error(GOT_ERR_OBJ_TYPE); goto done; } + if (type1 == GOT_OBJ_TYPE_BLOB && argc > 0) { + error = got_error_msg(GOT_ERR_OBJ_TYPE, + "path arguments cannot be used when diffing blobs"); + goto done; + } - switch (type1) { + for (i = 0; ncommit_args > 0 && i < argc; i++) { + char *in_repo_path; + struct got_pathlist_entry *new; + if (worktree) { + const char *prefix; + char *p; + error = got_worktree_resolve_path(&p, worktree, + argv[i]); + if (error) + goto done; + prefix = got_worktree_get_path_prefix(worktree); + while (prefix[0] == '/') + prefix++; + if (asprintf(&in_repo_path, "%s%s%s", prefix, + (p[0] != '\0' && prefix[0] != '\0') ? "/" : "", + p) == -1) { + error = got_error_from_errno("asprintf"); + free(p); + goto done; + } + free(p); + } else { + char *mapped_path, *s; + error = got_repo_map_path(&mapped_path, repo, argv[i]); + if (error) + goto done; + s = mapped_path; + while (s[0] == '/') + s++; + in_repo_path = strdup(s); + if (in_repo_path == NULL) { + error = got_error_from_errno("asprintf"); + free(mapped_path); + goto done; + } + free(mapped_path); + + } + error = got_pathlist_insert(&new, &paths, in_repo_path, NULL); + if (error || new == NULL /* duplicate */) + free(in_repo_path); + if (error) + goto done; + } + + switch (type1 == GOT_OBJ_TYPE_ANY ? type2 : type1) { case GOT_OBJ_TYPE_BLOB: error = got_diff_objects_as_blobs(NULL, NULL, ids[0], ids[1], NULL, NULL, diff_context, ignore_whitespace, @@ -4666,14 +4791,14 @@ cmd_diff(int argc, char *argv[]) break; case GOT_OBJ_TYPE_TREE: error = got_diff_objects_as_trees(NULL, NULL, ids[0], ids[1], - "", "", diff_context, ignore_whitespace, force_text_diff, - repo, stdout); + &paths, "", "", diff_context, ignore_whitespace, + force_text_diff, repo, stdout); break; case GOT_OBJ_TYPE_COMMIT: printf("diff %s %s\n", labels[0], labels[1]); error = got_diff_objects_as_commits(NULL, NULL, ids[0], ids[1], - diff_context, ignore_whitespace, force_text_diff, repo, - stdout); + &paths, diff_context, ignore_whitespace, force_text_diff, + repo, stdout); break; default: error = got_error(GOT_ERR_OBJ_TYPE); blob - 7b22d4ac7703c85955d363ec5f1b20d4c17c9f5d blob + eeaf4cbd73e78bea7a719194a8223f1826901821 --- gotweb/gotweb.c +++ gotweb/gotweb.c @@ -2887,11 +2887,11 @@ gw_output_diff(struct gw_trans *gw_trans, struct gw_he break; case GOT_OBJ_TYPE_TREE: error = got_diff_objects_as_trees(NULL, NULL, id1, id2, - "", "", 3, 0, 0, gw_trans->repo, f); + NULL, "", "", 3, 0, 0, gw_trans->repo, f); break; case GOT_OBJ_TYPE_COMMIT: error = got_diff_objects_as_commits(NULL, NULL, id1, id2, - 3, 0, 0, gw_trans->repo, f); + NULL, 3, 0, 0, gw_trans->repo, f); break; default: error = got_error(GOT_ERR_OBJ_TYPE); blob - 2e56321573d28e2eeb0cb21dadfcb6110292501c blob + 47d14eddfbf3c27bb83d461d7330e04389cd0700 --- include/got_diff.h +++ include/got_diff.h @@ -142,8 +142,8 @@ const struct got_error *got_diff_objects_as_blobs(off_ * array of line offsets for, and the number of lines in, the unidiff text. */ const struct got_error *got_diff_objects_as_trees(off_t **, size_t *, - struct got_object_id *, struct got_object_id *, char *, char *, - int, int, int, struct got_repository *, FILE *); + struct got_object_id *, struct got_object_id *, struct got_pathlist_head *, + char *, char *, int, int, int, struct got_repository *, FILE *); /* * Diff two objects, assuming both objects are commits. @@ -153,7 +153,7 @@ const struct got_error *got_diff_objects_as_trees(off_ * array of line offsets for, and the number of lines in, the unidiff text. */ const struct got_error *got_diff_objects_as_commits(off_t **, size_t *, - struct got_object_id *, struct got_object_id *, int, int, int, - struct got_repository *, FILE *); + struct got_object_id *, struct got_object_id *, struct got_pathlist_head *, + int, int, int, struct got_repository *, FILE *); #define GOT_DIFF_MAX_CONTEXT 64 blob - f0f73917f7b7ea9ee69014ce63be92ae9ad0b705 blob + b6870a5a2324dda8c85c26409b7fd6329fa970ed --- include/got_object.h +++ include/got_object.h @@ -93,6 +93,15 @@ struct got_object_id *got_object_id_dup(struct got_obj /* * Get a newly allocated ID of the object which resides at the specified + * path in the specified tree. + * The caller should dispose of it with free(3). + */ +const struct got_error *got_object_tree_find_path(struct got_object_id **id, + mode_t *mode, struct got_repository *repo, struct got_tree_object *tree, + const char *path); + +/* + * Get a newly allocated ID of the object which resides at the specified * path in the tree of the specified commit. * The caller should dispose of it with free(3). */ blob - 54a0944d64ec3e964a3e3d8938d5cea99aaf76f6 blob + 8049aab224eea3bbfc105be06c2c34e8f89c21d3 --- lib/diff.c +++ lib/diff.c @@ -761,9 +761,137 @@ done: return err; } +static const struct got_error * +diff_paths(struct got_tree_object *tree1, struct got_tree_object *tree2, + struct got_pathlist_head *paths, struct got_repository *repo, + got_diff_blob_cb cb, void *cb_arg) +{ + const struct got_error *err = NULL; + struct got_pathlist_entry *pe; + struct got_object_id *id1 = NULL, *id2 = NULL; + struct got_tree_object *subtree1 = NULL, *subtree2 = NULL; + struct got_blob_object *blob1 = NULL, *blob2 = NULL; + + TAILQ_FOREACH(pe, paths, entry) { + int type1 = GOT_OBJ_TYPE_ANY, type2 = GOT_OBJ_TYPE_ANY; + mode_t mode1 = 0, mode2 = 0; + + free(id1); + id1 = NULL; + free(id2); + id2 = NULL; + if (subtree1) { + got_object_tree_close(subtree1); + subtree1 = NULL; + } + if (subtree2) { + got_object_tree_close(subtree2); + subtree2 = NULL; + } + if (blob1) { + got_object_blob_close(blob1); + blob1 = NULL; + } + if (blob2) { + got_object_blob_close(blob2); + blob2 = NULL; + } + + err = got_object_tree_find_path(&id1, &mode1, repo, tree1, + pe->path); + if (err && err->code != GOT_ERR_NO_TREE_ENTRY) + goto done; + err = got_object_tree_find_path(&id2, &mode2, repo, tree2, + pe->path); + if (err && err->code != GOT_ERR_NO_TREE_ENTRY) + goto done; + if (id1 == NULL && id2 == NULL) { + err = got_error_path(pe->path, GOT_ERR_NO_TREE_ENTRY); + goto done; + } + if (id1) { + err = got_object_get_type(&type1, repo, id1); + if (err) + goto done; + } + if (id2) { + err = got_object_get_type(&type2, repo, id2); + if (err) + goto done; + } + if (type1 == GOT_OBJ_TYPE_ANY && + type2 == GOT_OBJ_TYPE_ANY) { + err = got_error_path(pe->path, GOT_ERR_NO_OBJ); + goto done; + } else if (type1 != GOT_OBJ_TYPE_ANY && + type2 != GOT_OBJ_TYPE_ANY && type1 != type2) { + err = got_error(GOT_ERR_OBJ_TYPE); + goto done; + } + + if (type1 == GOT_OBJ_TYPE_BLOB || + type2 == GOT_OBJ_TYPE_BLOB) { + if (id1) { + err = got_object_open_as_blob(&blob1, repo, + id1, 8192); + if (err) + goto done; + } + if (id2) { + err = got_object_open_as_blob(&blob2, repo, + id2, 8192); + if (err) + goto done; + } + err = cb(cb_arg, blob1, blob2, id1, id2, + id1 ? pe->path : "/dev/null", + id2 ? pe->path : "/dev/null", + mode1, mode2, repo); + if (err) + goto done; + } else if (type1 == GOT_OBJ_TYPE_TREE || + type2 == GOT_OBJ_TYPE_TREE) { + if (id1) { + err = got_object_open_as_tree(&subtree1, repo, + id1); + if (err) + goto done; + } + if (id2) { + err = got_object_open_as_tree(&subtree2, repo, + id2); + if (err) + goto done; + } + err = got_diff_tree(subtree1, subtree2, + id1 ? pe->path : "/dev/null", + id2 ? pe->path : "/dev/null", + repo, cb, cb_arg, 1); + if (err) + goto done; + } else { + err = got_error(GOT_ERR_OBJ_TYPE); + goto done; + } + } +done: + free(id1); + free(id2); + if (subtree1) + got_object_tree_close(subtree1); + if (subtree2) + got_object_tree_close(subtree2); + if (blob1) + got_object_blob_close(blob1); + if (blob2) + got_object_blob_close(blob2); + return err; +} + const struct got_error * got_diff_objects_as_trees(off_t **line_offsets, size_t *nlines, struct got_object_id *id1, struct got_object_id *id2, + struct got_pathlist_head *paths, char *label1, char *label2, int diff_context, int ignore_whitespace, int force_text_diff, struct got_repository *repo, FILE *outfile) { @@ -785,6 +913,7 @@ got_diff_objects_as_trees(off_t **line_offsets, size_t if (err) goto done; } + arg.diff_context = diff_context; arg.ignore_whitespace = ignore_whitespace; arg.force_text_diff = force_text_diff; @@ -796,9 +925,13 @@ got_diff_objects_as_trees(off_t **line_offsets, size_t arg.line_offsets = NULL; arg.nlines = 0; } - err = got_diff_tree(tree1, tree2, label1, label2, repo, - got_diff_blob_output_unidiff, &arg, 1); - + if (paths == NULL || TAILQ_EMPTY(paths)) { + err = got_diff_tree(tree1, tree2, label1, label2, repo, + got_diff_blob_output_unidiff, &arg, 1); + } else { + err = diff_paths(tree1, tree2, paths, repo, + got_diff_blob_output_unidiff, &arg); + } if (want_lineoffsets) { *line_offsets = arg.line_offsets; /* was likely re-allocated */ *nlines = arg.nlines; @@ -814,6 +947,7 @@ done: const struct got_error * got_diff_objects_as_commits(off_t **line_offsets, size_t *nlines, struct got_object_id *id1, struct got_object_id *id2, + struct got_pathlist_head *paths, int diff_context, int ignore_whitespace, int force_text_diff, struct got_repository *repo, FILE *outfile) { @@ -835,8 +969,8 @@ got_diff_objects_as_commits(off_t **line_offsets, size err = got_diff_objects_as_trees(line_offsets, nlines, commit1 ? got_object_commit_get_tree_id(commit1) : NULL, - got_object_commit_get_tree_id(commit2), "", "", diff_context, - ignore_whitespace, force_text_diff, repo, outfile); + got_object_commit_get_tree_id(commit2), paths, "", "", + diff_context, ignore_whitespace, force_text_diff, repo, outfile); done: if (commit1) got_object_commit_close(commit1); blob - 66e51456da85f0477bef25d1074a69d766e38fe2 blob + 436e51336253bb9f9800cd5a074e97627ef49d53 --- lib/object.c +++ lib/object.c @@ -1906,39 +1906,24 @@ got_object_tree_find_entry(struct got_tree_object *tre } 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) +got_object_tree_find_path(struct got_object_id **id, mode_t *mode, + struct got_repository *repo, struct got_tree_object *tree, + const char *path) { const struct got_error *err = NULL; - struct got_commit_object *commit = NULL; - struct got_tree_object *tree = NULL; + struct got_tree_object *subtree = NULL; struct got_tree_entry *te = NULL; const char *seg, *s; size_t seglen; *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); - if (*id == NULL) - err = got_error_from_errno("got_object_id_dup"); - goto done; - } - - err = got_object_open_as_tree(&tree, repo, commit->tree_id); - if (err) - goto done; - s = path; while (s[0] == '/') s++; seg = s; seglen = 0; + subtree = tree; while (*s) { struct got_tree_object *next_tree; @@ -1949,7 +1934,7 @@ got_object_id_by_path(struct got_object_id **id, struc continue; } - te = find_entry_by_name(tree, seg, seglen); + te = find_entry_by_name(subtree, seg, seglen); if (te == NULL) { err = got_error_path(path, GOT_ERR_NO_TREE_ENTRY); goto done; @@ -1967,8 +1952,9 @@ got_object_id_by_path(struct got_object_id **id, struc te = NULL; if (err) goto done; - got_object_tree_close(tree); - tree = next_tree; + if (subtree != tree) + got_object_tree_close(subtree); + subtree = next_tree; } } @@ -1976,9 +1962,39 @@ got_object_id_by_path(struct got_object_id **id, struc *id = got_object_id_dup(&te->id); if (*id == NULL) return got_error_from_errno("got_object_id_dup"); + if (mode) + *mode = te->mode; } else err = got_error_path(path, GOT_ERR_NO_TREE_ENTRY); done: + return err; +} +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) +{ + 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); + if (*id == NULL) + err = got_error_from_errno("got_object_id_dup"); + } else { + err = got_object_open_as_tree(&tree, repo, commit->tree_id); + if (err) + goto done; + err = got_object_tree_find_path(id, NULL, repo, tree, path); + } +done: if (commit) got_object_commit_close(commit); if (tree) blob - c04e7e02b4abed6321e3ffb41a3e8510fff59ec0 blob + 9180f57976107c762edd140fcaa6e736a9fb2f77 --- regress/cmdline/diff.sh +++ regress/cmdline/diff.sh @@ -974,6 +974,247 @@ test_diff_binary_files() { test_done "$testroot" "$ret" } +test_diff_commits() { + local testroot=`test_init diff_commits` + local commit_id0=`git_show_head $testroot/repo` + alpha_id0=`get_blob_id $testroot/repo "" alpha` + beta_id0=`get_blob_id $testroot/repo "" beta` + + got checkout $testroot/repo $testroot/wt > /dev/null + ret="$?" + if [ "$ret" != "0" ]; then + test_done "$testroot" "$ret" + return 1 + fi + + echo "modified alpha" > $testroot/wt/alpha + (cd $testroot/wt && got rm beta >/dev/null) + echo "new file" > $testroot/wt/new + (cd $testroot/wt && got add new >/dev/null) + (cd $testroot/wt && got commit -m 'committing changes' >/dev/null) + local commit_id1=`git_show_head $testroot/repo` + + alpha_id1=`get_blob_id $testroot/repo "" alpha` + new_id1=`get_blob_id $testroot/repo "" new` + + echo "diff $commit_id0 refs/heads/master" > $testroot/stdout.expected + echo "blob - $alpha_id0" >> $testroot/stdout.expected + echo "blob + $alpha_id1" >> $testroot/stdout.expected + echo '--- alpha' >> $testroot/stdout.expected + echo '+++ alpha' >> $testroot/stdout.expected + echo '@@ -1 +1 @@' >> $testroot/stdout.expected + echo '-alpha' >> $testroot/stdout.expected + echo '+modified alpha' >> $testroot/stdout.expected + echo "blob - $beta_id0 (mode 644)" >> $testroot/stdout.expected + echo 'blob + /dev/null' >> $testroot/stdout.expected + echo '--- beta' >> $testroot/stdout.expected + echo '+++ /dev/null' >> $testroot/stdout.expected + echo '@@ -1 +0,0 @@' >> $testroot/stdout.expected + echo '-beta' >> $testroot/stdout.expected + echo 'blob - /dev/null' >> $testroot/stdout.expected + echo "blob + $new_id1 (mode 644)" >> $testroot/stdout.expected + echo '--- /dev/null' >> $testroot/stdout.expected + echo '+++ new' >> $testroot/stdout.expected + echo '@@ -0,0 +1 @@' >> $testroot/stdout.expected + echo '+new file' >> $testroot/stdout.expected + + (cd $testroot/wt && got diff -c master > $testroot/stdout) + cmp -s $testroot/stdout.expected $testroot/stdout + ret="$?" + if [ "$ret" != "0" ]; then + diff -u $testroot/stdout.expected $testroot/stdout + test_done "$testroot" "$ret" + return 1 + fi + + # same diff with explicit parent commit ID + (cd $testroot/wt && got diff -c $commit_id0 -c master \ + > $testroot/stdout) + cmp -s $testroot/stdout.expected $testroot/stdout + ret="$?" + if [ "$ret" != "0" ]; then + diff -u $testroot/stdout.expected $testroot/stdout + test_done "$testroot" "$ret" + return 1 + fi + + # same diff with commit object IDs + echo "diff $commit_id0 $commit_id1" > $testroot/stdout.expected + echo "blob - $alpha_id0" >> $testroot/stdout.expected + echo "blob + $alpha_id1" >> $testroot/stdout.expected + echo '--- alpha' >> $testroot/stdout.expected + echo '+++ alpha' >> $testroot/stdout.expected + echo '@@ -1 +1 @@' >> $testroot/stdout.expected + echo '-alpha' >> $testroot/stdout.expected + echo '+modified alpha' >> $testroot/stdout.expected + echo "blob - $beta_id0 (mode 644)" >> $testroot/stdout.expected + echo 'blob + /dev/null' >> $testroot/stdout.expected + echo '--- beta' >> $testroot/stdout.expected + echo '+++ /dev/null' >> $testroot/stdout.expected + echo '@@ -1 +0,0 @@' >> $testroot/stdout.expected + echo '-beta' >> $testroot/stdout.expected + echo 'blob - /dev/null' >> $testroot/stdout.expected + echo "blob + $new_id1 (mode 644)" >> $testroot/stdout.expected + echo '--- /dev/null' >> $testroot/stdout.expected + echo '+++ new' >> $testroot/stdout.expected + echo '@@ -0,0 +1 @@' >> $testroot/stdout.expected + echo '+new file' >> $testroot/stdout.expected + (cd $testroot/wt && got diff -c $commit_id0 -c $commit_id1 \ + > $testroot/stdout) + cmp -s $testroot/stdout.expected $testroot/stdout + ret="$?" + if [ "$ret" != "0" ]; then + diff -u $testroot/stdout.expected $testroot/stdout + test_done "$testroot" "$ret" + return 1 + fi + + # same diff, filtered by paths + echo "diff $commit_id0 $commit_id1" > $testroot/stdout.expected + echo "blob - $alpha_id0" >> $testroot/stdout.expected + echo "blob + $alpha_id1" >> $testroot/stdout.expected + echo '--- alpha' >> $testroot/stdout.expected + echo '+++ alpha' >> $testroot/stdout.expected + echo '@@ -1 +1 @@' >> $testroot/stdout.expected + echo '-alpha' >> $testroot/stdout.expected + echo '+modified alpha' >> $testroot/stdout.expected + (cd $testroot/repo && got diff -c $commit_id0 -c $commit_id1 alpha \ + > $testroot/stdout) + cmp -s $testroot/stdout.expected $testroot/stdout + ret="$?" + if [ "$ret" != "0" ]; then + diff -u $testroot/stdout.expected $testroot/stdout + test_done "$testroot" "$ret" + return 1 + fi + # same in a work tree + (cd $testroot/wt && got diff -c $commit_id0 -c $commit_id1 alpha \ + > $testroot/stdout) + cmp -s $testroot/stdout.expected $testroot/stdout + ret="$?" + if [ "$ret" != "0" ]; then + diff -u $testroot/stdout.expected $testroot/stdout + test_done "$testroot" "$ret" + return 1 + fi + + echo "diff $commit_id0 $commit_id1" > $testroot/stdout.expected + echo "blob - $beta_id0 (mode 644)" >> $testroot/stdout.expected + echo 'blob + /dev/null' >> $testroot/stdout.expected + echo '--- beta' >> $testroot/stdout.expected + echo '+++ /dev/null' >> $testroot/stdout.expected + echo '@@ -1 +0,0 @@' >> $testroot/stdout.expected + echo '-beta' >> $testroot/stdout.expected + echo 'blob - /dev/null' >> $testroot/stdout.expected + echo "blob + $new_id1 (mode 644)" >> $testroot/stdout.expected + echo '--- /dev/null' >> $testroot/stdout.expected + echo '+++ new' >> $testroot/stdout.expected + echo '@@ -0,0 +1 @@' >> $testroot/stdout.expected + echo '+new file' >> $testroot/stdout.expected + (cd $testroot/repo && got diff -c $commit_id0 -c $commit_id1 \ + beta new > $testroot/stdout) + cmp -s $testroot/stdout.expected $testroot/stdout + ret="$?" + if [ "$ret" != "0" ]; then + diff -u $testroot/stdout.expected $testroot/stdout + test_done "$testroot" "$ret" + return 1 + fi + + # more than two -c options are not allowed + (cd $testroot/repo && got diff -c $commit_id0 -c $commit_id1 -c foo \ + 2> $testroot/stderr) + ret="$?" + if [ "$ret" == "0" ]; then + echo "diff succeeded unexpectedly" >&2 + test_done "$testroot" "1" + return 1 + fi + echo "got: too many -c options used" > $testroot/stderr.expected + cmp -s $testroot/stderr.expected $testroot/stderr + ret="$?" + if [ "$ret" != "0" ]; then + diff -u $testroot/stderr.expected $testroot/stderr + test_done "$testroot" "$ret" + return 1 + fi + + # use of -c options implies a repository diff; use with -P is an error + (cd $testroot/wt && got diff -c $commit_id0 -c $commit_id1 -P foo \ + 2> $testroot/stderr) + ret="$?" + if [ "$ret" == "0" ]; then + echo "diff succeeded unexpectedly" >&2 + test_done "$testroot" "1" + return 1 + fi + echo "got: -P option can only be used when diffing a work tree" \ + > $testroot/stderr.expected + cmp -s $testroot/stderr.expected $testroot/stderr + ret="$?" + if [ "$ret" != "0" ]; then + diff -u $testroot/stderr.expected $testroot/stderr + test_done "$testroot" "$ret" + return 1 + fi + + # use of -c options implies a repository diff; use with -s is an error + (cd $testroot/wt && got diff -c $commit_id0 -c $commit_id1 -s foo \ + 2> $testroot/stderr) + ret="$?" + if [ "$ret" == "0" ]; then + echo "diff succeeded unexpectedly" >&2 + test_done "$testroot" "1" + return 1 + fi + echo "got: -s option can only be used when diffing a work tree" \ + > $testroot/stderr.expected + cmp -s $testroot/stderr.expected $testroot/stderr + ret="$?" + if [ "$ret" != "0" ]; then + diff -u $testroot/stderr.expected $testroot/stderr + test_done "$testroot" "$ret" + return 1 + fi + + # three arguments imply use of path filtering (repository case) + (cd $testroot/repo && got diff $commit_id0 $commit_id1 foo \ + 2> $testroot/stderr) + ret="$?" + if [ "$ret" == "0" ]; then + echo "diff succeeded unexpectedly" >&2 + test_done "$testroot" "1" + return 1 + fi + echo "got: specified paths cannot be resolved: no got work tree found" \ + > $testroot/stderr.expected + cmp -s $testroot/stderr.expected $testroot/stderr + ret="$?" + if [ "$ret" != "0" ]; then + diff -u $testroot/stderr.expected $testroot/stderr + test_done "$testroot" "$ret" + return 1 + fi + + # three arguments imply use of path filtering (work tree case) + (cd $testroot/wt && got diff $commit_id0 $commit_id1 foo \ + 2> $testroot/stderr) + ret="$?" + if [ "$ret" == "0" ]; then + echo "diff succeeded unexpectedly" >&2 + test_done "$testroot" "1" + return 1 + fi + echo "got: $commit_id0: No such file or directory" \ + > $testroot/stderr.expected + cmp -s $testroot/stderr.expected $testroot/stderr + ret="$?" + if [ "$ret" != "0" ]; then + diff -u $testroot/stderr.expected $testroot/stderr + fi + test_done "$testroot" "$ret" +} + test_parseargs "$@" run_test test_diff_basic run_test test_diff_shows_conflict @@ -984,3 +1225,4 @@ run_test test_diff_submodule_of_same_repo run_test test_diff_symlinks_in_work_tree run_test test_diff_symlinks_in_repo run_test test_diff_binary_files +run_test test_diff_commits blob - 30214c79d2ad736c245d38dd73a745826ee4b7a0 blob + 810503c77404623c48136692ab83b09ef677b70f --- tog/tog.c +++ tog/tog.c @@ -3340,7 +3340,7 @@ create_diff(struct tog_diff_view_state *s) break; case GOT_OBJ_TYPE_TREE: err = got_diff_objects_as_trees(&s->line_offsets, &s->nlines, - s->id1, s->id2, "", "", s->diff_context, + s->id1, s->id2, NULL, "", "", s->diff_context, s->ignore_whitespace, s->force_text_diff, s->repo, s->f); break; case GOT_OBJ_TYPE_COMMIT: { @@ -3375,7 +3375,7 @@ create_diff(struct tog_diff_view_state *s) got_object_commit_close(commit2); err = got_diff_objects_as_commits(&s->line_offsets, &s->nlines, - s->id1, s->id2, s->diff_context, s->ignore_whitespace, + s->id1, s->id2, NULL, s->diff_context, s->ignore_whitespace, s->force_text_diff, s->repo, s->f); break; }

- 2021-10-08 20:45 Christian Weisgerber:
  got diff: add support for multiple path arguments
- - 2021-10-08 22:27 Stefan Sperling:
    got diff: add support for multiple path arguments
- 2021-10-09 20:29 Stefan Sperling:
  got diff: add support for multiple path arguments
2021-10-08 23:02 Stefan Sperling:
got diff: add support for multiple path arguments