{"thread":{"id":"60524","subject":"[RFC PATCH 1/2] diff: add tests for git diff --merge/--ours/--theirs","startedAt":"2023-11-15T12:04:33Z","lastAt":"2023-11-15T13:03:48Z","messageCount":3,"participants":["Vegard Nossum","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"484913","messageId":"20231115120417.1327259-1-vegard.nossum@oracle.com","threadId":"60524","inReplyTo":null,"subject":"[RFC PATCH 1/2] diff: add tests for git diff --merge/--ours/--theirs","fromName":"Vegard Nossum","fromEmail":"vegard.nossum@oracle.com","sentAt":"2023-11-15T12:04:16Z","receivedAt":"2023-11-15T12:04:33Z","isPatch":true,"sender":{"key":"vegard.nossum@oracle.com","avatar":"https://avatars.githubusercontent.com/u/24173?v=4"},"body":"These options don't seem to have any tests currently and the next\npatch in this series changes how these options are parsed. Add tests.\n\nBased loosely on t/t6417-merge-ours-theirs.sh.\n\nSigned-off-by: Vegard Nossum <vegard.nossum@oracle.com>\n---\n t/t4070-diff-merge.sh | 79 +++++++++++++++++++++++++++++++++++++++++++\n 1 file changed, 79 insertions(+)\n create mode 100755 t/t4070-diff-merge.sh\n\ndiff --git a/t/t4070-diff-merge.sh b/t/t4070-diff-merge.sh\nnew file mode 100755\nindex 0000000000..01ac82f0c4\n--- /dev/null\n+++ b/t/t4070-diff-merge.sh\n@@ -0,0 +1,79 @@\n+#!/bin/sh\n+\n+test_description='git diff --merge/--ours/--theirs'\n+GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main\n+export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME\n+\n+TEST_PASSES_SANITIZE_LEAK=true\n+. ./test-lib.sh\n+\n+test_expect_success setup '\n+\ttest_write_lines initial >file &&\n+\tgit add file &&\n+\tgit commit -m initial &&\n+\n+\tgit checkout -b ours main &&\n+\ttest_write_lines ours >file &&\n+\tgit commit -a -m ours &&\n+\n+\tgit checkout -b theirs main &&\n+\ttest_write_lines theirs >file &&\n+\tgit commit -a -m theirs &&\n+\n+\tgit checkout ours^0 &&\n+\ttest_must_fail git merge theirs &&\n+\n+\tINITIAL=$(git rev-parse main) &&\n+\tOURS=$(git rev-parse ours) &&\n+\tTHEIRS=$(git rev-parse theirs)\n+'\n+\n+test_expect_success 'git diff --merge' '\n+\tgit diff --merge | grep -v ^index >actual &&\n+\tcat >expect <<-\\EOF &&\n+\tdiff --cc file\n+\t--- a/file\n+\t+++ b/file\n+\t@@@ -1,1 -1,1 +1,1 @@@\n+\t- theirs\n+\t -initial\n+\t++ours\n+\tEOF\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'git diff --ours' '\n+\tgit diff --ours | grep -v ^index >actual &&\n+\tcat >expect <<-\\EOF &&\n+\t* Unmerged path file\n+\tdiff --git a/file b/file\n+\t--- a/file\n+\t+++ b/file\n+\t@@ -1 +1,5 @@\n+\t+<<<<<<< HEAD\n+\t ours\n+\t+=======\n+\t+theirs\n+\t+>>>>>>> theirs\n+\tEOF\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'git diff --theirs' '\n+\tgit diff --theirs | grep -v ^index >actual &&\n+\tcat >expect <<-\\EOF &&\n+\t* Unmerged path file\n+\tdiff --git a/file b/file\n+\t--- a/file\n+\t+++ b/file\n+\t@@ -1 +1,5 @@\n+\t+<<<<<<< HEAD\n+\t+ours\n+\t+=======\n+\t theirs\n+\t+>>>>>>> theirs\n+\tEOF\n+\ttest_cmp expect actual\n+'\n+\n+test_done\n-- \n2.34.1\n\n"},{"id":"484914","messageId":"20231115120417.1327259-2-vegard.nossum@oracle.com","threadId":"60524","inReplyTo":"20231115120417.1327259-1-vegard.nossum@oracle.com","subject":"[RFC PATCH 2/2] rev-list: add --ours/--theirs options","fromName":"Vegard Nossum","fromEmail":"vegard.nossum@oracle.com","sentAt":"2023-11-15T12:04:17Z","receivedAt":"2023-11-15T12:04:34Z","isPatch":true,"sender":{"key":"vegard.nossum@oracle.com","avatar":"https://avatars.githubusercontent.com/u/24173?v=4"},"body":"When resolving merge conflicts, it is useful to be able to inspect the\ncommits on either side of the attempted merge. git log/rev-list already\nhave the --merge option, which shows these commits; however, this doesn't\ntell you which side each commit appears on.\n\nAdd --ours and --theirs to view the commits from\n\n  $(git merge-base HEAD MERGE_HEAD)..HEAD and\n  $(git merge-base HEAD MERGE_HEAD)..MERGE_HEAD\n\nrespectively.\n\nI didn't see any existing tests for git rev-list --merge, but I've used\nt/t6417-merge-ours-theirs.sh as a template for the new tests, which also\ninclude --merge.\n\nSince git diff/diff-files have their own --ours/--theirs parsing and\nhandling, we need a mechanism to prevent the generic revision parsing\nfrom consuming these arguments. The mechanism I came up with was adding\na new flag, 'ignore_ours_theirs', to 'struct setup_revision_opt' that\nthese two commands must set. It's admittedly not extremely elegant, but\nI didn't see a better solution.\n\nSigned-off-by: Vegard Nossum <vegard.nossum@oracle.com>\n---\n Documentation/rev-list-options.txt |  8 ++++++\n builtin/diff-files.c               |  9 +++++-\n builtin/diff.c                     | 10 ++++++-\n revision.c                         | 16 ++++++++---\n revision.h                         |  6 ++--\n t/t6440-rev-list-merge.sh          | 45 ++++++++++++++++++++++++++++++\n 6 files changed, 86 insertions(+), 8 deletions(-)\n create mode 100755 t/t6440-rev-list-merge.sh\n\ndiff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt\nindex 2bf239ff03..1a75420190 100644\n--- a/Documentation/rev-list-options.txt\n+++ b/Documentation/rev-list-options.txt\n@@ -344,6 +344,14 @@ Under `--pretty=reference`, this information will not be shown at all.\n \tAfter a failed merge, show refs that touch files having a\n \tconflict and don't exist on all heads to merge.\n \n+--ours::\n+\tThe same as --merge, but only show commits from \"our branch\"\n+\t(`HEAD`).\n+\n+--theirs::\n+\tThe same as --merge, but only show commits from \"their branch\"\n+\t(`MERGE_HEAD`).\n+\n --boundary::\n \tOutput excluded boundary commits. Boundary commits are\n \tprefixed with `-`.\ndiff --git a/builtin/diff-files.c b/builtin/diff-files.c\nindex f38912cd40..64f3b1d284 100644\n--- a/builtin/diff-files.c\n+++ b/builtin/diff-files.c\n@@ -20,6 +20,7 @@ COMMON_DIFF_OPTIONS_HELP;\n \n int cmd_diff_files(int argc, const char **argv, const char *prefix)\n {\n+\tstruct setup_revision_opt opt = { 0 };\n \tstruct rev_info rev;\n \tint result;\n \tunsigned options = 0;\n@@ -43,7 +44,13 @@ int cmd_diff_files(int argc, const char **argv, const char *prefix)\n \n \tprefix = precompose_argv_prefix(argc, argv, prefix);\n \n-\targc = setup_revisions(argc, argv, &rev, NULL);\n+\t/*\n+\t * We have our own handling for --ours/--theirs, so don't\n+\t * restrict the revision ranges/paths.\n+\t */\n+\topt.ignore_ours_theirs = 1;\n+\n+\targc = setup_revisions(argc, argv, &rev, &opt);\n \twhile (1 < argc && argv[1][0] == '-') {\n \t\tif (!strcmp(argv[1], \"--base\"))\n \t\t\trev.max_count = 1;\ndiff --git a/builtin/diff.c b/builtin/diff.c\nindex 55e7d21755..ab2388b5bc 100644\n--- a/builtin/diff.c\n+++ b/builtin/diff.c\n@@ -393,6 +393,7 @@ static void symdiff_prepare(struct rev_info *rev, struct symdiff *sym)\n int cmd_diff(int argc, const char **argv, const char *prefix)\n {\n \tint i;\n+\tstruct setup_revision_opt opt = { 0 };\n \tstruct rev_info rev;\n \tstruct object_array ent = OBJECT_ARRAY_INIT;\n \tint first_non_parent = -1;\n@@ -499,7 +500,14 @@ int cmd_diff(int argc, const char **argv, const char *prefix)\n \n \tif (nongit)\n \t\tdie(_(\"Not a git repository\"));\n-\targc = setup_revisions(argc, argv, &rev, NULL);\n+\n+\t/*\n+\t * We have our own handling for --ours/--theirs, so don't\n+\t * restrict the revision ranges/paths.\n+\t */\n+\topt.ignore_ours_theirs = 1;\n+\n+\targc = setup_revisions(argc, argv, &rev, &opt);\n \tif (!rev.diffopt.output_format) {\n \t\trev.diffopt.output_format = DIFF_FORMAT_PATCH;\n \t\tdiff_setup_done(&rev.diffopt);\ndiff --git a/revision.c b/revision.c\nindex 00d5c29bfc..070b5dd73b 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -1978,8 +1978,10 @@ static void prepare_show_merge(struct rev_info *revs)\n \tif (repo_get_oid(the_repository, \"MERGE_HEAD\", &oid))\n \t\tdie(\"--merge without MERGE_HEAD?\");\n \tother = lookup_commit_or_die(&oid, \"MERGE_HEAD\");\n-\tadd_pending_object(revs, &head->object, \"HEAD\");\n-\tadd_pending_object(revs, &other->object, \"MERGE_HEAD\");\n+\tif (revs->show_merge_ours)\n+\t\tadd_pending_object(revs, &head->object, \"HEAD\");\n+\tif (revs->show_merge_theirs)\n+\t\tadd_pending_object(revs, &other->object, \"MERGE_HEAD\");\n \tbases = repo_get_merge_bases(the_repository, head, other);\n \tadd_rev_cmdline_list(revs, bases, REV_CMD_MERGE_BASE, UNINTERESTING | BOTTOM);\n \tadd_pending_commit_list(revs, bases, UNINTERESTING | BOTTOM);\n@@ -2231,6 +2233,7 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\n \tconst char *optarg = NULL;\n \tint argcount;\n \tconst unsigned hexsz = the_hash_algo->hexsz;\n+\tint parse_ours_theirs = !(opt && opt->ignore_ours_theirs);\n \n \t/* pseudo revision arguments */\n \tif (!strcmp(arg, \"--all\") || !strcmp(arg, \"--branches\") ||\n@@ -2324,7 +2327,12 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\n \t\trevs->def = argv[1];\n \t\treturn 2;\n \t} else if (!strcmp(arg, \"--merge\")) {\n-\t\trevs->show_merge = 1;\n+\t\trevs->show_merge_ours = 1;\n+\t\trevs->show_merge_theirs = 1;\n+\t} else if (parse_ours_theirs && !strcmp(arg, \"--ours\")) {\n+\t\trevs->show_merge_ours = 1;\n+\t} else if (parse_ours_theirs && !strcmp(arg, \"--theirs\")) {\n+\t\trevs->show_merge_theirs = 1;\n \t} else if (!strcmp(arg, \"--topo-order\")) {\n \t\trevs->sort_order = REV_SORT_IN_GRAPH_ORDER;\n \t\trevs->topo_order = 1;\n@@ -2982,7 +2990,7 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s\n \t\trevs->def = opt ? opt->def : NULL;\n \tif (opt && opt->tweak)\n \t\topt->tweak(revs);\n-\tif (revs->show_merge)\n+\tif (revs->show_merge_ours | revs->show_merge_theirs)\n \t\tprepare_show_merge(revs);\n \tif (revs->def && !revs->pending.nr && !revs->rev_input_given) {\n \t\tstruct object_id oid;\ndiff --git a/revision.h b/revision.h\nindex 94c43138bc..63effe69c7 100644\n--- a/revision.h\n+++ b/revision.h\n@@ -253,7 +253,8 @@ struct rev_info {\n \tint\t\tshow_notes;\n \tunsigned int\tshown_one:1,\n \t\t\tshown_dashes:1,\n-\t\t\tshow_merge:1,\n+\t\t\tshow_merge_ours:1,\n+\t\t\tshow_merge_theirs:1,\n \t\t\tshow_notes_given:1,\n \t\t\tshow_notes_by_default:1,\n \t\t\tshow_signature:1,\n@@ -436,7 +437,8 @@ void repo_init_revisions(struct repository *r,\n struct setup_revision_opt {\n \tconst char *def;\n \tvoid (*tweak)(struct rev_info *);\n-\tunsigned int\tassume_dashdash:1,\n+\tunsigned int\tignore_ours_theirs:1,\n+\t\t\tassume_dashdash:1,\n \t\t\tallow_exclude_promisor_objects:1,\n \t\t\tfree_removed_argv_elements:1;\n \tunsigned revarg_opt;\ndiff --git a/t/t6440-rev-list-merge.sh b/t/t6440-rev-list-merge.sh\nnew file mode 100755\nindex 0000000000..d1d6af6f31\n--- /dev/null\n+++ b/t/t6440-rev-list-merge.sh\n@@ -0,0 +1,45 @@\n+#!/bin/sh\n+\n+test_description='git rev-list --merge/--ours/--theirs'\n+GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main\n+export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME\n+\n+TEST_PASSES_SANITIZE_LEAK=true\n+. ./test-lib.sh\n+\n+test_expect_success setup '\n+\ttest_write_lines initial >file &&\n+\tgit add file &&\n+\tgit commit -m initial &&\n+\n+\tgit checkout -b ours main &&\n+\ttest_write_lines ours >file &&\n+\tgit commit -a -m ours &&\n+\n+\tgit checkout -b theirs main &&\n+\ttest_write_lines theirs >file &&\n+\tgit commit -a -m theirs &&\n+\n+\tgit checkout ours^0 &&\n+\ttest_must_fail git merge theirs &&\n+\n+\tINITIAL=$(git rev-parse main) &&\n+\tOURS=$(git rev-parse ours) &&\n+\tTHEIRS=$(git rev-parse theirs)\n+'\n+\n+test_expect_success 'git rev-list --merge' '\n+\tgit rev-parse $OURS $THEIRS >expected &&\n+\tgit rev-list --merge >actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'git rev-list --ours' '\n+\ttest $OURS = $(git rev-list --ours)\n+'\n+\n+test_expect_success 'git rev-list --theirs' '\n+\ttest $THEIRS = $(git rev-list --theirs)\n+'\n+\n+test_done\n-- \n2.34.1\n\n"},{"id":"484916","messageId":"xmqqo7fvw4lx.fsf@gitster.g","threadId":"60524","inReplyTo":"20231115120417.1327259-2-vegard.nossum@oracle.com","subject":"Re: [RFC PATCH 2/2] rev-list: add --ours/--theirs options","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-11-15T13:03:38Z","receivedAt":"2023-11-15T13:03:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Vegard Nossum <vegard.nossum@oracle.com> writes:\n\n> Add --ours and --theirs to view the commits from\n>\n>   $(git merge-base HEAD MERGE_HEAD)..HEAD and\n>   $(git merge-base HEAD MERGE_HEAD)..MERGE_HEAD\n\nThe range you wrote with \"merge-base\" would not work well, when\nthere are multiple merge bases between HEAD and MERGE_HEAD.  \n\nJust saying\n\n\tMERGE_HEAD..HEAD and\n\tHEAD..MERGE_HEAD\n\nor even simpler, saying\n\n\tMERGE_HEAD.. and\n\t..MERGE_HEAD\n\nwould be sufficient, simpler, and more importantly, more correct.\n\n"}]}