The result from "git diff" that compares two blobs, e.g. "git diff
$commit1:$path $commit2:$path", used to be shown with the full
object name as given on the command line, but it is more natural to
use the $path in the output and use it to look up .gitattributes.
* jk/diff-blob:
diff: use blob path for blob/file diffs
diff: use pending "path" if it is available
diff: use the word "path" instead of "name" for blobs
diff: pass whole pending entry in blobinfo
handle_revision_arg: record paths for pending objects
handle_revision_arg: record modes for "a..b" endpoints
t4063: add tests of direct blob diffs
get_sha1_with_context: dynamically allocate oc->path
get_sha1_with_context: always initialize oc->symlink_path
sha1_name: consistently refer to object_context as "oc"
handle_revision_arg: add handle_dotdot() helper
handle_revision_arg: hoist ".." check out of range parsing
handle_revision_arg: stop using "dotdot" as a generic pointer
handle_revision_arg: simplify commit reference lookups
handle_revision_arg: reset "dotdot" consistently
if (unknown_type)
flags |= LOOKUP_UNKNOWN_OBJECT;
- if (get_sha1_with_context(obj_name, 0, oid.hash, &obj_context))
+ if (get_sha1_with_context(obj_name, GET_SHA1_RECORD_PATH,
+ oid.hash, &obj_context))
die("Not a valid object name %s", obj_name);
if (!path)
write_or_die(1, buf, size);
free(buf);
+ free(obj_context.path);
return 0;
}
#define DIFF_NO_INDEX_EXPLICIT 1
#define DIFF_NO_INDEX_IMPLICIT 2
-struct blobinfo {
- struct object_id oid;
- const char *name;
- unsigned mode;
-};
-
static const char builtin_diff_usage[] =
"git diff [<options>] [<commit> [<commit>]] [--] [<path>...]";
+static const char *blob_path(struct object_array_entry *entry)
+{
+ return entry->path ? entry->path : entry->name;
+}
+
static void stuff_change(struct diff_options *opt,
unsigned old_mode, unsigned new_mode,
const struct object_id *old_oid,
const struct object_id *new_oid,
int old_oid_valid,
int new_oid_valid,
- const char *old_name,
- const char *new_name)
+ const char *old_path,
+ const char *new_path)
{
struct diff_filespec *one, *two;
if (DIFF_OPT_TST(opt, REVERSE_DIFF)) {
SWAP(old_mode, new_mode);
SWAP(old_oid, new_oid);
- SWAP(old_name, new_name);
+ SWAP(old_path, new_path);
}
if (opt->prefix &&
- (strncmp(old_name, opt->prefix, opt->prefix_length) ||
- strncmp(new_name, opt->prefix, opt->prefix_length)))
+ (strncmp(old_path, opt->prefix, opt->prefix_length) ||
+ strncmp(new_path, opt->prefix, opt->prefix_length)))
return;
- one = alloc_filespec(old_name);
- two = alloc_filespec(new_name);
+ one = alloc_filespec(old_path);
+ two = alloc_filespec(new_path);
fill_filespec(one, old_oid->hash, old_oid_valid, old_mode);
fill_filespec(two, new_oid->hash, new_oid_valid, new_mode);
static int builtin_diff_b_f(struct rev_info *revs,
int argc, const char **argv,
- struct blobinfo *blob)
+ struct object_array_entry **blob)
{
/* Blob vs file in the working tree*/
struct stat st;
diff_set_mnemonic_prefix(&revs->diffopt, "o/", "w/");
- if (blob[0].mode == S_IFINVALID)
- blob[0].mode = canon_mode(st.st_mode);
+ if (blob[0]->mode == S_IFINVALID)
+ blob[0]->mode = canon_mode(st.st_mode);
stuff_change(&revs->diffopt,
- blob[0].mode, canon_mode(st.st_mode),
- &blob[0].oid, &null_oid,
+ blob[0]->mode, canon_mode(st.st_mode),
+ &blob[0]->item->oid, &null_oid,
1, 0,
- path, path);
+ blob[0]->path ? blob[0]->path : path,
+ path);
diffcore_std(&revs->diffopt);
diff_flush(&revs->diffopt);
return 0;
static int builtin_diff_blobs(struct rev_info *revs,
int argc, const char **argv,
- struct blobinfo *blob)
+ struct object_array_entry **blob)
{
unsigned mode = canon_mode(S_IFREG | 0644);
if (argc > 1)
usage(builtin_diff_usage);
- if (blob[0].mode == S_IFINVALID)
- blob[0].mode = mode;
+ if (blob[0]->mode == S_IFINVALID)
+ blob[0]->mode = mode;
- if (blob[1].mode == S_IFINVALID)
- blob[1].mode = mode;
+ if (blob[1]->mode == S_IFINVALID)
+ blob[1]->mode = mode;
stuff_change(&revs->diffopt,
- blob[0].mode, blob[1].mode,
- &blob[0].oid, &blob[1].oid,
+ blob[0]->mode, blob[1]->mode,
+ &blob[0]->item->oid, &blob[1]->item->oid,
1, 1,
- blob[0].name, blob[1].name);
+ blob_path(blob[0]), blob_path(blob[1]));
diffcore_std(&revs->diffopt);
diff_flush(&revs->diffopt);
return 0;
struct rev_info rev;
struct object_array ent = OBJECT_ARRAY_INIT;
int blobs = 0, paths = 0;
- struct blobinfo blob[2];
+ struct object_array_entry *blob[2];
int nongit = 0, no_index = 0;
int result = 0;
} else if (obj->type == OBJ_BLOB) {
if (2 <= blobs)
die(_("more than two blobs given: '%s'"), name);
- hashcpy(blob[blobs].oid.hash, obj->oid.hash);
- blob[blobs].name = name;
- blob[blobs].mode = entry->mode;
+ blob[blobs] = entry;
blobs++;
} else {
break;
}
- if (get_sha1_with_context(arg, 0, oid.hash, &oc)) {
+ if (get_sha1_with_context(arg, GET_SHA1_RECORD_PATH,
+ oid.hash, &oc)) {
if (seen_dashdash)
die(_("unable to resolve revision: %s"), arg);
break;
if (!seen_dashdash)
verify_non_filename(prefix, arg);
add_object_array_with_path(object, arg, &list, oc.mode, oc.path);
+ free(oc.path);
}
/*
!DIFF_OPT_TST(&rev->diffopt, ALLOW_TEXTCONV))
return stream_blob_to_fd(1, oid, NULL, 0);
- if (get_sha1_with_context(obj_name, 0, oidc.hash, &obj_context))
+ if (get_sha1_with_context(obj_name, GET_SHA1_RECORD_PATH,
+ oidc.hash, &obj_context))
die(_("Not a valid object name %s"), obj_name);
- if (!obj_context.path[0] ||
- !textconv_object(obj_context.path, obj_context.mode, &oidc, 1, &buf, &size))
+ if (!obj_context.path ||
+ !textconv_object(obj_context.path, obj_context.mode, &oidc, 1, &buf, &size)) {
+ free(obj_context.path);
return stream_blob_to_fd(1, oid, NULL, 0);
+ }
if (!buf)
die(_("git show %s: bad file"), obj_name);
write_or_die(1, buf, size);
+ free(obj_context.path);
return 0;
}
struct object_context {
unsigned char tree[20];
- char path[PATH_MAX];
unsigned mode;
/*
* symlink_path is only used by get_tree_entry_follow_symlinks,
* and only for symlinks that point outside the repository.
*/
struct strbuf symlink_path;
+ /*
+ * If GET_SHA1_RECORD_PATH is set, this will record path (if any)
+ * found when resolving the name. The caller is responsible for
+ * releasing the memory.
+ */
+ char *path;
};
#define GET_SHA1_QUIETLY 01
#define GET_SHA1_TREEISH 020
#define GET_SHA1_BLOB 040
#define GET_SHA1_FOLLOW_SYMLINKS 0100
+#define GET_SHA1_RECORD_PATH 0200
#define GET_SHA1_ONLY_TO_DIE 04000
#define GET_SHA1_DISAMBIGUATORS \
extern int get_sha1_treeish(const char *str, unsigned char *sha1);
extern int get_sha1_blob(const char *str, unsigned char *sha1);
extern void maybe_die_on_misspelt_object_name(const char *name, const char *prefix);
-extern int get_sha1_with_context(const char *str, unsigned flags, unsigned char *sha1, struct object_context *orc);
+extern int get_sha1_with_context(const char *str, unsigned flags, unsigned char *sha1, struct object_context *oc);
extern int get_oid(const char *str, struct object_id *oid);
revs->limited = 1;
}
+static int dotdot_missing(const char *arg, char *dotdot,
+ struct rev_info *revs, int symmetric)
+{
+ if (revs->ignore_missing)
+ return 0;
+ /* de-munge so we report the full argument */
+ *dotdot = '.';
+ die(symmetric
+ ? "Invalid symmetric difference expression %s"
+ : "Invalid revision range %s", arg);
+}
+
+static int handle_dotdot_1(const char *arg, char *dotdot,
+ struct rev_info *revs, int flags,
+ int cant_be_filename,
+ struct object_context *a_oc,
+ struct object_context *b_oc)
+{
+ const char *a_name, *b_name;
+ struct object_id a_oid, b_oid;
+ struct object *a_obj, *b_obj;
+ unsigned int a_flags, b_flags;
+ int symmetric = 0;
+ unsigned int flags_exclude = flags ^ (UNINTERESTING | BOTTOM);
+ unsigned int oc_flags = GET_SHA1_COMMITTISH | GET_SHA1_RECORD_PATH;
+
+ a_name = arg;
+ if (!*a_name)
+ a_name = "HEAD";
+
+ b_name = dotdot + 2;
+ if (*b_name == '.') {
+ symmetric = 1;
+ b_name++;
+ }
+ if (!*b_name)
+ b_name = "HEAD";
+
+ if (get_sha1_with_context(a_name, oc_flags, a_oid.hash, a_oc) ||
+ get_sha1_with_context(b_name, oc_flags, b_oid.hash, b_oc))
+ return -1;
+
+ if (!cant_be_filename) {
+ *dotdot = '.';
+ verify_non_filename(revs->prefix, arg);
+ *dotdot = '\0';
+ }
+
+ a_obj = parse_object(a_oid.hash);
+ b_obj = parse_object(b_oid.hash);
+ if (!a_obj || !b_obj)
+ return dotdot_missing(arg, dotdot, revs, symmetric);
+
+ if (!symmetric) {
+ /* just A..B */
+ b_flags = flags;
+ a_flags = flags_exclude;
+ } else {
+ /* A...B -- find merge bases between the two */
+ struct commit *a, *b;
+ struct commit_list *exclude;
+
+ a = lookup_commit_reference(a_obj->oid.hash);
+ b = lookup_commit_reference(b_obj->oid.hash);
+ if (!a || !b)
+ return dotdot_missing(arg, dotdot, revs, symmetric);
+
+ exclude = get_merge_bases(a, b);
+ add_rev_cmdline_list(revs, exclude, REV_CMD_MERGE_BASE,
+ flags_exclude);
+ add_pending_commit_list(revs, exclude, flags_exclude);
+ free_commit_list(exclude);
+
+ b_flags = flags;
+ a_flags = flags | SYMMETRIC_LEFT;
+ }
+
+ a_obj->flags |= a_flags;
+ b_obj->flags |= b_flags;
+ add_rev_cmdline(revs, a_obj, a_name, REV_CMD_LEFT, a_flags);
+ add_rev_cmdline(revs, b_obj, b_name, REV_CMD_RIGHT, b_flags);
+ add_pending_object_with_path(revs, a_obj, a_name, a_oc->mode, a_oc->path);
+ add_pending_object_with_path(revs, b_obj, b_name, b_oc->mode, b_oc->path);
+ return 0;
+}
+
+static int handle_dotdot(const char *arg,
+ struct rev_info *revs, int flags,
+ int cant_be_filename)
+{
+ struct object_context a_oc, b_oc;
+ char *dotdot = strstr(arg, "..");
+ int ret;
+
+ if (!dotdot)
+ return -1;
+
+ memset(&a_oc, 0, sizeof(a_oc));
+ memset(&b_oc, 0, sizeof(b_oc));
+
+ *dotdot = '\0';
+ ret = handle_dotdot_1(arg, dotdot, revs, flags, cant_be_filename,
+ &a_oc, &b_oc);
+ *dotdot = '.';
+
+ free(a_oc.path);
+ free(b_oc.path);
+
+ return ret;
+}
+
int handle_revision_arg(const char *arg_, struct rev_info *revs, int flags, unsigned revarg_opt)
{
struct object_context oc;
- char *dotdot;
+ char *mark;
struct object *object;
unsigned char sha1[20];
int local_flags;
const char *arg = arg_;
int cant_be_filename = revarg_opt & REVARG_CANNOT_BE_FILENAME;
- unsigned get_sha1_flags = 0;
+ unsigned get_sha1_flags = GET_SHA1_RECORD_PATH;
flags = flags & UNINTERESTING ? flags | BOTTOM : flags & ~BOTTOM;
- dotdot = strstr(arg, "..");
- if (dotdot) {
- unsigned char from_sha1[20];
- const char *next = dotdot + 2;
- const char *this = arg;
- int symmetric = *next == '.';
- unsigned int flags_exclude = flags ^ (UNINTERESTING | BOTTOM);
- static const char head_by_default[] = "HEAD";
- unsigned int a_flags;
-
- *dotdot = 0;
- next += symmetric;
-
- if (!*next)
- next = head_by_default;
- if (dotdot == arg)
- this = head_by_default;
- if (this == head_by_default && next == head_by_default &&
- !symmetric) {
- /*
- * Just ".."? That is not a range but the
- * pathspec for the parent directory.
- */
- if (!cant_be_filename) {
- *dotdot = '.';
- return -1;
- }
- }
- if (!get_sha1_committish(this, from_sha1) &&
- !get_sha1_committish(next, sha1)) {
- struct object *a_obj, *b_obj;
-
- if (!cant_be_filename) {
- *dotdot = '.';
- verify_non_filename(revs->prefix, arg);
- }
-
- a_obj = parse_object(from_sha1);
- b_obj = parse_object(sha1);
- if (!a_obj || !b_obj) {
- missing:
- if (revs->ignore_missing)
- return 0;
- die(symmetric
- ? "Invalid symmetric difference expression %s"
- : "Invalid revision range %s", arg);
- }
-
- if (!symmetric) {
- /* just A..B */
- a_flags = flags_exclude;
- } else {
- /* A...B -- find merge bases between the two */
- struct commit *a, *b;
- struct commit_list *exclude;
-
- a = (a_obj->type == OBJ_COMMIT
- ? (struct commit *)a_obj
- : lookup_commit_reference(a_obj->oid.hash));
- b = (b_obj->type == OBJ_COMMIT
- ? (struct commit *)b_obj
- : lookup_commit_reference(b_obj->oid.hash));
- if (!a || !b)
- goto missing;
- exclude = get_merge_bases(a, b);
- add_rev_cmdline_list(revs, exclude,
- REV_CMD_MERGE_BASE,
- flags_exclude);
- add_pending_commit_list(revs, exclude,
- flags_exclude);
- free_commit_list(exclude);
-
- a_flags = flags | SYMMETRIC_LEFT;
- }
-
- a_obj->flags |= a_flags;
- b_obj->flags |= flags;
- add_rev_cmdline(revs, a_obj, this,
- REV_CMD_LEFT, a_flags);
- add_rev_cmdline(revs, b_obj, next,
- REV_CMD_RIGHT, flags);
- add_pending_object(revs, a_obj, this);
- add_pending_object(revs, b_obj, next);
- return 0;
- }
- *dotdot = '.';
+ if (!cant_be_filename && !strcmp(arg, "..")) {
+ /*
+ * Just ".."? That is not a range but the
+ * pathspec for the parent directory.
+ */
+ return -1;
}
- dotdot = strstr(arg, "^@");
- if (dotdot && !dotdot[2]) {
- *dotdot = 0;
+ if (!handle_dotdot(arg, revs, flags, revarg_opt))
+ return 0;
+
+ mark = strstr(arg, "^@");
+ if (mark && !mark[2]) {
+ *mark = 0;
if (add_parents_only(revs, arg, flags, 0))
return 0;
- *dotdot = '^';
+ *mark = '^';
}
- dotdot = strstr(arg, "^!");
- if (dotdot && !dotdot[2]) {
- *dotdot = 0;
+ mark = strstr(arg, "^!");
+ if (mark && !mark[2]) {
+ *mark = 0;
if (!add_parents_only(revs, arg, flags ^ (UNINTERESTING | BOTTOM), 0))
- *dotdot = '^';
+ *mark = '^';
}
- dotdot = strstr(arg, "^-");
- if (dotdot) {
+ mark = strstr(arg, "^-");
+ if (mark) {
int exclude_parent = 1;
- if (dotdot[2]) {
+ if (mark[2]) {
char *end;
- exclude_parent = strtoul(dotdot + 2, &end, 10);
+ exclude_parent = strtoul(mark + 2, &end, 10);
if (*end != '\0' || !exclude_parent)
return -1;
}
- *dotdot = 0;
+ *mark = 0;
if (!add_parents_only(revs, arg, flags ^ (UNINTERESTING | BOTTOM), exclude_parent))
- *dotdot = '^';
+ *mark = '^';
}
local_flags = 0;
}
if (revarg_opt & REVARG_COMMITTISH)
- get_sha1_flags = GET_SHA1_COMMITTISH;
+ get_sha1_flags |= GET_SHA1_COMMITTISH;
if (get_sha1_with_context(arg, get_sha1_flags, sha1, &oc))
return revs->ignore_missing ? 0 : -1;
verify_non_filename(revs->prefix, arg);
object = get_reference(revs, arg, sha1, flags ^ local_flags);
add_rev_cmdline(revs, object, arg_, REV_CMD_REV, flags ^ local_flags);
- add_pending_object_with_mode(revs, object, arg, oc.mode);
+ add_pending_object_with_path(revs, object, arg, oc.mode, oc.path);
+ free(oc.path);
return 0;
}
memset(oc, 0, sizeof(*oc));
oc->mode = S_IFINVALID;
+ strbuf_init(&oc->symlink_path, 0);
ret = get_sha1_1(name, namelen, sha1, flags);
if (!ret)
return ret;
namelen = strlen(cp);
}
- strlcpy(oc->path, cp, sizeof(oc->path));
+ if (flags & GET_SHA1_RECORD_PATH)
+ oc->path = xstrdup(cp);
if (!active_cache)
read_cache();
}
}
hashcpy(oc->tree, tree_sha1);
- strlcpy(oc->path, filename, sizeof(oc->path));
+ if (flags & GET_SHA1_RECORD_PATH)
+ oc->path = xstrdup(filename);
free(new_filename);
return ret;
get_sha1_with_context_1(name, GET_SHA1_ONLY_TO_DIE, prefix, sha1, &oc);
}
-int get_sha1_with_context(const char *str, unsigned flags, unsigned char *sha1, struct object_context *orc)
+int get_sha1_with_context(const char *str, unsigned flags, unsigned char *sha1, struct object_context *oc)
{
if (flags & GET_SHA1_FOLLOW_SYMLINKS && flags & GET_SHA1_ONLY_TO_DIE)
die("BUG: incompatible flags for get_sha1_with_context");
- return get_sha1_with_context_1(str, flags, NULL, sha1, orc);
+ return get_sha1_with_context_1(str, flags, NULL, sha1, oc);
}
--- /dev/null
+#!/bin/sh
+
+test_description='test direct comparison of blobs via git-diff'
+. ./test-lib.sh
+
+run_diff () {
+ # use full-index to make it easy to match the index line
+ git diff --full-index "$@" >diff
+}
+
+check_index () {
+ grep "^index $1\\.\\.$2" diff
+}
+
+check_mode () {
+ grep "^old mode $1" diff &&
+ grep "^new mode $2" diff
+}
+
+check_paths () {
+ grep "^diff --git a/$1 b/$2" diff
+}
+
+test_expect_success 'create some blobs' '
+ echo one >one &&
+ echo two >two &&
+ chmod +x two &&
+ git add . &&
+
+ # cover systems where modes are ignored
+ git update-index --chmod=+x two &&
+
+ git commit -m base &&
+
+ sha1_one=$(git rev-parse HEAD:one) &&
+ sha1_two=$(git rev-parse HEAD:two)
+'
+
+test_expect_success 'diff by sha1' '
+ run_diff $sha1_one $sha1_two
+'
+test_expect_success 'index of sha1 diff' '
+ check_index $sha1_one $sha1_two
+'
+test_expect_success 'sha1 diff uses arguments as paths' '
+ check_paths $sha1_one $sha1_two
+'
+test_expect_success 'sha1 diff has no mode change' '
+ ! grep mode diff
+'
+
+test_expect_success 'diff by tree:path (run)' '
+ run_diff HEAD:one HEAD:two
+'
+test_expect_success 'index of tree:path diff' '
+ check_index $sha1_one $sha1_two
+'
+test_expect_success 'tree:path diff uses filenames as paths' '
+ check_paths one two
+'
+test_expect_success 'tree:path diff shows mode change' '
+ check_mode 100644 100755
+'
+
+test_expect_success 'diff by ranged tree:path' '
+ run_diff HEAD:one..HEAD:two
+'
+test_expect_success 'index of ranged tree:path diff' '
+ check_index $sha1_one $sha1_two
+'
+test_expect_success 'ranged tree:path diff uses filenames as paths' '
+ check_paths one two
+'
+test_expect_success 'ranged tree:path diff shows mode change' '
+ check_mode 100644 100755
+'
+
+test_expect_success 'diff blob against file' '
+ run_diff HEAD:one two
+'
+test_expect_success 'index of blob-file diff' '
+ check_index $sha1_one $sha1_two
+'
+test_expect_success 'blob-file diff uses filename as paths' '
+ check_paths one two
+'
+test_expect_success FILEMODE 'blob-file diff shows mode change' '
+ check_mode 100644 100755
+'
+
+test_expect_success 'blob-file diff prefers filename to sha1' '
+ run_diff $sha1_one two &&
+ check_paths two two
+'
+
+test_done
test_cmp expect actual
'
+test_expect_success 'log --source paints symmetric ranges' '
+ cat >expect <<-\EOF &&
+ 09e12a9 source-b three
+ 8e393e1 source-a two
+ EOF
+ git log --oneline --source source-a...source-b >actual &&
+ test_cmp expect actual
+'
+
test_done
int i;
init_tree_desc(&t, NULL, 0UL);
- strbuf_init(result_path, 0);
strbuf_addstr(&namebuf, name);
hashcpy(current_tree_sha1, tree_sha1);