Merge branch 'ct/diff-with-merge-base-clarification'

"git diff" used to take arguments in random and nonsense range
notation, e.g. "git diff A..B C", "git diff A..B C...D", etc.,
which has been cleaned up.

* ct/diff-with-merge-base-clarification:
  Documentation: usage for diff combined commits
  git diff: improve range handling
  t/t3430: avoid undefined git diff behavior
This commit is contained in:
Junio C Hamano 2020-06-25 12:27:46 -07:00
Родитель 53674699c0 b7e10b2ca2
Коммит 1457886ce2
4 изменённых файлов: 226 добавлений и 19 удалений

Просмотреть файл

@ -11,15 +11,17 @@ SYNOPSIS
[verse]
'git diff' [<options>] [<commit>] [--] [<path>...]
'git diff' [<options>] --cached [<commit>] [--] [<path>...]
'git diff' [<options>] <commit> <commit> [--] [<path>...]
'git diff' [<options>] <commit> [<commit>...] <commit> [--] [<path>...]
'git diff' [<options>] <commit>...<commit> [--] [<path>...]
'git diff' [<options>] <blob> <blob>
'git diff' [<options>] --no-index [--] <path> <path>
DESCRIPTION
-----------
Show changes between the working tree and the index or a tree, changes
between the index and a tree, changes between two trees, changes between
two blob objects, or changes between two files on disk.
between the index and a tree, changes between two trees, changes resulting
from a merge, changes between two blob objects, or changes between two
files on disk.
'git diff' [<options>] [--] [<path>...]::
@ -67,6 +69,15 @@ two blob objects, or changes between two files on disk.
one side is omitted, it will have the same effect as
using HEAD instead.
'git diff' [<options>] <commit> [<commit>...] <commit> [--] [<path>...]::
This form is to view the results of a merge commit. The first
listed <commit> must be the merge itself; the remaining two or
more commits should be its parents. A convenient way to produce
the desired set of revisions is to use the {caret}@ suffix.
For instance, if `master` names a merge commit, `git diff master
master^@` gives the same combined diff as `git show master`.
'git diff' [<options>] <commit>\...<commit> [--] [<path>...]::
This form is to view the changes on the branch containing
@ -196,7 +207,8 @@ linkgit:git-difftool[1],
linkgit:git-log[1],
linkgit:gitdiffcore[7],
linkgit:git-format-patch[1],
linkgit:git-apply[1]
linkgit:git-apply[1],
linkgit:git-show[1]
GIT
---

Просмотреть файл

@ -6,6 +6,7 @@
#define USE_THE_INDEX_COMPATIBILITY_MACROS
#include "cache.h"
#include "config.h"
#include "ewah/ewok.h"
#include "lockfile.h"
#include "color.h"
#include "commit.h"
@ -23,7 +24,13 @@
#define DIFF_NO_INDEX_IMPLICIT 2
static const char builtin_diff_usage[] =
"git diff [<options>] [<commit> [<commit>]] [--] [<path>...]";
"git diff [<options>] [<commit>] [--] [<path>...]\n"
" or: git diff [<options>] --cached [<commit>] [--] [<path>...]\n"
" or: git diff [<options>] <commit> [<commit>...] <commit> [--] [<path>...]\n"
" or: git diff [<options>] <commit>...<commit>] [--] [<path>...]\n"
" or: git diff [<options>] <blob> <blob>]\n"
" or: git diff [<options>] --no-index [--] <path> <path>]\n"
COMMON_DIFF_OPTIONS_HELP;
static const char *blob_path(struct object_array_entry *entry)
{
@ -254,6 +261,108 @@ static int builtin_diff_files(struct rev_info *revs, int argc, const char **argv
return 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,
* weed out all bogus range-based revision specifications, e.g.,
* "git diff A..B C..D" or "git diff A..B C" get rejected.
*
* For an actual symmetric diff, *symdiff is set this way:
*
* - its skip is non-NULL and marks *all* rev->pending.objects[i]
* indices that the caller should ignore (extra merge bases, of
* which there might be many, and A in A...B). Note that the
* chosen merge base and right side are NOT marked.
* - warn is set if there are multiple merge bases.
* - base, left, and right point to the names to use in a
* warning about multiple merge bases.
*
* If there is no symmetric diff argument, sym->skip is NULL and
* sym->warn is cleared. The remaining fields are not set.
*/
static void symdiff_prepare(struct rev_info *rev, struct symdiff *sym)
{
int i, is_symdiff = 0, basecount = 0, othercount = 0;
int lpos = -1, rpos = -1, basepos = -1;
struct bitmap *map = NULL;
/*
* Use the whence fields to find merge bases and left and
* right parts of symmetric difference, so that we do not
* depend on the order that revisions are parsed. If there
* are any revs that aren't from these sources, we have a
* "git diff C A...B" or "git diff A...B C" case. Or we
* could even get "git diff A...B C...E", for instance.
*
* If we don't have just one merge base, we pick one
* at random.
*
* NB: REV_CMD_LEFT, REV_CMD_RIGHT are also used for A..B,
* so we must check for SYMMETRIC_LEFT too. The two arrays
* rev->pending.objects and rev->cmdline.rev are parallel.
*/
for (i = 0; i < rev->cmdline.nr; i++) {
struct object *obj = rev->pending.objects[i].item;
switch (rev->cmdline.rev[i].whence) {
case REV_CMD_MERGE_BASE:
if (basepos < 0)
basepos = i;
basecount++;
break; /* do mark all bases */
case REV_CMD_LEFT:
if (lpos >= 0)
usage(builtin_diff_usage);
lpos = i;
if (obj->flags & SYMMETRIC_LEFT) {
is_symdiff = 1;
break; /* do mark A */
}
continue;
case REV_CMD_RIGHT:
if (rpos >= 0)
usage(builtin_diff_usage);
rpos = i;
continue; /* don't mark B */
case REV_CMD_PARENTS_ONLY:
case REV_CMD_REF:
case REV_CMD_REV:
othercount++;
continue;
}
if (map == NULL)
map = bitmap_new();
bitmap_set(map, i);
}
/*
* Forbid any additional revs for both A...B and A..B.
*/
if (lpos >= 0 && othercount > 0)
usage(builtin_diff_usage);
if (!is_symdiff) {
bitmap_free(map);
sym->warn = 0;
sym->skip = NULL;
return;
}
sym->left = rev->pending.objects[lpos].name;
sym->right = rev->pending.objects[rpos].name;
sym->base = rev->pending.objects[basepos].name;
if (basecount == 0)
die(_("%s...%s: no merge base"), sym->left, sym->right);
bitmap_unset(map, basepos); /* unmark the base we want */
sym->warn = basecount > 1;
sym->skip = map;
}
int cmd_diff(int argc, const char **argv, const char *prefix)
{
int i;
@ -263,6 +372,7 @@ int cmd_diff(int argc, const char **argv, const char *prefix)
struct object_array_entry *blob[2];
int nongit = 0, no_index = 0;
int result = 0;
struct symdiff sdiff;
/*
* We could get N tree-ish in the rev.pending_objects list.
@ -382,6 +492,7 @@ int cmd_diff(int argc, const char **argv, const char *prefix)
}
}
symdiff_prepare(&rev, &sdiff);
for (i = 0; i < rev.pending.nr; i++) {
struct object_array_entry *entry = &rev.pending.objects[i];
struct object *obj = entry->item;
@ -396,6 +507,8 @@ int cmd_diff(int argc, const char **argv, const char *prefix)
obj = &get_commit_tree(((struct commit *)obj))->object;
if (obj->type == OBJ_TREE) {
if (sdiff.skip && bitmap_get(sdiff.skip, i))
continue;
obj->flags |= flags;
add_object_array(obj, name, &ent);
} else if (obj->type == OBJ_BLOB) {
@ -437,21 +550,12 @@ int cmd_diff(int argc, const char **argv, const char *prefix)
usage(builtin_diff_usage);
else if (ent.nr == 1)
result = builtin_diff_index(&rev, argc, argv);
else if (ent.nr == 2)
else if (ent.nr == 2) {
if (sdiff.warn)
warning(_("%s...%s: multiple merge bases, using %s"),
sdiff.left, sdiff.right, sdiff.base);
result = builtin_diff_tree(&rev, argc, argv,
&ent.objects[0], &ent.objects[1]);
else if (ent.objects[0].item->flags & UNINTERESTING) {
/*
* diff A...B where there is at least one merge base
* between A and B. We have ent.objects[0] ==
* merge-base, ent.objects[ents-2] == A, and
* ent.objects[ents-1] == B. Show diff between the
* base and B. Note that we pick one merge base at
* random if there are more than one.
*/
result = builtin_diff_tree(&rev, argc, argv,
&ent.objects[0],
&ent.objects[ent.nr-1]);
} else
result = builtin_diff_combined(&rev, argc, argv,
ent.objects, ent.nr);

Просмотреть файл

@ -420,7 +420,7 @@ test_expect_success 'with --autosquash and --exec' '
git commit --fixup B B.t &&
write_script show.sh <<-\EOF &&
subject="$(git show -s --format=%s HEAD)"
content="$(git diff HEAD^! | tail -n 1)"
content="$(git diff HEAD^ HEAD | tail -n 1)"
echo "$subject: $content"
EOF
test_tick &&

91
t/t4068-diff-symmetric.sh Executable file
Просмотреть файл

@ -0,0 +1,91 @@
#!/bin/sh
test_description='behavior of diff with symmetric-diff setups'
. ./test-lib.sh
# build these situations:
# - normal merge with one merge base (br1...b2r);
# - criss-cross merge ie 2 merge bases (br1...master);
# - disjoint subgraph (orphan branch, br3...master).
#
# B---E <-- master
# / \ /
# A X
# \ / \
# C---D--G <-- br1
# \ /
# ---F <-- br2
#
# H <-- br3
#
# We put files into a few commits so that we can verify the
# output as well.
test_expect_success setup '
git commit --allow-empty -m A &&
echo b >b &&
git add b &&
git commit -m B &&
git checkout -b br1 HEAD^ &&
echo c >c &&
git add c &&
git commit -m C &&
git tag commit-C &&
git merge -m D master &&
git tag commit-D &&
git checkout master &&
git merge -m E commit-C &&
git checkout -b br2 commit-C &&
echo f >f &&
git add f &&
git commit -m F &&
git checkout br1 &&
git merge -m G br2 &&
git checkout --orphan br3 &&
git commit -m H
'
test_expect_success 'diff with one merge base' '
git diff commit-D...br1 >tmp &&
tail -n 1 tmp >actual &&
echo +f >expect &&
test_cmp expect actual
'
# The output (in tmp) can have +b or +c depending
# on which merge base (commit B or C) is picked.
# It should have one of those two, which comes out
# to seven lines.
test_expect_success 'diff with two merge bases' '
git diff br1...master >tmp 2>err &&
test_line_count = 7 tmp &&
test_line_count = 1 err
'
test_expect_success 'diff with no merge bases' '
test_must_fail git diff br2...br3 >tmp 2>err &&
test_i18ngrep "fatal: br2...br3: no merge base" err
'
test_expect_success 'diff with too many symmetric differences' '
test_must_fail git diff br1...master br2...br3 >tmp 2>err &&
test_i18ngrep "usage" err
'
test_expect_success 'diff with symmetric difference and extraneous arg' '
test_must_fail git diff master br1...master >tmp 2>err &&
test_i18ngrep "usage" err
'
test_expect_success 'diff with two ranges' '
test_must_fail git diff master br1..master br2..br3 >tmp 2>err &&
test_i18ngrep "usage" err
'
test_expect_success 'diff with ranges and extra arg' '
test_must_fail git diff master br1..master commit-D >tmp 2>err &&
test_i18ngrep "usage" err
'
test_done