From 785a308d02629d7a084ae11059baf6778f01efb8 Mon Sep 17 00:00:00 2001 From: Jon Mathews Date: Thu, 18 Sep 2025 13:49:53 -0500 Subject: [PATCH] diff: pass endpoints to GIT_EXTERNAL_DIFF Git diff is about comparing two endpoints. Pass endpoints as determined by diff argument parsing to external tools via environment variables. External tools receive a path and the contents to compare in temporary files. Tools are unable to display the commits being compared, nor the paths if one was renamed. A common scenario is a GUI client invoking an external tool via difftool. The GUI client knows the selected commits, but few if any clients pass that information to the external tool. Git receives the commits to compare, and is in a position to share the endpoints with the external tool. External tools should be able to display endpoints that enable users to locate the content being compared, ideally in terms of commit objects and paths. Address this problem by exporting the following variables: GIT_DIFF_ENDPOINT_A GIT_DIFF_ENDPOINT_B GIT_DIFF_PATH_A GIT_DIFF_PATH_B The ENDPOINT_* variables are the endpoints as determined by the builtin diff argument parsing. Ideally the values are object ids for commits, but will degrade to tree objects if the arguments are trees. A PATH_* variable is the path of the contents in the corresponding endpoint. For clarity, both variables are set even if the paths are the same. When a blob is added or deleted, the other path is /dev/null. If --relative is specified, the paths are relative. Out of scope: - diffs vs the index, working tree, or no-index: these could be handled in the future by indicating the endpoint is 'index', 'working-tree' or 'no-index'. - merge-commits: the current approach does not consider how to handle more than two endpoints. Environment variables will not be set. - GIT_EXTERNAL_DIFF does not support --submodule=diff, and therefore is not considered at this time. - difftool --dir-diff invokes the external tool on two directories. This is achieved by invoking diff to find the differences. This prevents environment variables from being passed back to difftool. --- builtin/diff.c | 47 ++++++++++++++--- diff.c | 16 ++++++ diff.h | 5 ++ t/t4020-diff-external.sh | 108 +++++++++++++++++++++++++++++++++++---- 4 files changed, 157 insertions(+), 19 deletions(-) diff --git a/builtin/diff.c b/builtin/diff.c index 9a89e25a982abb..df46851b961819 100644 --- a/builtin/diff.c +++ b/builtin/diff.c @@ -25,6 +25,7 @@ #include "setup.h" #include "oid-array.h" #include "tree.h" +#include "hex.h" #define DIFF_NO_INDEX_EXPLICIT 1 #define DIFF_NO_INDEX_IMPLICIT 2 @@ -170,14 +171,27 @@ static void builtin_diff_index(struct rev_info *revs, run_diff_index(revs, option); } +struct symdiff { + struct bitmap *skip; + int warn; + const char *base, *left, *right; + struct object_array_entry *end0, *end1; +}; + static void builtin_diff_tree(struct rev_info *revs, int argc, const char **argv, struct object_array_entry *ent0, - struct object_array_entry *ent1) + struct object_array_entry *ent1, + struct symdiff *sdiff) { const struct object_id *(oid[2]); struct object_id mb_oid; int merge_base = 0; + const struct object_id *(endpoint[2]); + int reverse = 0; + + if (revs->diffopt.flags.reverse_diff) + reverse = 1; while (1 < argc) { const char *arg = argv[1]; @@ -192,6 +206,8 @@ static void builtin_diff_tree(struct rev_info *revs, diff_get_merge_base(revs, &mb_oid); oid[0] = &mb_oid; oid[1] = &revs->pending.objects[1].item->oid; + endpoint[0] = oid[0]; + endpoint[1] = oid[1]; } else { int swap = 0; @@ -203,7 +219,26 @@ static void builtin_diff_tree(struct rev_info *revs, swap = 1; oid[swap] = &ent0->item->oid; oid[1 - swap] = &ent1->item->oid; + + /* + * If this is a symmetric diff, obtain the endpoints from previous + * argument filtering. Otherwise there are only two revs, which are + * the endpoints. + */ + if (sdiff->skip) { + endpoint[swap] = &sdiff->end0->item->oid; + endpoint[1 - swap] = &sdiff->end1->item->oid; + } else { + if (revs->pending.nr != 2) + BUG("unexpected revs->pending.nr: %d", revs->pending.nr); + endpoint[swap] = &revs->pending.objects[0].item->oid; + endpoint[1 - swap] = &revs->pending.objects[1].item->oid; + } } + + revs->diffopt.endpoint.oid[0] = endpoint[reverse]; + revs->diffopt.endpoint.oid[1] = endpoint[1 - reverse]; + diff_tree_oid(oid[0], oid[1], "", &revs->diffopt); log_tree_diff_flush(revs); } @@ -289,12 +324,6 @@ static void builtin_diff_files(struct rev_info *revs, int argc, const char **arg run_diff_files(revs, options); } -struct symdiff { - struct bitmap *skip; - int warn; - const char *base, *left, *right; -}; - /* * Check for symmetric-difference arguments, and if present, arrange * everything we need to know to handle them correctly. As a bonus, @@ -389,6 +418,8 @@ static void symdiff_prepare(struct rev_info *rev, struct symdiff *sym) bitmap_unset(map, basepos); /* unmark the base we want */ sym->warn = basecount > 1; sym->skip = map; + sym->end0 = &rev->pending.objects[basepos]; + sym->end1 = &rev->pending.objects[rpos]; } static void symdiff_release(struct symdiff *sdiff) @@ -621,7 +652,7 @@ int cmd_diff(int argc, warning(_("%s...%s: multiple merge bases, using %s"), sdiff.left, sdiff.right, sdiff.base); builtin_diff_tree(&rev, argc, argv, - &ent.objects[0], &ent.objects[1]); + &ent.objects[0], &ent.objects[1], &sdiff); } else builtin_diff_combined(&rev, argc, argv, ent.objects, ent.nr, diff --git a/diff.c b/diff.c index dca87e164fb615..767b1677e2fcef 100644 --- a/diff.c +++ b/diff.c @@ -46,6 +46,7 @@ #include "setup.h" #include "strmap.h" #include "ws.h" +#include "hash.h" #ifdef NO_FAST_WORKING_DIRECTORY #define FAST_WORKING_DIRECTORY 0 @@ -4406,6 +4407,8 @@ static void run_external_diff(const struct external_diff *pgm, struct diff_queue_struct *q = &diff_queued_diff; int quiet = !(o->output_format & DIFF_FORMAT_PATCH); int rc; + const char *path_one; + const char *path_two; /* * Trivial equality is handled by diff_unmodified_pair() before @@ -4435,6 +4438,19 @@ static void run_external_diff(const struct external_diff *pgm, ++o->diff_path_counter); strvec_pushf(&cmd.env, "GIT_DIFF_PATH_TOTAL=%d", q->nr); + if (o->endpoint.oid[0] && o->endpoint.oid[1]) { + strvec_pushf(&cmd.env, "GIT_DIFF_ENDPOINT_A=%s", + oid_to_hex(o->endpoint.oid[0])); + strvec_pushf(&cmd.env, "GIT_DIFF_ENDPOINT_B=%s", + oid_to_hex(o->endpoint.oid[1])); + + path_one = DIFF_FILE_VALID(one) ? name : "/dev/null"; + path_two = DIFF_FILE_VALID(two) ? (other ? other : name) : "/dev/null"; + + strvec_pushf(&cmd.env, "GIT_DIFF_PATH_A=%s", path_one); + strvec_pushf(&cmd.env, "GIT_DIFF_PATH_B=%s", path_two); + } + diff_free_filespec_data(one); diff_free_filespec_data(two); cmd.use_shell = 1; diff --git a/diff.h b/diff.h index 62e5768a9a379e..5ea8fe754e661b 100644 --- a/diff.h +++ b/diff.h @@ -404,6 +404,11 @@ struct diff_options { struct strmap *additional_path_headers; int no_free; + + /* The end-points of the diff */ + struct { + const struct object_id *oid[2]; + } endpoint; }; unsigned diff_filter_bit(char status); diff --git a/t/t4020-diff-external.sh b/t/t4020-diff-external.sh index c8a23d51483e37..3e9d74c7957cb7 100755 --- a/t/t4020-diff-external.sh +++ b/t/t4020-diff-external.sh @@ -2,21 +2,18 @@ test_description='external diff interface test' +GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main +export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME + . ./test-lib.sh test_expect_success setup ' - test_tick && - echo initial >file && - git add file && - git commit -m initial && + test_commit initial file initial A && - test_tick && - echo second >file && + test_commit second file second B && before=$(git hash-object file) && before=$(git rev-parse --short $before) && - git add file && - git commit -m second && test_tick && echo third >file @@ -250,9 +247,7 @@ test_expect_success 'force diff with "diff"' ' ' test_expect_success 'GIT_EXTERNAL_DIFF with more than one changed files' ' - echo anotherfile > file2 && - git add file2 && - git commit -m "added 2nd file" && + test_commit "added 2nd file" file2 anotherfile C && echo modified >file2 && GIT_EXTERNAL_DIFF=echo git diff ' @@ -336,4 +331,95 @@ test_expect_success 'submodule diff' ' test_cmp expected actual ' +test_expect_success 'setup script for export endpoints' ' + write_script ext-diff-endpoints.sh <<-\EOF + printf "END_A=$GIT_DIFF_ENDPOINT_A " >>revs_and_paths.txt && + printf "END_B=$GIT_DIFF_ENDPOINT_B " >>revs_and_paths.txt && + printf "PATH_A=$GIT_DIFF_PATH_A " >>revs_and_paths.txt && + printf "PATH_B=$GIT_DIFF_PATH_B\n" >>revs_and_paths.txt + EOF +' + +test_expect_success 'setup renamed files' ' + git reset --hard && + + test_seq -f "Line %d" 15 > path0 && + test_commit --append path0 path0 "" P0 && + mv path0 path1 && + git add path0 path1 && + git commit -m "rename path0 to path1" && + git tag P1 && + + mkdir dir && + sed "s/Line 11/line 11/" dir/path2 && + rm -f path1 && + git add path1 dir/path2 && + git commit -m "rename path1 to dir/path2, change contents" && + git tag P2 && + + git checkout -b topic P0 && + sed "s/Line 12/line 12/" path3 && + rm -f path0 && + git add path0 path3 && + git commit -m "rename path0 to path3, change contents" && + git tag T +' + +check_export_endpoints () { + local args= + if test $# -gt 5 + then + args="$1" + shift + else + args="$1 $2" + fi + + local endpoint_a="$1" + local endpoint_B="$2" + local path_a="$3" + local path_b="$4" + local desc="$5" + + test_expect_success "GIT_EXTERNAL_DIFF endpoints are commits, $desc" " + >revs_and_paths.txt && + e1=\$(git rev-parse $endpoint_a) && + e2=\$(git rev-parse $endpoint_B) && + cat >expect <<-EOF && + END_A=\$e1 END_B=\$e2 PATH_A=$path_a PATH_B=$path_b + EOF + + GIT_EXTERNAL_DIFF=./ext-diff-endpoints.sh git diff $args && + test_cmp expect revs_and_paths.txt + " +} + +# NB: inputs are tags or branches, output is always in terms of commits +check_export_endpoints A B file file "file changed" +check_export_endpoints B C /dev/null file2 "file added" +check_export_endpoints C B file2 /dev/null "file deleted" +check_export_endpoints "-R B C" C B file2 /dev/null "-R reverses diff" +check_export_endpoints P0 P1 path0 path1 "path renamed, contents unchanged" +check_export_endpoints P1 P2 path1 dir/path2 "path renamed and contents changed" +check_export_endpoints "P2^^ P2^" P0 P1 path0 path1 "expression resolves to commit" +check_export_endpoints A..B A B file file "range A..B" +check_export_endpoints P0...P2 P0 P2 path0 dir/path2 "merge base range (base is same as left) P0...P2" +check_export_endpoints "-R P0...P2" P2 P0 dir/path2 path0 "merge base reverse -R P0...P2" +check_export_endpoints P2...T P0 T path0 path3 "merge-base range P2...T" +check_export_endpoints "--merge-base P2 T" P0 T path0 path3 "--merge-base P2 T" +check_export_endpoints main...topic P0 T path0 path3 "merge-base range on branches main...topic" +check_export_endpoints "P0 P1 -- \"path1\"" P0 P1 /dev/null path1 "add instead of rename as a result of pathspec scope" +check_export_endpoints "--relative=dir P1 P2" P1 P2 /dev/null path2 "--relative=dir" + +test_expect_success 'GIT_EXTERNAL_DIFF endpoints are trees' ' + >revs_and_paths.txt && + end_a=$(git rev-parse A^{tree}) && + end_b=$(git rev-parse B^{tree}) && + cat >expect <<-EOF && + END_A=$end_a END_B=$end_b PATH_A=file PATH_B=file + EOF + GIT_EXTERNAL_DIFF=./ext-diff-endpoints.sh git diff $end_a $end_b && + test_cmp expect revs_and_paths.txt +' + test_done