* Broken directory pathname pruning..
@ 2005-05-27 0:41 Linus Torvalds
2005-05-27 0:42 ` Junio C Hamano
` (2 more replies)
0 siblings, 3 replies; 45+ messages in thread
From: Linus Torvalds @ 2005-05-27 0:41 UTC (permalink / raw)
To: Junio C Hamano, Git Mailing List
The directory-based pathname pruning doesn't work any more.
I used to just say
git-whatchanged -v -p drivers/usb/
and I just noticed that it doesn't work any more.
It _does_ work if I leave the final '/' off the thing.
diff-tree itself does this right: that's shown by the fact that with the
slash in place, we still do get the right _changelog_ that is restricted
to drivers/usb changes, but the diffs themselves are missing.
So it seems to be purely "diffcore_pathspec()" that is broken.
Btw, I don't think we should call "diffcore_pathspec()" at all in
diff-tree, because diff-tree already handles "interesting" paths correctly
(and has to do so for performance reasons, since it's not acceptable to
expand all the trees and diff them).
So in the "git-whatchanged" case the fix is as simple as just removing
those two lines, but the bug then continues to exist in the other *diff*
programs..
Junio?
Linus
diff --git a/diff-tree.c b/diff-tree.c
--- a/diff-tree.c
+++ b/diff-tree.c
@@ -268,8 +268,6 @@ static int call_diff_flush(void)
diff_flush(DIFF_FORMAT_NO_OUTPUT, 0);
return 0;
}
- if (nr_paths)
- diffcore_pathspec(paths);
if (header) {
if (diff_output_format == DIFF_FORMAT_MACHINE) {
const char *ep, *cp;
^ permalink raw reply [flat|nested] 45+ messages in thread* Re: Broken directory pathname pruning.. 2005-05-27 0:41 Broken directory pathname pruning Linus Torvalds @ 2005-05-27 0:42 ` Junio C Hamano 2005-05-27 0:49 ` [PATCH] allow pathspec to end with a slash Junio C Hamano 2005-05-27 6:41 ` [PATCH] Diff updates, fixing pathspec and rename/copy interaction Junio C Hamano 2 siblings, 0 replies; 45+ messages in thread From: Junio C Hamano @ 2005-05-27 0:42 UTC (permalink / raw) To: Linus Torvalds; +Cc: Git Mailing List Agreed in both counts. ^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH] allow pathspec to end with a slash 2005-05-27 0:41 Broken directory pathname pruning Linus Torvalds 2005-05-27 0:42 ` Junio C Hamano @ 2005-05-27 0:49 ` Junio C Hamano 2005-05-27 0:52 ` [PATCH] allow pathspec to end with a slash (take #2) Junio C Hamano 2005-05-27 6:41 ` [PATCH] Diff updates, fixing pathspec and rename/copy interaction Junio C Hamano 2 siblings, 1 reply; 45+ messages in thread From: Junio C Hamano @ 2005-05-27 0:49 UTC (permalink / raw) To: Linus Torvalds; +Cc: Git Mailing List Recent rewrite broke "git-whatchanged -v -p drivers/usb/" but "git-whatchanged -v -p drivers/usb" still works. Just strip out the trailing slashes internally to make it work again. Signed-off-by: Junio C Hamano <junkio@cox.net> --- # - linus: [PATCH] Make ls-* output consistent with diff-* output format. # + (working tree) diff --git a/diffcore-pathspec.c b/diffcore-pathspec.c --- a/diffcore-pathspec.c +++ b/diffcore-pathspec.c @@ -45,8 +45,12 @@ void diffcore_pathspec(const char **path speccnt = i; spec = xmalloc(sizeof(*spec) * speccnt); for (i = 0; pathspec[i]; i++) { + int l; spec[i].spec = pathspec[i]; - spec[i].len = strlen(pathspec[i]); + l = strlen(pathspec[i]); + while (pathspec[i][l-1] == '/') + l--; + spec[i].len = l; } for (i = 0; i < q->nr; i++) { Compilation finished at Thu May 26 17:46:56 ^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH] allow pathspec to end with a slash (take #2) 2005-05-27 0:49 ` [PATCH] allow pathspec to end with a slash Junio C Hamano @ 2005-05-27 0:52 ` Junio C Hamano 0 siblings, 0 replies; 45+ messages in thread From: Junio C Hamano @ 2005-05-27 0:52 UTC (permalink / raw) To: Linus Torvalds; +Cc: Git Mailing List Oops, wrong patch; I am an idiot. ------------ Recent rewrite broke "git-whatchanged -v -p drivers/usb/" but "git-whatchanged -v -p drivers/usb" still works. Just strip out the trailing slashes internally to make it work again. It uses compare-thing-with-number comparison order instead of visual comparison order ;-). Signed-off-by: Junio C Hamano <junkio@cox.net> --- cd /opt/packrat/playpen/public/in-place/git/git.junio/ jit-diff 1: diffcore-pathspec.c # - linus: [PATCH] Make ls-* output consistent with diff-* output format. # + (working tree) diff --git a/diffcore-pathspec.c b/diffcore-pathspec.c --- a/diffcore-pathspec.c +++ b/diffcore-pathspec.c @@ -45,8 +45,12 @@ void diffcore_pathspec(const char **path speccnt = i; spec = xmalloc(sizeof(*spec) * speccnt); for (i = 0; pathspec[i]; i++) { + int l; spec[i].spec = pathspec[i]; - spec[i].len = strlen(pathspec[i]); + l = strlen(pathspec[i]); + while (l > 0 && pathspec[i][l-1] == '/') + l--; + spec[i].len = l; } for (i = 0; i < q->nr; i++) { Compilation finished at Thu May 26 17:50:40 ^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH] Diff updates, fixing pathspec and rename/copy interaction. 2005-05-27 0:41 Broken directory pathname pruning Linus Torvalds 2005-05-27 0:42 ` Junio C Hamano 2005-05-27 0:49 ` [PATCH] allow pathspec to end with a slash Junio C Hamano @ 2005-05-27 6:41 ` Junio C Hamano 2005-05-27 15:56 ` Linus Torvalds 2 siblings, 1 reply; 45+ messages in thread From: Junio C Hamano @ 2005-05-27 6:41 UTC (permalink / raw) To: Linus Torvalds; +Cc: Git Mailing List During the mailing list discussion about diff-tree omitting to call diffcore-pathspec, I realized that the current rename/copy differentiator has a major flaw interacting with pathspec (or any other filepair filters, including pickaxe). The problem was that in order to tell if the rename-copy source still remains in the resulting tree (that is what determines if one of the rename-copy can become a rename or everybody needs to be a copy), diffcore-rename was sending a filepair that records unmodified source downstream, expecting that it reaches resolve_rename_copy() which happens as the final stage before actual output happens. Of course, pathspec and pickaxe can interfere and happily remove that entry, in which case what should be shown as a copy suddenly becomes a rename. The problem was fixed by changing the way diffcore-rename records whether the source file remains in the resulting tree. This patch also introduces an optimization for diff-tree -M and diff-tree -C to throw away implausible rename/copy source candidate by keeping the filesize information of already seen SHA1 object IDs in a cache. This cache is activated only when diff-tree is reading from --stdin (i.e. processing sequence of tree pairs) and when rename detection is used. Pickaxe acquired another option, --pickaxe-all, to diff-* three brothers. When a search string is touched, instead of showing only the changed file that touches that searched string, it shows the entire changeset. This is useful to see the change in context. Signed-off-by: Junio C Hamano <junkio@cox.net> --- diff-cache.c | 13 ++ diff-files.c | 11 +- diff-helper.c | 9 +- diff-tree.c | 16 ++- diff.c | 209 ++++++++++++++++++++++++++++++----------------- diff.h | 18 ++-- diffcore-pathspec.c | 5 - diffcore-pickaxe.c | 79 +++++++++++++---- diffcore-rename.c | 91 +++++++++----------- diffcore.h | 14 +-- git-external-diff-script | 2 t/t4007-rename-3.sh | 103 +++++++++++++++++++++++ 12 files changed, 392 insertions(+), 178 deletions(-) new file (100755): t/t4007-rename-3.sh diff --git a/diff-cache.c b/diff-cache.c --- a/diff-cache.c +++ b/diff-cache.c @@ -5,9 +5,10 @@ static int cached_only = 0; static int diff_output_format = DIFF_FORMAT_HUMAN; static int match_nonexisting = 0; static int detect_rename = 0; -static int reverse_diff = 0; +static int diff_setup_opt = 0; static int diff_score_opt = 0; static const char *pickaxe = NULL; +static int pickaxe_opts = 0; /* A file entry went away or appeared */ static void show_file(const char *prefix, struct cache_entry *ce, unsigned char *sha1, unsigned int mode) @@ -202,13 +203,17 @@ int main(int argc, const char **argv) continue; } if (!strcmp(arg, "-R")) { - reverse_diff = 1; + diff_setup_opt |= DIFF_SETUP_REVERSE; continue; } if (!strcmp(arg, "-S")) { pickaxe = arg + 2; continue; } + if (!strcmp(arg, "--pickaxe-all")) { + pickaxe_opts = DIFF_PICKAXE_ALL; + continue; + } if (!strcmp(arg, "-m")) { match_nonexisting = 1; continue; @@ -224,7 +229,7 @@ int main(int argc, const char **argv) usage(diff_cache_usage); /* The rest is for paths restriction. */ - diff_setup(reverse_diff); + diff_setup(diff_setup_opt); mark_merge_entries(); @@ -238,7 +243,7 @@ int main(int argc, const char **argv) if (detect_rename) diffcore_rename(detect_rename, diff_score_opt); if (pickaxe) - diffcore_pickaxe(pickaxe); + diffcore_pickaxe(pickaxe, pickaxe_opts); if (pathspec) diffcore_pathspec(pathspec); diff_flush(diff_output_format, 1); diff --git a/diff-files.c b/diff-files.c --- a/diff-files.c +++ b/diff-files.c @@ -11,9 +11,10 @@ static const char *diff_files_usage = static int diff_output_format = DIFF_FORMAT_HUMAN; static int detect_rename = 0; -static int reverse_diff = 0; +static int diff_setup_opt = 0; static int diff_score_opt = 0; static const char *pickaxe = NULL; +static int pickaxe_opts = 0; static int silent = 0; static void show_unmerge(const char *path) @@ -51,9 +52,11 @@ int main(int argc, const char **argv) else if (!strcmp(argv[1], "-z")) diff_output_format = DIFF_FORMAT_MACHINE; else if (!strcmp(argv[1], "-R")) - reverse_diff = 1; + diff_setup_opt |= DIFF_SETUP_REVERSE; else if (!strcmp(argv[1], "-S")) pickaxe = argv[1] + 2; + else if (!strcmp(argv[1], "--pickaxe-all")) + pickaxe_opts = DIFF_PICKAXE_ALL; else if (!strncmp(argv[1], "-M", 2)) { diff_score_opt = diff_scoreopt_parse(argv[1]); detect_rename = DIFF_DETECT_RENAME; @@ -75,7 +78,7 @@ int main(int argc, const char **argv) exit(1); } - diff_setup(reverse_diff); + diff_setup(diff_setup_opt); for (i = 0; i < entries; i++) { struct stat st; @@ -116,7 +119,7 @@ int main(int argc, const char **argv) if (detect_rename) diffcore_rename(detect_rename, diff_score_opt); if (pickaxe) - diffcore_pickaxe(pickaxe); + diffcore_pickaxe(pickaxe, pickaxe_opts); if (1 < argc) diffcore_pathspec(argv + 1); diff_flush(diff_output_format, 1); diff --git a/diff-helper.c b/diff-helper.c --- a/diff-helper.c +++ b/diff-helper.c @@ -4,9 +4,9 @@ #include "cache.h" #include "strbuf.h" #include "diff.h" -#include "diffcore.h" /* just for MAX_SCORE */ static const char *pickaxe = NULL; +static int pickaxe_opts = 0; static int line_termination = '\n'; static int inter_name_termination = '\t'; @@ -24,6 +24,8 @@ int main(int ac, const char **av) { else if (av[1][1] == 'S') { pickaxe = av[1] + 2; } + else if (!strcmp(av[1], "--pickaxe-all")) + pickaxe_opts = DIFF_PICKAXE_ALL; else usage(diff_helper_usage); ac--; av++; @@ -78,7 +80,6 @@ int main(int ac, const char **av) { if (status == 'R' || status == 'C') { two_paths = 1; sscanf(cp, "%d", &score); - score = score * MAX_SCORE / 100; if (line_termination) { cp = strchr(cp, inter_name_termination); @@ -129,14 +130,14 @@ int main(int ac, const char **av) { continue; } if (pickaxe) - diffcore_pickaxe(pickaxe); + diffcore_pickaxe(pickaxe, pickaxe_opts); if (1 < ac) diffcore_pathspec(av + 1); diff_flush(DIFF_FORMAT_PATCH, 0); printf("%s\n", sb.buf); } if (pickaxe) - diffcore_pickaxe(pickaxe); + diffcore_pickaxe(pickaxe, pickaxe_opts); if (1 < ac) diffcore_pathspec(av + 1); diff_flush(DIFF_FORMAT_PATCH, 0); diff --git a/diff-tree.c b/diff-tree.c --- a/diff-tree.c +++ b/diff-tree.c @@ -10,9 +10,10 @@ static int show_tree_entry_in_recursive static int read_stdin = 0; static int diff_output_format = DIFF_FORMAT_HUMAN; static int detect_rename = 0; -static int reverse_diff = 0; +static int diff_setup_opt = 0; static int diff_score_opt = 0; static const char *pickaxe = NULL; +static int pickaxe_opts = 0; static const char *header = NULL; static const char *header_prefix = ""; @@ -255,7 +256,7 @@ static int diff_tree_sha1(const unsigned static void call_diff_setup(void) { - diff_setup(reverse_diff); + diff_setup(diff_setup_opt); } static int call_diff_flush(void) @@ -263,7 +264,7 @@ static int call_diff_flush(void) if (detect_rename) diffcore_rename(detect_rename, diff_score_opt); if (pickaxe) - diffcore_pickaxe(pickaxe); + diffcore_pickaxe(pickaxe, pickaxe_opts); if (diff_queue_is_empty()) { diff_flush(DIFF_FORMAT_NO_OUTPUT, 0); return 0; @@ -504,7 +505,7 @@ int main(int argc, const char **argv) continue; } if (!strcmp(arg, "-R")) { - reverse_diff = 1; + diff_setup_opt |= DIFF_SETUP_REVERSE; continue; } if (!strcmp(arg, "-p")) { @@ -516,6 +517,10 @@ int main(int argc, const char **argv) pickaxe = arg + 2; continue; } + if (!strcmp(arg, "--pickaxe-all")) { + pickaxe_opts = DIFF_PICKAXE_ALL; + continue; + } if (!strncmp(arg, "-M", 2)) { detect_rename = DIFF_DETECT_RENAME; diff_score_opt = diff_scoreopt_parse(arg); @@ -580,6 +585,9 @@ int main(int argc, const char **argv) if (!read_stdin) return 0; + if (detect_rename) + diff_setup_opt |= (DIFF_SETUP_USE_SIZE_CACHE | + DIFF_SETUP_USE_CACHE); while (fgets(line, sizeof(line), stdin)) diff_tree_stdin(line); diff --git a/diff.c b/diff.c --- a/diff.c +++ b/diff.c @@ -12,6 +12,7 @@ static const char *diff_opts = "-pu"; static unsigned char null_sha1[20] = { 0, }; static int reverse_diff; +static int use_size_cache; static const char *external_diff(void) { @@ -141,7 +142,7 @@ static void builtin_diff(const char *nam printf("new mode %s\n", temp[1].mode); } if (xfrm_msg && xfrm_msg[0]) - fputs(xfrm_msg, stdout); + puts(xfrm_msg); if (strncmp(temp[0].mode, temp[1].mode, 3)) /* we do not run diff between different kind @@ -222,12 +223,60 @@ static int work_tree_matches(const char return 1; } +static struct sha1_size_cache { + unsigned char sha1[20]; + unsigned long size; +} **sha1_size_cache; +static int sha1_size_cache_nr, sha1_size_cache_alloc; + +static struct sha1_size_cache *locate_size_cache(unsigned char *sha1, + unsigned long size) +{ + int first, last; + struct sha1_size_cache *e; + + first = 0; + last = sha1_size_cache_nr; + while (last > first) { + int next = (last + first) >> 1; + e = sha1_size_cache[next]; + int cmp = memcmp(e->sha1, sha1, 20); + if (!cmp) + return e; + if (cmp < 0) { + last = next; + continue; + } + first = next+1; + } + /* not found */ + if (size == UINT_MAX) + return NULL; + /* insert to make it at "first" */ + if (sha1_size_cache_alloc <= sha1_size_cache_nr) { + sha1_size_cache_alloc = alloc_nr(sha1_size_cache_alloc); + sha1_size_cache = xrealloc(sha1_size_cache, + sha1_size_cache_alloc * + sizeof(*sha1_size_cache)); + } + sha1_size_cache_nr++; + if (first < sha1_size_cache_nr) + memmove(sha1_size_cache + first + 1, sha1_size_cache + first, + (sha1_size_cache_nr - first - 1) * + sizeof(*sha1_size_cache)); + e = xmalloc(sizeof(struct sha1_size_cache)); + sha1_size_cache[first] = e; + memcpy(e->sha1, sha1, 20); + e->size = size; + return e; +} + /* * While doing rename detection and pickaxe operation, we may need to * grab the data for the blob (or file) for our own in-core comparison. * diff_filespec has data and size fields for this purpose. */ -int diff_populate_filespec(struct diff_filespec *s) +int diff_populate_filespec(struct diff_filespec *s, int size_only) { int err = 0; if (!DIFF_FILE_VALID(s)) @@ -235,6 +284,9 @@ int diff_populate_filespec(struct diff_f if (S_ISDIR(s->mode)) return -1; + if (!use_size_cache) + size_only = 0; + if (s->data) return err; if (!s->sha1_valid || @@ -254,6 +306,8 @@ int diff_populate_filespec(struct diff_f s->size = st.st_size; if (!s->size) goto empty; + if (size_only) + return 0; if (S_ISLNK(st.st_mode)) { int ret; s->data = xmalloc(s->size); @@ -273,9 +327,21 @@ int diff_populate_filespec(struct diff_f close(fd); } else { + /* We cannot do size only for SHA1 blobs */ char type[20]; + struct sha1_size_cache *e; + + if (size_only) { + e = locate_size_cache(s->sha1, UINT_MAX); + if (e) { + s->size = e->size; + return 0; + } + } s->data = read_sha1_file(s->sha1, type, &s->size); s->should_free = 1; + if (s->data && size_only) + locate_size_cache(s->sha1, s->size); } return 0; } @@ -361,7 +427,7 @@ static void prepare_temp_file(const char return; } else { - if (diff_populate_filespec(one)) + if (diff_populate_filespec(one, 0)) die("cannot read data blob for %s", one->path); prep_temp_blob(temp, one->data, one->size, one->sha1, one->mode); @@ -492,9 +558,23 @@ static void run_diff(const char *name, run_external_diff(pgm, name, other, one, two, xfrm_msg); } -void diff_setup(int reverse_diff_) +void diff_setup(int flags) { - reverse_diff = reverse_diff_; + if (flags & DIFF_SETUP_REVERSE) + reverse_diff = 1; + if (flags & DIFF_SETUP_USE_CACHE) { + if (!active_cache) + /* read-cache does not die even when it fails + * so it is safe for us to do this here. Also + * it does not smudge active_cache or active_nr + * when it fails, so we do not have to worry about + * cleaning it up oufselves either. + */ + read_cache(); + } + if (flags & DIFF_SETUP_USE_SIZE_CACHE) + use_size_cache = 1; + } struct diff_queue_struct diff_queued_diff; @@ -517,10 +597,18 @@ struct diff_filepair *diff_queue(struct dp->one = one; dp->two = two; dp->score = 0; + dp->source_stays = 0; diff_q(queue, dp); return dp; } +void diff_free_filepair(struct diff_filepair *p) +{ + diff_free_filespec_data(p->one); + diff_free_filespec_data(p->two); + free(p); +} + static void diff_flush_raw(struct diff_filepair *p, int line_termination, int inter_name_termination) @@ -615,7 +703,7 @@ static void diff_flush_patch(struct diff sprintf(msg_, "similarity index %d%%\n" "copy from %s\n" - "copy to %s\n", + "copy to %s", (int)(0.5 + p->score * 100.0/MAX_SCORE), p->one->path, p->two->path); msg = msg_; @@ -624,7 +712,7 @@ static void diff_flush_patch(struct diff sprintf(msg_, "similarity index %d%%\n" "rename old %s\n" - "rename new %s\n", + "rename new %s", (int)(0.5 + p->score * 100.0/MAX_SCORE), p->one->path, p->two->path); msg = msg_; @@ -639,28 +727,6 @@ static void diff_flush_patch(struct diff run_diff(name, other, p->one, p->two, msg); } -int diff_needs_to_stay(struct diff_queue_struct *q, int i, - struct diff_filespec *it) -{ - /* If it will be used in later entry (either stay or used - * as the source of rename/copy), we need to copy, not rename. - */ - while (i < q->nr) { - struct diff_filepair *p = q->queue[i++]; - if (!DIFF_FILE_VALID(p->two)) - continue; /* removed is fine */ - if (strcmp(p->one->path, it->path)) - continue; /* not relevant */ - - /* p has its src set to *it and it is not a delete; - * it will be used for in-place change, rename/copy, - * or just stays there. We cannot rename it out. - */ - return 1; - } - return 0; -} - int diff_queue_is_empty(void) { struct diff_queue_struct *q = &diff_queued_diff; @@ -689,8 +755,8 @@ void diff_debug_filepair(const struct di { diff_debug_filespec(p->one, i, "one"); diff_debug_filespec(p->two, i, "two"); - fprintf(stderr, "score %d, status %c\n", - p->score, p->status ? : '?'); + fprintf(stderr, "score %d, status %c source_stays %d\n", + p->score, p->status ? : '?', p->source_stays); } void diff_debug_queue(const char *msg, struct diff_queue_struct *q) @@ -712,8 +778,6 @@ static void diff_resolve_rename_copy(voi struct diff_filepair *p, *pp; struct diff_queue_struct *q = &diff_queued_diff; - /* This should not depend on the ordering of things. */ - diff_debug_queue("resolve-rename-copy", q); for (i = 0; i < q->nr; i++) { @@ -721,23 +785,28 @@ static void diff_resolve_rename_copy(voi p->status = 0; /* undecided */ if (DIFF_PAIR_UNMERGED(p)) p->status = 'U'; - else if (!DIFF_FILE_VALID((p)->one)) + else if (!DIFF_FILE_VALID(p->one)) p->status = 'N'; - else if (!DIFF_FILE_VALID((p)->two)) { - /* Deletion record should be omitted if there - * are rename/copy entries using this one as - * the source. Then we can say one of them - * is a rename and the rest are copies. + else if (!DIFF_FILE_VALID(p->two)) { + /* Deleted entry may have been picked up by + * another rename-copy entry. So we scan the + * queue and if we find one that uses us as the + * source we do not say delete for this entry. */ - p->status = 'D'; for (j = 0; j < q->nr; j++) { pp = q->queue[j]; - if (!strcmp(pp->one->path, p->one->path) && - strcmp(pp->one->path, pp->two->path)) { + if (!strcmp(p->one->path, pp->one->path) && + pp->score) { + /* rename/copy are always valid + * so we do not say DIFF_FILE_VALID() + * on pp->one and pp->two. + */ p->status = 'X'; break; } } + if (!p->status) + p->status = 'D'; } else if (DIFF_PAIR_TYPE_CHANGED(p)) p->status = 'T'; @@ -746,33 +815,24 @@ static void diff_resolve_rename_copy(voi * whose both sides are valid and of the same type, i.e. * either in-place edit or rename/copy edit. */ - else if (strcmp(p->one->path, p->two->path)) { - /* See if there is somebody else anywhere that - * will keep the path (either modified or - * unmodified). If so, we have to be a copy, - * not a rename. In addition, if there is - * some other rename or copy that comes later - * than us that uses the same source, we - * have to be a copy, not a rename. + else if (p->score) { + if (p->source_stays) { + p->status = 'C'; + continue; + } + /* See if there is some other filepair that + * copies from the same source as us. If so + * we are a copy. Otherwise we are a rename. */ - for (j = 0; j < q->nr; j++) { + for (j = i + 1; j < q->nr; j++) { pp = q->queue[j]; if (strcmp(pp->one->path, p->one->path)) - continue; - if (!strcmp(pp->one->path, pp->two->path)) { - if (DIFF_FILE_VALID(pp->two)) { - /* non-delete */ - p->status = 'C'; - break; - } - continue; - } - /* pp is a rename/copy ... */ - if (i < j) { - /* ... and comes later than us */ - p->status = 'C'; - break; - } + continue; /* not us */ + if (!pp->score) + continue; /* not a rename/copy */ + /* pp is a rename/copy from the same source */ + p->status = 'C'; + break; } if (!p->status) p->status = 'R'; @@ -781,8 +841,11 @@ static void diff_resolve_rename_copy(voi p->one->mode != p->two->mode) p->status = 'M'; else - /* this is a "no-change" entry */ - p->status = 'X'; + /* this is a "no-change" entry. + * should not happen anymore. + * p->status = 'X'; + */ + die("internal error in diffcore: unmodified entry remains"); } diff_debug_queue("resolve-rename-copy done", q); } @@ -817,12 +880,8 @@ void diff_flush(int diff_output_style, i break; } } - for (i = 0; i < q->nr; i++) { - struct diff_filepair *p = q->queue[i]; - diff_free_filespec_data(p->one); - diff_free_filespec_data(p->two); - free(p); - } + for (i = 0; i < q->nr; i++) + diff_free_filepair(q->queue[i]); free(q->queue); q->queue = NULL; q->nr = q->alloc = 0; @@ -883,7 +942,7 @@ void diff_helper_input(unsigned old_mode if (new_mode) fill_filespec(two, new_sha1, new_mode); dp = diff_queue(&diff_queued_diff, one, two); - dp->score = score; + dp->score = score * MAX_SCORE / 100; dp->status = status; } diff --git a/diff.h b/diff.h --- a/diff.h +++ b/diff.h @@ -28,22 +28,28 @@ extern void diff_unmerge(const char *pat extern int diff_scoreopt_parse(const char *opt); -#define DIFF_FORMAT_HUMAN 0 -#define DIFF_FORMAT_MACHINE 1 -#define DIFF_FORMAT_PATCH 2 -#define DIFF_FORMAT_NO_OUTPUT 3 -extern void diff_setup(int reverse); +#define DIFF_SETUP_REVERSE 1 +#define DIFF_SETUP_USE_CACHE 2 +#define DIFF_SETUP_USE_SIZE_CACHE 4 +extern void diff_setup(int flags); #define DIFF_DETECT_RENAME 1 #define DIFF_DETECT_COPY 2 extern void diffcore_rename(int rename_copy, int minimum_score); -extern void diffcore_pickaxe(const char *needle); +#define DIFF_PICKAXE_ALL 1 +extern void diffcore_pickaxe(const char *needle, int opts); + extern void diffcore_pathspec(const char **pathspec); extern int diff_queue_is_empty(void); +#define DIFF_FORMAT_HUMAN 0 +#define DIFF_FORMAT_MACHINE 1 +#define DIFF_FORMAT_PATCH 2 +#define DIFF_FORMAT_NO_OUTPUT 3 + extern void diff_flush(int output_style, int resolve_rename_copy); #endif /* DIFF_H */ diff --git a/diffcore-pathspec.c b/diffcore-pathspec.c --- a/diffcore-pathspec.c +++ b/diffcore-pathspec.c @@ -55,11 +55,10 @@ void diffcore_pathspec(const char **path for (i = 0; i < q->nr; i++) { struct diff_filepair *p = q->queue[i]; - if (matches_pathspec(p->one->path, spec, speccnt) || - matches_pathspec(p->two->path, spec, speccnt)) + if (matches_pathspec(p->two->path, spec, speccnt)) diff_q(&outq, p); else - free(p); + diff_free_filepair(p); } free(q->queue); *q = outq; diff --git a/diffcore-pickaxe.c b/diffcore-pickaxe.c --- a/diffcore-pickaxe.c +++ b/diffcore-pickaxe.c @@ -11,7 +11,7 @@ static int contains(struct diff_filespec { unsigned long offset, sz; const char *data; - if (diff_populate_filespec(one)) + if (diff_populate_filespec(one, 0)) return 0; sz = one->size; data = one->data; @@ -21,36 +21,73 @@ static int contains(struct diff_filespec return 0; } -void diffcore_pickaxe(const char *needle) +void diffcore_pickaxe(const char *needle, int opts) { struct diff_queue_struct *q = &diff_queued_diff; unsigned long len = strlen(needle); - int i; + int i, has_changes; struct diff_queue_struct outq; outq.queue = NULL; outq.nr = outq.alloc = 0; - for (i = 0; i < q->nr; i++) { - struct diff_filepair *p = q->queue[i]; - int onum = outq.nr; - if (!DIFF_FILE_VALID(p->one)) { - if (!DIFF_FILE_VALID(p->two)) - continue; /* ignore nonsense */ - /* created */ - if (contains(p->two, needle, len)) - diff_q(&outq, p); + if (opts & DIFF_PICKAXE_ALL) { + /* Showing the whole changeset if needle exists */ + for (i = has_changes = 0; !has_changes && i < q->nr; i++) { + struct diff_filepair *p = q->queue[i]; + if (!DIFF_FILE_VALID(p->one)) { + if (!DIFF_FILE_VALID(p->two)) + continue; /* ignore unmerged */ + /* created */ + if (contains(p->two, needle, len)) + has_changes++; + } + else if (!DIFF_FILE_VALID(p->two)) { + if (contains(p->one, needle, len)) + has_changes++; + } + else if (!diff_unmodified_pair(p) && + contains(p->one, needle, len) != + contains(p->two, needle, len)) + has_changes++; } - else if (!DIFF_FILE_VALID(p->two)) { - if (contains(p->one, needle, len)) + if (has_changes) + return; /* not munge the queue */ + + /* otherwise we will clear the whole queue + * by copying the empty outq at the end of this + * function, but first clear the current entries + * in the queue. + */ + for (i = 0; i < q->nr; i++) + diff_free_filepair(q->queue[i]); + } + else + /* Showing only the filepairs that has the needle */ + for (i = 0; i < q->nr; i++) { + struct diff_filepair *p = q->queue[i]; + has_changes = 0; + if (!DIFF_FILE_VALID(p->one)) { + if (!DIFF_FILE_VALID(p->two)) + ; /* ignore unmerged */ + /* created */ + else if (contains(p->two, needle, len)) + has_changes = 1; + } + else if (!DIFF_FILE_VALID(p->two)) { + if (contains(p->one, needle, len)) + has_changes = 1; + } + else if (!diff_unmodified_pair(p) && + contains(p->one, needle, len) != + contains(p->two, needle, len)) + has_changes = 1; + + if (has_changes) diff_q(&outq, p); + else + diff_free_filepair(p); } - else if (!diff_unmodified_pair(p) && - contains(p->one, needle, len) != - contains(p->two, needle, len)) - diff_q(&outq, p); - if (onum == outq.nr) - free(p); - } + free(q->queue); *q = outq; return; diff --git a/diffcore-rename.c b/diffcore-rename.c --- a/diffcore-rename.c +++ b/diffcore-rename.c @@ -52,14 +52,15 @@ static struct diff_rename_dst *locate_re return &(rename_dst[first]); } +/* Table of rename/copy src files */ static struct diff_rename_src { struct diff_filespec *one; - unsigned src_used : 1; + unsigned src_stays : 1; } *rename_src; static int rename_src_nr, rename_src_alloc; -static struct diff_rename_src *locate_rename_src(struct diff_filespec *one, - int insert_ok) +static struct diff_rename_src *register_rename_src(struct diff_filespec *one, + int src_stays) { int first, last; @@ -77,9 +78,7 @@ static struct diff_rename_src *locate_re } first = next+1; } - /* not found */ - if (!insert_ok) - return NULL; + /* insert to make it at "first" */ if (rename_src_alloc <= rename_src_nr) { rename_src_alloc = alloc_nr(rename_src_alloc); @@ -91,7 +90,7 @@ static struct diff_rename_src *locate_re memmove(rename_src + first + 1, rename_src + first, (rename_src_nr - first - 1) * sizeof(*rename_src)); rename_src[first].one = one; - rename_src[first].src_used = 0; + rename_src[first].src_stays = src_stays; return &(rename_src[first]); } @@ -100,8 +99,11 @@ static int is_exact_match(struct diff_fi if (src->sha1_valid && dst->sha1_valid && !memcmp(src->sha1, dst->sha1, 20)) return 1; - if (diff_populate_filespec(src) || diff_populate_filespec(dst)) - /* this is an error but will be caught downstream */ + if (diff_populate_filespec(src, 1) || diff_populate_filespec(dst, 1)) + return 0; + if (src->size != dst->size) + return 0; + if (diff_populate_filespec(src, 0) || diff_populate_filespec(dst, 0)) return 0; if (src->size == dst->size && !memcmp(src->data, dst->data, src->size)) @@ -113,7 +115,6 @@ struct diff_score { int src; /* index in rename_src */ int dst; /* index in rename_dst */ int score; - int rank; }; static int estimate_similarity(struct diff_filespec *src, @@ -127,9 +128,11 @@ static int estimate_similarity(struct di * dst, and then some edit has been applied to dst. * * Compare them and return how similar they are, representing - * the score as an integer between 0 and 10000, except - * where they match exactly it is considered better than anything - * else. + * the score as an integer between 0 and MAX_SCORE. + * + * When there is an exact match, it is considered a better + * match than anything else; the destination does not even + * call into this function in that case. */ void *delta; unsigned long delta_size, base_size; @@ -149,6 +152,7 @@ static int estimate_similarity(struct di /* We would not consider edits that change the file size so * drastically. delta_size must be smaller than * (MAX_SCORE-minimum_score)/MAX_SCORE * min(src->size, dst->size). + * * Note that base_size == 0 case is handled here already * and the final score computation below would not have a * divide-by-zero issue. @@ -156,6 +160,9 @@ static int estimate_similarity(struct di if (base_size * (MAX_SCORE-minimum_score) < delta_size * MAX_SCORE) return 0; + if (diff_populate_filespec(src, 0) || diff_populate_filespec(dst, 0)) + return 0; /* error but caught downstream */ + delta = diff_delta(src->data, src->size, dst->data, dst->size, &delta_size); @@ -163,7 +170,7 @@ static int estimate_similarity(struct di /* A delta that has a lot of literal additions would have * big delta_size no matter what else it does. */ - if (minimum_score < MAX_SCORE * delta_size / base_size) + if (base_size * (MAX_SCORE-minimum_score) < delta_size * MAX_SCORE) return 0; /* Estimate the edit size by interpreting delta. */ @@ -200,15 +207,14 @@ static void record_rename_pair(struct di fill_filespec(two, dst->sha1, dst->mode); dp = diff_queue(renq, one, two); - dp->score = score; - - rename_src[src_index].src_used = 1; + dp->score = score ? : 1; /* make sure it is at least 1 */ + dp->source_stays = rename_src[src_index].src_stays; rename_dst[dst_index].pair = dp; } /* * We sort the rename similarity matrix with the score, in descending - * order (more similar first). + * order (the most similar first). */ static int score_compare(const void *a_, const void *b_) { @@ -223,7 +229,7 @@ int diff_scoreopt_parse(const char *opt) return -1; /* that is not a -M nor -C option */ diglen = strspn(opt+2, "0123456789"); if (diglen == 0 || strlen(opt+2) != diglen) - return 0; /* use default */ + return DEFAULT_MINIMUM_SCORE; /* use default */ sscanf(opt+2, "%d", &num); for (i = 0, scale = 1; i < diglen; i++) scale *= 10; @@ -255,9 +261,9 @@ void diffcore_rename(int detect_rename, else locate_rename_dst(p->two, 1); else if (!DIFF_FILE_VALID(p->two)) - locate_rename_src(p->one, 1); - else if (1 < detect_rename) /* find copy, too */ - locate_rename_src(p->one, 1); + register_rename_src(p->one, 0); + else if (detect_rename == DIFF_DETECT_COPY) + register_rename_src(p->one, 1); } if (rename_dst_nr == 0) goto cleanup; /* nothing to do */ @@ -308,7 +314,7 @@ void diffcore_rename(int detect_rename, if (dst->pair) continue; /* already done, either exact or fuzzy. */ if (mx[i].score < minimum_score) - break; /* there is not any more diffs applicable. */ + break; /* there is no more usable pair. */ record_rename_pair(&renq, mx[i].dst, mx[i].src, mx[i].score); } free(mx); @@ -317,28 +323,21 @@ void diffcore_rename(int detect_rename, flush_rest: /* At this point, we have found some renames and copies and they * are kept in renq. The original list is still in *q. - * - * Scan the original list and move them into the outq; we will sort - * outq and swap it into the queue supplied to pass that to - * downstream, so we assign the sort keys in this loop. - * - * See comments at the top of record_rename_pair for numbers used - * to assign rename_rank. */ outq.queue = NULL; outq.nr = outq.alloc = 0; for (i = 0; i < q->nr; i++) { struct diff_filepair *p = q->queue[i]; - struct diff_rename_src *src = locate_rename_src(p->one, 0); struct diff_rename_dst *dst = locate_rename_dst(p->two, 0); struct diff_filepair *pair_to_free = NULL; if (dst) { /* creation */ if (dst->pair) { - /* renq has rename/copy already to produce - * this file, so we do not emit the creation - * record in the output. + /* renq has rename/copy to produce + * this file already, so we do not + * emit the creation record in the + * output. */ diff_q(&outq, dst->pair); pair_to_free = p; @@ -350,25 +349,17 @@ void diffcore_rename(int detect_rename, diff_q(&outq, p); } else if (!diff_unmodified_pair(p)) - /* all the other cases need to be recorded as is */ + /* all the usual ones need to be kept */ diff_q(&outq, p); - else { - /* unmodified pair needs to be recorded only if - * it is used as the source of rename/copy - */ - if (src && src->src_used) - diff_q(&outq, p); - else - pair_to_free = p; - } - if (pair_to_free) { - diff_free_filespec_data(pair_to_free->one); - diff_free_filespec_data(pair_to_free->two); - free(pair_to_free); - } + else + /* no need to keep unmodified pairs */ + pair_to_free = p; + + if (pair_to_free) + diff_free_filepair(pair_to_free); } diff_debug_queue("done copying original", &outq); - + free(renq.queue); free(q->queue); *q = outq; diff --git a/diffcore.h b/diffcore.h --- a/diffcore.h +++ b/diffcore.h @@ -33,14 +33,17 @@ extern struct diff_filespec *alloc_files extern void fill_filespec(struct diff_filespec *, const unsigned char *, unsigned short); -extern int diff_populate_filespec(struct diff_filespec *); +extern int diff_populate_filespec(struct diff_filespec *, int); extern void diff_free_filespec_data(struct diff_filespec *); struct diff_filepair { struct diff_filespec *one; struct diff_filespec *two; - int score; /* only valid when one and two are different paths */ - int status; /* M C R N D U (see Documentation/diff-format.txt) */ + unsigned short int score; /* only valid when one and two are + * different paths + */ + char source_stays; /* all of R/C are copies */ + char status; /* M C R N D U (see Documentation/diff-format.txt) */ }; #define DIFF_PAIR_UNMERGED(p) \ (!DIFF_FILE_VALID((p)->one) && !DIFF_FILE_VALID((p)->two)) @@ -54,6 +57,8 @@ struct diff_filepair { (S_ISREG(mode) ? (S_IFREG | ce_permissions(mode)) : \ S_ISLNK(mode) ? S_IFLNK : S_IFDIR) +extern void diff_free_filepair(struct diff_filepair *); + extern int diff_unmodified_pair(struct diff_filepair *); struct diff_queue_struct { @@ -68,9 +73,6 @@ extern struct diff_filepair *diff_queue( struct diff_filespec *); extern void diff_q(struct diff_queue_struct *, struct diff_filepair *); -extern int diff_needs_to_stay(struct diff_queue_struct *, int, - struct diff_filespec *); - #define DIFF_DEBUG 0 #if DIFF_DEBUG void diff_debug_filespec(struct diff_filespec *, int, const char *); diff --git a/git-external-diff-script b/git-external-diff-script --- a/git-external-diff-script +++ b/git-external-diff-script @@ -59,7 +59,7 @@ then echo "new mode $mode2" if test "$xfrm_msg" != "" then - echo -n $xfrm_msg + echo "$xfrm_msg" fi fi diff ${GIT_DIFF_OPTS-'-pu'} -L "a/$name1" -L "b/$name2" "$tmp1" "$tmp2" diff --git a/t/t4007-rename-3.sh b/t/t4007-rename-3.sh new file mode 100755 --- /dev/null +++ b/t/t4007-rename-3.sh @@ -0,0 +1,103 @@ +#!/bin/sh +# +# Copyright (c) 2005 Junio C Hamano +# + +test_description='Rename interaction with pathspec. + +' +. ./test-lib.sh + +_x40='[0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f]' +_x40="$_x40$_x40$_x40$_x40$_x40$_x40$_x40$_x40" +sanitize_diff_raw='s/ \('"$_x40"'\) \1 \([CR]\)[0-9]* / \1 \1 \2# /' +compare_diff_raw () { + # When heuristics are improved, the score numbers would change. + # Ignore them while comparing. + # Also we do not check SHA1 hash generation in this test, which + # is a job for t0000-basic.sh + + sed -e "$sanitize_diff_raw" <"$1" >.tmp-1 + sed -e "$sanitize_diff_raw" <"$2" >.tmp-2 + diff -u .tmp-1 .tmp-2 && rm -f .tmp-1 .tmp-2 +} + +test_expect_success \ + 'prepare reference tree' \ + 'mkdir path0 path1 && + cp ../../COPYING path0/COPYING && + git-update-cache --add path0/COPYING && + tree=$(git-write-tree) && + echo $tree' + +test_expect_success \ + 'prepare work tree' \ + 'cp path0/COPYING path1/COPYING && + git-update-cache --add --remove path0/COPYING path1/COPYING' + +# In the tree, there is only path0/COPYING. In the cache, path0 and +# path1 both have COPYING and the latter is a copy of path0/COPYING. +# Comparing the full tree with cache should tell us so. + +git-diff-cache -C $tree >current + +cat >expected <<\EOF +:100644 100644 6ff87c4664981e4397625791c8ea3bbb5f2279a3 6ff87c4664981e4397625791c8ea3bbb5f2279a3 C100 path0/COPYING path1/COPYING +EOF + +test_expect_success \ + 'validate the result' \ + 'compare_diff_raw current expected' + +# In the tree, there is only path0/COPYING. In the cache, path0 and +# path1 both have COPYING and the latter is a copy of path0/COPYING. +# When we omit output from path0 it should still be able to tell us +# that path1/COPYING is result from a copy from path0/COPYING, not +# rename, which would imply path0/COPYING is now gone. + +git-diff-cache -C $tree path1 >current + +cat >expected <<\EOF +:100644 100644 6ff87c4664981e4397625791c8ea3bbb5f2279a3 6ff87c4664981e4397625791c8ea3bbb5f2279a3 C100 path0/COPYING path1/COPYING +EOF + +test_expect_success \ + 'validate the result' \ + 'compare_diff_raw current expected' + +test_expect_success \ + 'tweak work tree' \ + 'rm -f path0/COPYING && + git-update-cache --remove path0/COPYING' + +# In the tree, there is only path0/COPYING. In the cache, path0 does +# not have COPYING anymore and path1 has COPYING which is a copy of +# path0/COPYING. Showing the full tree with cache should tell us about +# the rename. + +git-diff-cache -C $tree >current + +cat >expected <<\EOF +:100644 100644 6ff87c4664981e4397625791c8ea3bbb5f2279a3 6ff87c4664981e4397625791c8ea3bbb5f2279a3 R100 path0/COPYING path1/COPYING +EOF + +test_expect_success \ + 'validate the result' \ + 'compare_diff_raw current expected' + +# In the tree, there is only path0/COPYING. In the cache, path0 does +# not have COPYING anymore and path1 has COPYING which is a copy of +# path0/COPYING. Even if we restrict the output to path1, it still +# should show us the rename. + +git-diff-cache -C $tree path1 >current + +cat >expected <<\EOF +:100644 100644 6ff87c4664981e4397625791c8ea3bbb5f2279a3 6ff87c4664981e4397625791c8ea3bbb5f2279a3 R100 path0/COPYING path1/COPYING +EOF + +test_expect_success \ + 'validate the result' \ + 'compare_diff_raw current expected' + +test_done \ No newline at end of file ------------------------------------------------ ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH] Diff updates, fixing pathspec and rename/copy interaction. 2005-05-27 6:41 ` [PATCH] Diff updates, fixing pathspec and rename/copy interaction Junio C Hamano @ 2005-05-27 15:56 ` Linus Torvalds 2005-05-27 18:22 ` Junio C Hamano 2005-05-27 22:43 ` [PATCH 00/12] Diff updates Junio C Hamano 0 siblings, 2 replies; 45+ messages in thread From: Linus Torvalds @ 2005-05-27 15:56 UTC (permalink / raw) To: Junio C Hamano; +Cc: Git Mailing List On Thu, 26 May 2005, Junio C Hamano wrote: > > During the mailing list discussion about diff-tree omitting to > call diffcore-pathspec, I realized that the current rename/copy > differentiator has a major flaw interacting with pathspec (or > any other filepair filters, including pickaxe). > > The problem was that in order to tell if the rename-copy source > still remains in the resulting tree (that is what determines if > one of the rename-copy can become a rename or everybody needs to > be a copy), diffcore-rename was sending a filepair that records > unmodified source downstream, expecting that it reaches > resolve_rename_copy() which happens as the final stage before > actual output happens. Of course, pathspec and pickaxe can > interfere and happily remove that entry, in which case what > should be shown as a copy suddenly becomes a rename. Umm. I would much prefer a _much_ simpler fix at least for the pathname part, which is to just always require that pathspec handling is done _first_. Why? Because that's fundamentally how git-diff-tree has to work, and it's how my mental model has always been: the path limitations are a first-order filter, and if you give a directory, the end result should always look exactly as if that directory was a project of its own. In other words, if you limit yourself to a directory, and there was a rename that moved a file from outside that directory into it, then it is not a rename at all, it is a "create". The stuff outside the pathspec limit simply doesn't exist. This is fundamentally how git-diff-tree has to work for pathspec to make any sense at all, and it's also the only usage that makes any sense (ie when I say "git-whatchanged -p arch/i386 include/asm-i386", I expect that the patches that show up _only_ concern themselves with what happened in x86, and there's no cross-pollination with other stuff at all, even if rename detection is enabled). That in turn implies that the other pathspec users have to work the same way, for the thing to be consistent. Now, I don't know how you want pickaxe to work, and maybe you want that to run _after_ rename detection, I dunno. So I suspect you still want to do what this patch does, but I really don't want pathspec to be involved in this thing.. Linus ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH] Diff updates, fixing pathspec and rename/copy interaction. 2005-05-27 15:56 ` Linus Torvalds @ 2005-05-27 18:22 ` Junio C Hamano 2005-05-27 22:43 ` [PATCH 00/12] Diff updates Junio C Hamano 1 sibling, 0 replies; 45+ messages in thread From: Junio C Hamano @ 2005-05-27 18:22 UTC (permalink / raw) To: Linus Torvalds; +Cc: Git Mailing List >>>>> "LT" == Linus Torvalds <torvalds@osdl.org> writes: LT> I would much prefer a _much_ simpler fix at least for the pathname part, LT> which is to just always require that pathspec handling is done _first_. LT> Why? Because that's fundamentally how git-diff-tree has to work, and it's LT> how my mental model has always been: the path limitations are a LT> first-order filter, and if you give a directory, the end result should LT> always look exactly as if that directory was a project of its own. Enlightment! My initial thinking was to do rename/copy first, deliberately ignoring pathspec (I was thinking about tweaking diff-tree to break out of its built-in pathspec limit to feed diffcore _everything_, even things outside of the specified directories when --detect-copies-harder is used), to cast a wider net so that it can come up with "the best matches". But what you just said made me realize that the definition of "the best matches" I had in mind was not best at all. I agree with you 100% that pathspec should come first before everything else to limit the world diffcore operates in, not just to limit the set of output paths. I still want to do what this patch does for another reason. It is so much simpler if I do not have to carry around _some_ unmatched pair after rename/copy. The earlier representation was forcing all the downstream diffcore filters to be aware of what rename/copy did, which was simply _wrong_. They should not have to care. About ordering of pickaxe and rename/copy, I think for most uses of diffcore filters, not limited to pickaxe, would make more sense if they come after rename/copy when rename/copy detection is in effect. What I really wanted to do (I made a side note comment in the earlier discussion about this, saying "something like streams", but have not pursued it further) is to make these diffcore filters/transformations stackable so that the main programs can control not just if each of them is used or not, but the order of application of the used ones. I cannot offhand think of a good use case of having pickaxe come _before_ rename/copy, but there may be a case the user want to have things in that order, and having the application order not hardcoded in three diff-* brothers is a major problem if we wanted to give that option. ^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH 00/12] Diff updates 2005-05-27 15:56 ` Linus Torvalds 2005-05-27 18:22 ` Junio C Hamano @ 2005-05-27 22:43 ` Junio C Hamano 2005-05-27 22:49 ` [PATCH 01/12] Fix math thinko in similarity estimator Junio C Hamano ` (12 more replies) 1 sibling, 13 replies; 45+ messages in thread From: Junio C Hamano @ 2005-05-27 22:43 UTC (permalink / raw) To: Linus Torvalds; +Cc: Git Mailing List This series consists of the following 12 patches. Most of them are bugfixes and cleanups. The last one is somewhat iffy, although it does not break things, and lies somewhere between a request for inclusion and a request for comments. [PATCH 01/12] Fix math thinko in similarity estimator. [PATCH 02/12] Introduce diff_free_filepair() funcion. [PATCH 03/12] Make pathspec only care about the detination tree. [PATCH 04/12] Remove unused rank field from diff_core structure. [PATCH 05/12] Do not expose internal scaling to diff-helper. [PATCH 06/12] Remove final newline from the value of xfrm_msg variable. [PATCH 07/12] Clean up diff_setup() to make it more extensible. [PATCH 08/12] Remove a function not used anymore. [PATCH 09/12] Add --pickaxe-all to diff-* brothers. [PATCH 10/12] Fix the way diffcore-rename records unremoved source. [PATCH 11/12] Move pathspec to the beginning of the diffcore chain. [PATCH 12/12] Optimize diff-tree -[CM] --stdin ^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH 01/12] Fix math thinko in similarity estimator. 2005-05-27 22:43 ` [PATCH 00/12] Diff updates Junio C Hamano @ 2005-05-27 22:49 ` Junio C Hamano 2005-05-27 22:50 ` [PATCH 02/12] Introduce diff_free_filepair() funcion Junio C Hamano ` (11 subsequent siblings) 12 siblings, 0 replies; 45+ messages in thread From: Junio C Hamano @ 2005-05-27 22:49 UTC (permalink / raw) To: Linus Torvalds; +Cc: Git Mailing List The math to reject delta that is too big was confused. Signed-off-by: Junio C Hamano <junkio@cox.net> --- diffcore-rename.c | 2 +- 1 files changed, 1 insertion(+), 1 deletion(-) diff --git a/diffcore-rename.c b/diffcore-rename.c --- a/diffcore-rename.c +++ b/diffcore-rename.c @@ -163,7 +163,7 @@ static int estimate_similarity(struct di /* A delta that has a lot of literal additions would have * big delta_size no matter what else it does. */ - if (minimum_score < MAX_SCORE * delta_size / base_size) + if (base_size * (MAX_SCORE-minimum_score) < delta_size * MAX_SCORE) return 0; /* Estimate the edit size by interpreting delta. */ ------------------------------------------------ ^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH 02/12] Introduce diff_free_filepair() funcion. 2005-05-27 22:43 ` [PATCH 00/12] Diff updates Junio C Hamano 2005-05-27 22:49 ` [PATCH 01/12] Fix math thinko in similarity estimator Junio C Hamano @ 2005-05-27 22:50 ` Junio C Hamano 2005-05-27 22:51 ` [PATCH 03/12] Make pathspec only care about the detination tree Junio C Hamano ` (10 subsequent siblings) 12 siblings, 0 replies; 45+ messages in thread From: Junio C Hamano @ 2005-05-27 22:50 UTC (permalink / raw) To: Linus Torvalds; +Cc: Git Mailing List This introduces a new function to free a common data structure, and plugs some leaks. Signed-off-by: Junio C Hamano <junkio@cox.net> --- diff.c | 15 +++++++++------ diffcore-pathspec.c | 2 +- diffcore-pickaxe.c | 2 +- diffcore-rename.c | 7 ++----- diffcore.h | 2 ++ 5 files changed, 15 insertions(+), 13 deletions(-) diff --git a/diff.c b/diff.c --- a/diff.c +++ b/diff.c @@ -521,6 +521,13 @@ struct diff_filepair *diff_queue(struct return dp; } +void diff_free_filepair(struct diff_filepair *p) +{ + diff_free_filespec_data(p->one); + diff_free_filespec_data(p->two); + free(p); +} + static void diff_flush_raw(struct diff_filepair *p, int line_termination, int inter_name_termination) @@ -817,12 +824,8 @@ void diff_flush(int diff_output_style, i break; } } - for (i = 0; i < q->nr; i++) { - struct diff_filepair *p = q->queue[i]; - diff_free_filespec_data(p->one); - diff_free_filespec_data(p->two); - free(p); - } + for (i = 0; i < q->nr; i++) + diff_free_filepair(q->queue[i]); free(q->queue); q->queue = NULL; q->nr = q->alloc = 0; diff --git a/diffcore-pathspec.c b/diffcore-pathspec.c --- a/diffcore-pathspec.c +++ b/diffcore-pathspec.c @@ -59,7 +59,7 @@ void diffcore_pathspec(const char **path matches_pathspec(p->two->path, spec, speccnt)) diff_q(&outq, p); else - free(p); + diff_free_filepair(p); } free(q->queue); *q = outq; diff --git a/diffcore-pickaxe.c b/diffcore-pickaxe.c --- a/diffcore-pickaxe.c +++ b/diffcore-pickaxe.c @@ -49,7 +49,7 @@ void diffcore_pickaxe(const char *needle contains(p->two, needle, len)) diff_q(&outq, p); if (onum == outq.nr) - free(p); + diff_free_filepair(p); } free(q->queue); *q = outq; diff --git a/diffcore-rename.c b/diffcore-rename.c --- a/diffcore-rename.c +++ b/diffcore-rename.c @@ -361,11 +361,8 @@ void diffcore_rename(int detect_rename, else pair_to_free = p; } - if (pair_to_free) { - diff_free_filespec_data(pair_to_free->one); - diff_free_filespec_data(pair_to_free->two); - free(pair_to_free); - } + if (pair_to_free) + diff_free_filepair(pair_to_free); } diff_debug_queue("done copying original", &outq); diff --git a/diffcore.h b/diffcore.h --- a/diffcore.h +++ b/diffcore.h @@ -54,6 +54,8 @@ struct diff_filepair { (S_ISREG(mode) ? (S_IFREG | ce_permissions(mode)) : \ S_ISLNK(mode) ? S_IFLNK : S_IFDIR) +extern void diff_free_filepair(struct diff_filepair *); + extern int diff_unmodified_pair(struct diff_filepair *); struct diff_queue_struct { ------------------------------------------------ ^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH 03/12] Make pathspec only care about the detination tree. 2005-05-27 22:43 ` [PATCH 00/12] Diff updates Junio C Hamano 2005-05-27 22:49 ` [PATCH 01/12] Fix math thinko in similarity estimator Junio C Hamano 2005-05-27 22:50 ` [PATCH 02/12] Introduce diff_free_filepair() funcion Junio C Hamano @ 2005-05-27 22:51 ` Junio C Hamano 2005-05-27 22:52 ` [PATCH 04/12] Remove unused rank field from diff_core structure Junio C Hamano ` (9 subsequent siblings) 12 siblings, 0 replies; 45+ messages in thread From: Junio C Hamano @ 2005-05-27 22:51 UTC (permalink / raw) To: Linus Torvalds; +Cc: Git Mailing List Earlier it had a misguided attempt to include paths that matches either source tree or destination tree after the rename/copy detection. The new semantics will be that pathspec defines a narrowed down world the diffcore operates in, so it should not even look at where in the source tree the path came from. Signed-off-by: Junio C Hamano <junkio@cox.net> --- diffcore-pathspec.c | 3 +-- 1 files changed, 1 insertion(+), 2 deletions(-) diff --git a/diffcore-pathspec.c b/diffcore-pathspec.c --- a/diffcore-pathspec.c +++ b/diffcore-pathspec.c @@ -55,8 +55,7 @@ void diffcore_pathspec(const char **path for (i = 0; i < q->nr; i++) { struct diff_filepair *p = q->queue[i]; - if (matches_pathspec(p->one->path, spec, speccnt) || - matches_pathspec(p->two->path, spec, speccnt)) + if (matches_pathspec(p->two->path, spec, speccnt)) diff_q(&outq, p); else diff_free_filepair(p); ------------------------------------------------ ^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH 04/12] Remove unused rank field from diff_core structure. 2005-05-27 22:43 ` [PATCH 00/12] Diff updates Junio C Hamano ` (2 preceding siblings ...) 2005-05-27 22:51 ` [PATCH 03/12] Make pathspec only care about the detination tree Junio C Hamano @ 2005-05-27 22:52 ` Junio C Hamano 2005-05-27 22:53 ` [PATCH 05/12] Do not expose internal scaling to diff-helper Junio C Hamano ` (8 subsequent siblings) 12 siblings, 0 replies; 45+ messages in thread From: Junio C Hamano @ 2005-05-27 22:52 UTC (permalink / raw) To: Linus Torvalds; +Cc: Git Mailing List This removes a field that is no longer used from diff_score structure. Signed-off-by: Junio C Hamano <junkio@cox.net> --- diffcore-rename.c | 1 - 1 files changed, 1 deletion(-) diff --git a/diffcore-rename.c b/diffcore-rename.c --- a/diffcore-rename.c +++ b/diffcore-rename.c @@ -113,7 +113,6 @@ struct diff_score { int src; /* index in rename_src */ int dst; /* index in rename_dst */ int score; - int rank; }; static int estimate_similarity(struct diff_filespec *src, ------------------------------------------------ ^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH 05/12] Do not expose internal scaling to diff-helper. 2005-05-27 22:43 ` [PATCH 00/12] Diff updates Junio C Hamano ` (3 preceding siblings ...) 2005-05-27 22:52 ` [PATCH 04/12] Remove unused rank field from diff_core structure Junio C Hamano @ 2005-05-27 22:53 ` Junio C Hamano 2005-05-27 22:54 ` [PATCH 06/12] Remove final newline from the value of xfrm_msg variable Junio C Hamano ` (7 subsequent siblings) 12 siblings, 0 replies; 45+ messages in thread From: Junio C Hamano @ 2005-05-27 22:53 UTC (permalink / raw) To: Linus Torvalds; +Cc: Git Mailing List Instead we can normalize what diff-raw records at the diffcore side. Signed-off-by: Junio C Hamano <junkio@cox.net> --- diff-helper.c | 2 -- diff.c | 2 +- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/diff-helper.c b/diff-helper.c --- a/diff-helper.c +++ b/diff-helper.c @@ -4,7 +4,6 @@ #include "cache.h" #include "strbuf.h" #include "diff.h" -#include "diffcore.h" /* just for MAX_SCORE */ static const char *pickaxe = NULL; static int line_termination = '\n'; @@ -78,7 +77,6 @@ int main(int ac, const char **av) { if (status == 'R' || status == 'C') { two_paths = 1; sscanf(cp, "%d", &score); - score = score * MAX_SCORE / 100; if (line_termination) { cp = strchr(cp, inter_name_termination); diff --git a/diff.c b/diff.c --- a/diff.c +++ b/diff.c @@ -886,7 +886,7 @@ void diff_helper_input(unsigned old_mode if (new_mode) fill_filespec(two, new_sha1, new_mode); dp = diff_queue(&diff_queued_diff, one, two); - dp->score = score; + dp->score = score * MAX_SCORE / 100; dp->status = status; } ------------------------------------------------ ^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH 06/12] Remove final newline from the value of xfrm_msg variable. 2005-05-27 22:43 ` [PATCH 00/12] Diff updates Junio C Hamano ` (4 preceding siblings ...) 2005-05-27 22:53 ` [PATCH 05/12] Do not expose internal scaling to diff-helper Junio C Hamano @ 2005-05-27 22:54 ` Junio C Hamano 2005-05-27 22:54 ` [PATCH 07/12] Clean up diff_setup() to make it more extensible Junio C Hamano ` (6 subsequent siblings) 12 siblings, 0 replies; 45+ messages in thread From: Junio C Hamano @ 2005-05-27 22:54 UTC (permalink / raw) To: Linus Torvalds; +Cc: Git Mailing List This change makes the implementation of git-external-diff-script cleaner. Signed-off-by: Junio C Hamano <junkio@cox.net> --- diff.c | 6 +++--- git-external-diff-script | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/diff.c b/diff.c --- a/diff.c +++ b/diff.c @@ -141,7 +141,7 @@ static void builtin_diff(const char *nam printf("new mode %s\n", temp[1].mode); } if (xfrm_msg && xfrm_msg[0]) - fputs(xfrm_msg, stdout); + puts(xfrm_msg); if (strncmp(temp[0].mode, temp[1].mode, 3)) /* we do not run diff between different kind @@ -622,7 +622,7 @@ static void diff_flush_patch(struct diff sprintf(msg_, "similarity index %d%%\n" "copy from %s\n" - "copy to %s\n", + "copy to %s", (int)(0.5 + p->score * 100.0/MAX_SCORE), p->one->path, p->two->path); msg = msg_; @@ -631,7 +631,7 @@ static void diff_flush_patch(struct diff sprintf(msg_, "similarity index %d%%\n" "rename old %s\n" - "rename new %s\n", + "rename new %s", (int)(0.5 + p->score * 100.0/MAX_SCORE), p->one->path, p->two->path); msg = msg_; diff --git a/git-external-diff-script b/git-external-diff-script --- a/git-external-diff-script +++ b/git-external-diff-script @@ -59,7 +59,7 @@ then echo "new mode $mode2" if test "$xfrm_msg" != "" then - echo -n $xfrm_msg + echo "$xfrm_msg" fi fi diff ${GIT_DIFF_OPTS-'-pu'} -L "a/$name1" -L "b/$name2" "$tmp1" "$tmp2" ------------------------------------------------ ^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH 07/12] Clean up diff_setup() to make it more extensible. 2005-05-27 22:43 ` [PATCH 00/12] Diff updates Junio C Hamano ` (5 preceding siblings ...) 2005-05-27 22:54 ` [PATCH 06/12] Remove final newline from the value of xfrm_msg variable Junio C Hamano @ 2005-05-27 22:54 ` Junio C Hamano 2005-05-27 22:55 ` [PATCH 08/12] Remove a function not used anymore Junio C Hamano ` (5 subsequent siblings) 12 siblings, 0 replies; 45+ messages in thread From: Junio C Hamano @ 2005-05-27 22:54 UTC (permalink / raw) To: Linus Torvalds; +Cc: Git Mailing List This changes the argument of diff_setup() from an integer that says if we are feeding reversed diff to a bitmask, so that later global options can be added more easily. Signed-off-by: Junio C Hamano <junkio@cox.net> --- diff-cache.c | 6 +++--- diff-files.c | 6 +++--- diff-tree.c | 6 +++--- diff.c | 5 +++-- diff.h | 12 +++++++----- 5 files changed, 19 insertions(+), 16 deletions(-) diff --git a/diff-cache.c b/diff-cache.c --- a/diff-cache.c +++ b/diff-cache.c @@ -5,7 +5,7 @@ static int cached_only = 0; static int diff_output_format = DIFF_FORMAT_HUMAN; static int match_nonexisting = 0; static int detect_rename = 0; -static int reverse_diff = 0; +static int diff_setup_opt = 0; static int diff_score_opt = 0; static const char *pickaxe = NULL; @@ -202,7 +202,7 @@ int main(int argc, const char **argv) continue; } if (!strcmp(arg, "-R")) { - reverse_diff = 1; + diff_setup_opt |= DIFF_SETUP_REVERSE; continue; } if (!strcmp(arg, "-S")) { @@ -224,7 +224,7 @@ int main(int argc, const char **argv) usage(diff_cache_usage); /* The rest is for paths restriction. */ - diff_setup(reverse_diff); + diff_setup(diff_setup_opt); mark_merge_entries(); diff --git a/diff-files.c b/diff-files.c --- a/diff-files.c +++ b/diff-files.c @@ -11,7 +11,7 @@ static const char *diff_files_usage = static int diff_output_format = DIFF_FORMAT_HUMAN; static int detect_rename = 0; -static int reverse_diff = 0; +static int diff_setup_opt = 0; static int diff_score_opt = 0; static const char *pickaxe = NULL; static int silent = 0; @@ -51,7 +51,7 @@ int main(int argc, const char **argv) else if (!strcmp(argv[1], "-z")) diff_output_format = DIFF_FORMAT_MACHINE; else if (!strcmp(argv[1], "-R")) - reverse_diff = 1; + diff_setup_opt |= DIFF_SETUP_REVERSE; else if (!strcmp(argv[1], "-S")) pickaxe = argv[1] + 2; else if (!strncmp(argv[1], "-M", 2)) { @@ -75,7 +75,7 @@ int main(int argc, const char **argv) exit(1); } - diff_setup(reverse_diff); + diff_setup(diff_setup_opt); for (i = 0; i < entries; i++) { struct stat st; diff --git a/diff-tree.c b/diff-tree.c --- a/diff-tree.c +++ b/diff-tree.c @@ -10,7 +10,7 @@ static int show_tree_entry_in_recursive static int read_stdin = 0; static int diff_output_format = DIFF_FORMAT_HUMAN; static int detect_rename = 0; -static int reverse_diff = 0; +static int diff_setup_opt = 0; static int diff_score_opt = 0; static const char *pickaxe = NULL; static const char *header = NULL; @@ -255,7 +255,7 @@ static int diff_tree_sha1(const unsigned static void call_diff_setup(void) { - diff_setup(reverse_diff); + diff_setup(diff_setup_opt); } static int call_diff_flush(void) @@ -497,7 +497,7 @@ int main(int argc, const char **argv) continue; } if (!strcmp(arg, "-R")) { - reverse_diff = 1; + diff_setup_opt |= DIFF_SETUP_REVERSE; continue; } if (!strcmp(arg, "-p")) { diff --git a/diff.c b/diff.c --- a/diff.c +++ b/diff.c @@ -492,9 +492,10 @@ static void run_diff(const char *name, run_external_diff(pgm, name, other, one, two, xfrm_msg); } -void diff_setup(int reverse_diff_) +void diff_setup(int flags) { - reverse_diff = reverse_diff_; + if (flags & DIFF_SETUP_REVERSE) + reverse_diff = 1; } struct diff_queue_struct diff_queued_diff; diff --git a/diff.h b/diff.h --- a/diff.h +++ b/diff.h @@ -28,11 +28,8 @@ extern void diff_unmerge(const char *pat extern int diff_scoreopt_parse(const char *opt); -#define DIFF_FORMAT_HUMAN 0 -#define DIFF_FORMAT_MACHINE 1 -#define DIFF_FORMAT_PATCH 2 -#define DIFF_FORMAT_NO_OUTPUT 3 -extern void diff_setup(int reverse); +#define DIFF_SETUP_REVERSE 1 +extern void diff_setup(int flags); #define DIFF_DETECT_RENAME 1 #define DIFF_DETECT_COPY 2 @@ -44,6 +41,11 @@ extern void diffcore_pathspec(const char extern int diff_queue_is_empty(void); +#define DIFF_FORMAT_HUMAN 0 +#define DIFF_FORMAT_MACHINE 1 +#define DIFF_FORMAT_PATCH 2 +#define DIFF_FORMAT_NO_OUTPUT 3 + extern void diff_flush(int output_style, int resolve_rename_copy); #endif /* DIFF_H */ ------------------------------------------------ ^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH 08/12] Remove a function not used anymore. 2005-05-27 22:43 ` [PATCH 00/12] Diff updates Junio C Hamano ` (6 preceding siblings ...) 2005-05-27 22:54 ` [PATCH 07/12] Clean up diff_setup() to make it more extensible Junio C Hamano @ 2005-05-27 22:55 ` Junio C Hamano 2005-05-27 22:55 ` [PATCH 09/12] Add --pickaxe-all to diff-* brothers Junio C Hamano ` (4 subsequent siblings) 12 siblings, 0 replies; 45+ messages in thread From: Junio C Hamano @ 2005-05-27 22:55 UTC (permalink / raw) To: Linus Torvalds; +Cc: Git Mailing List Earlier rename/copy detection left unmodified filepair in the output and forced downstream to keep them even when they are filtering, and the diff_needs_to_stay() function was used for the logic. It is not used anymore, so remove it. Signed-off-by: Junio C Hamano <junkio@cox.net> --- diff.c | 22 ---------------------- diffcore.h | 3 --- 2 files changed, 25 deletions(-) diff --git a/diff.c b/diff.c --- a/diff.c +++ b/diff.c @@ -647,28 +647,6 @@ static void diff_flush_patch(struct diff run_diff(name, other, p->one, p->two, msg); } -int diff_needs_to_stay(struct diff_queue_struct *q, int i, - struct diff_filespec *it) -{ - /* If it will be used in later entry (either stay or used - * as the source of rename/copy), we need to copy, not rename. - */ - while (i < q->nr) { - struct diff_filepair *p = q->queue[i++]; - if (!DIFF_FILE_VALID(p->two)) - continue; /* removed is fine */ - if (strcmp(p->one->path, it->path)) - continue; /* not relevant */ - - /* p has its src set to *it and it is not a delete; - * it will be used for in-place change, rename/copy, - * or just stays there. We cannot rename it out. - */ - return 1; - } - return 0; -} - int diff_queue_is_empty(void) { struct diff_queue_struct *q = &diff_queued_diff; diff --git a/diffcore.h b/diffcore.h --- a/diffcore.h +++ b/diffcore.h @@ -70,9 +70,6 @@ extern struct diff_filepair *diff_queue( struct diff_filespec *); extern void diff_q(struct diff_queue_struct *, struct diff_filepair *); -extern int diff_needs_to_stay(struct diff_queue_struct *, int, - struct diff_filespec *); - #define DIFF_DEBUG 0 #if DIFF_DEBUG void diff_debug_filespec(struct diff_filespec *, int, const char *); ------------------------------------------------ ^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH 09/12] Add --pickaxe-all to diff-* brothers. 2005-05-27 22:43 ` [PATCH 00/12] Diff updates Junio C Hamano ` (7 preceding siblings ...) 2005-05-27 22:55 ` [PATCH 08/12] Remove a function not used anymore Junio C Hamano @ 2005-05-27 22:55 ` Junio C Hamano 2005-05-27 22:55 ` [PATCH 10/12] Fix the way diffcore-rename records unremoved source Junio C Hamano ` (3 subsequent siblings) 12 siblings, 0 replies; 45+ messages in thread From: Junio C Hamano @ 2005-05-27 22:55 UTC (permalink / raw) To: Linus Torvalds; +Cc: Git Mailing List When --pickaxe-all is given in addition to -S, pickaxe shows the entire diffs contained in the changeset, not just the diffs for the filepair that touched the sought-after string. This is useful to see the changes in context. Signed-off-by: Junio C Hamano <junkio@cox.net> --- diff-cache.c | 7 ++++ diff-files.c | 5 ++- diff-helper.c | 7 +++- diff-tree.c | 7 ++++ diff.h | 4 ++ diffcore-pickaxe.c | 77 +++++++++++++++++++++++++++++++++++++++-------------- 6 files changed, 81 insertions(+), 26 deletions(-) diff --git a/diff-cache.c b/diff-cache.c --- a/diff-cache.c +++ b/diff-cache.c @@ -8,6 +8,7 @@ static int detect_rename = 0; static int diff_setup_opt = 0; static int diff_score_opt = 0; static const char *pickaxe = NULL; +static int pickaxe_opts = 0; /* A file entry went away or appeared */ static void show_file(const char *prefix, struct cache_entry *ce, unsigned char *sha1, unsigned int mode) @@ -209,6 +210,10 @@ int main(int argc, const char **argv) pickaxe = arg + 2; continue; } + if (!strcmp(arg, "--pickaxe-all")) { + pickaxe_opts = DIFF_PICKAXE_ALL; + continue; + } if (!strcmp(arg, "-m")) { match_nonexisting = 1; continue; @@ -238,7 +243,7 @@ int main(int argc, const char **argv) if (detect_rename) diffcore_rename(detect_rename, diff_score_opt); if (pickaxe) - diffcore_pickaxe(pickaxe); + diffcore_pickaxe(pickaxe, pickaxe_opts); if (pathspec) diffcore_pathspec(pathspec); diff_flush(diff_output_format, 1); diff --git a/diff-files.c b/diff-files.c --- a/diff-files.c +++ b/diff-files.c @@ -14,6 +14,7 @@ static int detect_rename = 0; static int diff_setup_opt = 0; static int diff_score_opt = 0; static const char *pickaxe = NULL; +static int pickaxe_opts = 0; static int silent = 0; static void show_unmerge(const char *path) @@ -54,6 +55,8 @@ int main(int argc, const char **argv) diff_setup_opt |= DIFF_SETUP_REVERSE; else if (!strcmp(argv[1], "-S")) pickaxe = argv[1] + 2; + else if (!strcmp(argv[1], "--pickaxe-all")) + pickaxe_opts = DIFF_PICKAXE_ALL; else if (!strncmp(argv[1], "-M", 2)) { diff_score_opt = diff_scoreopt_parse(argv[1]); detect_rename = DIFF_DETECT_RENAME; @@ -116,7 +119,7 @@ int main(int argc, const char **argv) if (detect_rename) diffcore_rename(detect_rename, diff_score_opt); if (pickaxe) - diffcore_pickaxe(pickaxe); + diffcore_pickaxe(pickaxe, pickaxe_opts); if (1 < argc) diffcore_pathspec(argv + 1); diff_flush(diff_output_format, 1); diff --git a/diff-helper.c b/diff-helper.c --- a/diff-helper.c +++ b/diff-helper.c @@ -6,6 +6,7 @@ #include "diff.h" static const char *pickaxe = NULL; +static int pickaxe_opts = 0; static int line_termination = '\n'; static int inter_name_termination = '\t'; @@ -23,6 +24,8 @@ int main(int ac, const char **av) { else if (av[1][1] == 'S') { pickaxe = av[1] + 2; } + else if (!strcmp(av[1], "--pickaxe-all")) + pickaxe_opts = DIFF_PICKAXE_ALL; else usage(diff_helper_usage); ac--; av++; @@ -127,14 +130,14 @@ int main(int ac, const char **av) { continue; } if (pickaxe) - diffcore_pickaxe(pickaxe); + diffcore_pickaxe(pickaxe, pickaxe_opts); if (1 < ac) diffcore_pathspec(av + 1); diff_flush(DIFF_FORMAT_PATCH, 0); printf("%s\n", sb.buf); } if (pickaxe) - diffcore_pickaxe(pickaxe); + diffcore_pickaxe(pickaxe, pickaxe_opts); if (1 < ac) diffcore_pathspec(av + 1); diff_flush(DIFF_FORMAT_PATCH, 0); diff --git a/diff-tree.c b/diff-tree.c --- a/diff-tree.c +++ b/diff-tree.c @@ -13,6 +13,7 @@ static int detect_rename = 0; static int diff_setup_opt = 0; static int diff_score_opt = 0; static const char *pickaxe = NULL; +static int pickaxe_opts = 0; static const char *header = NULL; static const char *header_prefix = ""; @@ -263,7 +264,7 @@ static int call_diff_flush(void) if (detect_rename) diffcore_rename(detect_rename, diff_score_opt); if (pickaxe) - diffcore_pickaxe(pickaxe); + diffcore_pickaxe(pickaxe, pickaxe_opts); if (diff_queue_is_empty()) { diff_flush(DIFF_FORMAT_NO_OUTPUT, 0); return 0; @@ -509,6 +510,10 @@ int main(int argc, const char **argv) pickaxe = arg + 2; continue; } + if (!strcmp(arg, "--pickaxe-all")) { + pickaxe_opts = DIFF_PICKAXE_ALL; + continue; + } if (!strncmp(arg, "-M", 2)) { detect_rename = DIFF_DETECT_RENAME; diff_score_opt = diff_scoreopt_parse(arg); diff --git a/diff.h b/diff.h --- a/diff.h +++ b/diff.h @@ -36,7 +36,9 @@ extern void diff_setup(int flags); extern void diffcore_rename(int rename_copy, int minimum_score); -extern void diffcore_pickaxe(const char *needle); +#define DIFF_PICKAXE_ALL 1 +extern void diffcore_pickaxe(const char *needle, int opts); + extern void diffcore_pathspec(const char **pathspec); extern int diff_queue_is_empty(void); diff --git a/diffcore-pickaxe.c b/diffcore-pickaxe.c --- a/diffcore-pickaxe.c +++ b/diffcore-pickaxe.c @@ -21,36 +21,73 @@ static int contains(struct diff_filespec return 0; } -void diffcore_pickaxe(const char *needle) +void diffcore_pickaxe(const char *needle, int opts) { struct diff_queue_struct *q = &diff_queued_diff; unsigned long len = strlen(needle); - int i; + int i, has_changes; struct diff_queue_struct outq; outq.queue = NULL; outq.nr = outq.alloc = 0; - for (i = 0; i < q->nr; i++) { - struct diff_filepair *p = q->queue[i]; - int onum = outq.nr; - if (!DIFF_FILE_VALID(p->one)) { - if (!DIFF_FILE_VALID(p->two)) - continue; /* ignore nonsense */ - /* created */ - if (contains(p->two, needle, len)) - diff_q(&outq, p); + if (opts & DIFF_PICKAXE_ALL) { + /* Showing the whole changeset if needle exists */ + for (i = has_changes = 0; !has_changes && i < q->nr; i++) { + struct diff_filepair *p = q->queue[i]; + if (!DIFF_FILE_VALID(p->one)) { + if (!DIFF_FILE_VALID(p->two)) + continue; /* ignore unmerged */ + /* created */ + if (contains(p->two, needle, len)) + has_changes++; + } + else if (!DIFF_FILE_VALID(p->two)) { + if (contains(p->one, needle, len)) + has_changes++; + } + else if (!diff_unmodified_pair(p) && + contains(p->one, needle, len) != + contains(p->two, needle, len)) + has_changes++; } - else if (!DIFF_FILE_VALID(p->two)) { - if (contains(p->one, needle, len)) + if (has_changes) + return; /* not munge the queue */ + + /* otherwise we will clear the whole queue + * by copying the empty outq at the end of this + * function, but first clear the current entries + * in the queue. + */ + for (i = 0; i < q->nr; i++) + diff_free_filepair(q->queue[i]); + } + else + /* Showing only the filepairs that has the needle */ + for (i = 0; i < q->nr; i++) { + struct diff_filepair *p = q->queue[i]; + has_changes = 0; + if (!DIFF_FILE_VALID(p->one)) { + if (!DIFF_FILE_VALID(p->two)) + ; /* ignore unmerged */ + /* created */ + else if (contains(p->two, needle, len)) + has_changes = 1; + } + else if (!DIFF_FILE_VALID(p->two)) { + if (contains(p->one, needle, len)) + has_changes = 1; + } + else if (!diff_unmodified_pair(p) && + contains(p->one, needle, len) != + contains(p->two, needle, len)) + has_changes = 1; + + if (has_changes) diff_q(&outq, p); + else + diff_free_filepair(p); } - else if (!diff_unmodified_pair(p) && - contains(p->one, needle, len) != - contains(p->two, needle, len)) - diff_q(&outq, p); - if (onum == outq.nr) - diff_free_filepair(p); - } + free(q->queue); *q = outq; return; ------------------------------------------------ ^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH 10/12] Fix the way diffcore-rename records unremoved source. 2005-05-27 22:43 ` [PATCH 00/12] Diff updates Junio C Hamano ` (8 preceding siblings ...) 2005-05-27 22:55 ` [PATCH 09/12] Add --pickaxe-all to diff-* brothers Junio C Hamano @ 2005-05-27 22:55 ` Junio C Hamano 2005-05-27 22:56 ` [PATCH 11/12] Move pathspec to the beginning of the diffcore chain Junio C Hamano ` (2 subsequent siblings) 12 siblings, 0 replies; 45+ messages in thread From: Junio C Hamano @ 2005-05-27 22:55 UTC (permalink / raw) To: Linus Torvalds; +Cc: Git Mailing List Earier version of diffcore-rename used to keep unmodified filepair in its output so that the last stage of the processing that tells renames from copies can make all of rename/copy to copies. However this had a bad interaction with other diffcore filters that wanted to run after diffcore-rename, in that such unmodified filepair must be retained for proper distinction between renames and copies to happen. This patch fixes the problem by changing the way diffcore-rename records the information needed to distinguish "all are copies" case and "the last one is a rename" case. Signed-off-by: Junio C Hamano <junkio@cox.net> --- diff.c | 76 ++++++++++++++++++-------------------- diffcore-rename.c | 63 ++++++++++++------------------- diffcore.h | 7 ++- t/t4007-rename-3.sh | 103 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 169 insertions(+), 80 deletions(-) new file (100644): t/t4007-rename-3.sh diff --git a/diff.c b/diff.c --- a/diff.c +++ b/diff.c @@ -518,6 +518,7 @@ struct diff_filepair *diff_queue(struct dp->one = one; dp->two = two; dp->score = 0; + dp->source_stays = 0; diff_q(queue, dp); return dp; } @@ -675,8 +676,8 @@ void diff_debug_filepair(const struct di { diff_debug_filespec(p->one, i, "one"); diff_debug_filespec(p->two, i, "two"); - fprintf(stderr, "score %d, status %c\n", - p->score, p->status ? : '?'); + fprintf(stderr, "score %d, status %c source_stays %d\n", + p->score, p->status ? : '?', p->source_stays); } void diff_debug_queue(const char *msg, struct diff_queue_struct *q) @@ -698,8 +699,6 @@ static void diff_resolve_rename_copy(voi struct diff_filepair *p, *pp; struct diff_queue_struct *q = &diff_queued_diff; - /* This should not depend on the ordering of things. */ - diff_debug_queue("resolve-rename-copy", q); for (i = 0; i < q->nr; i++) { @@ -707,23 +706,28 @@ static void diff_resolve_rename_copy(voi p->status = 0; /* undecided */ if (DIFF_PAIR_UNMERGED(p)) p->status = 'U'; - else if (!DIFF_FILE_VALID((p)->one)) + else if (!DIFF_FILE_VALID(p->one)) p->status = 'N'; - else if (!DIFF_FILE_VALID((p)->two)) { - /* Deletion record should be omitted if there - * are rename/copy entries using this one as - * the source. Then we can say one of them - * is a rename and the rest are copies. + else if (!DIFF_FILE_VALID(p->two)) { + /* Deleted entry may have been picked up by + * another rename-copy entry. So we scan the + * queue and if we find one that uses us as the + * source we do not say delete for this entry. */ - p->status = 'D'; for (j = 0; j < q->nr; j++) { pp = q->queue[j]; - if (!strcmp(pp->one->path, p->one->path) && - strcmp(pp->one->path, pp->two->path)) { + if (!strcmp(p->one->path, pp->one->path) && + pp->score) { + /* rename/copy are always valid + * so we do not say DIFF_FILE_VALID() + * on pp->one and pp->two. + */ p->status = 'X'; break; } } + if (!p->status) + p->status = 'D'; } else if (DIFF_PAIR_TYPE_CHANGED(p)) p->status = 'T'; @@ -732,33 +736,24 @@ static void diff_resolve_rename_copy(voi * whose both sides are valid and of the same type, i.e. * either in-place edit or rename/copy edit. */ - else if (strcmp(p->one->path, p->two->path)) { - /* See if there is somebody else anywhere that - * will keep the path (either modified or - * unmodified). If so, we have to be a copy, - * not a rename. In addition, if there is - * some other rename or copy that comes later - * than us that uses the same source, we - * have to be a copy, not a rename. + else if (p->score) { + if (p->source_stays) { + p->status = 'C'; + continue; + } + /* See if there is some other filepair that + * copies from the same source as us. If so + * we are a copy. Otherwise we are a rename. */ - for (j = 0; j < q->nr; j++) { + for (j = i + 1; j < q->nr; j++) { pp = q->queue[j]; if (strcmp(pp->one->path, p->one->path)) - continue; - if (!strcmp(pp->one->path, pp->two->path)) { - if (DIFF_FILE_VALID(pp->two)) { - /* non-delete */ - p->status = 'C'; - break; - } - continue; - } - /* pp is a rename/copy ... */ - if (i < j) { - /* ... and comes later than us */ - p->status = 'C'; - break; - } + continue; /* not us */ + if (!pp->score) + continue; /* not a rename/copy */ + /* pp is a rename/copy from the same source */ + p->status = 'C'; + break; } if (!p->status) p->status = 'R'; @@ -767,8 +762,11 @@ static void diff_resolve_rename_copy(voi p->one->mode != p->two->mode) p->status = 'M'; else - /* this is a "no-change" entry */ - p->status = 'X'; + /* this is a "no-change" entry. + * should not happen anymore. + * p->status = 'X'; + */ + die("internal error in diffcore: unmodified entry remains"); } diff_debug_queue("resolve-rename-copy done", q); } diff --git a/diffcore-rename.c b/diffcore-rename.c --- a/diffcore-rename.c +++ b/diffcore-rename.c @@ -52,14 +52,15 @@ static struct diff_rename_dst *locate_re return &(rename_dst[first]); } +/* Table of rename/copy src files */ static struct diff_rename_src { struct diff_filespec *one; - unsigned src_used : 1; + unsigned src_stays : 1; } *rename_src; static int rename_src_nr, rename_src_alloc; -static struct diff_rename_src *locate_rename_src(struct diff_filespec *one, - int insert_ok) +static struct diff_rename_src *register_rename_src(struct diff_filespec *one, + int src_stays) { int first, last; @@ -77,9 +78,7 @@ static struct diff_rename_src *locate_re } first = next+1; } - /* not found */ - if (!insert_ok) - return NULL; + /* insert to make it at "first" */ if (rename_src_alloc <= rename_src_nr) { rename_src_alloc = alloc_nr(rename_src_alloc); @@ -91,7 +90,7 @@ static struct diff_rename_src *locate_re memmove(rename_src + first + 1, rename_src + first, (rename_src_nr - first - 1) * sizeof(*rename_src)); rename_src[first].one = one; - rename_src[first].src_used = 0; + rename_src[first].src_stays = src_stays; return &(rename_src[first]); } @@ -199,15 +198,14 @@ static void record_rename_pair(struct di fill_filespec(two, dst->sha1, dst->mode); dp = diff_queue(renq, one, two); - dp->score = score; - - rename_src[src_index].src_used = 1; + dp->score = score ? : 1; /* make sure it is at least 1 */ + dp->source_stays = rename_src[src_index].src_stays; rename_dst[dst_index].pair = dp; } /* * We sort the rename similarity matrix with the score, in descending - * order (more similar first). + * order (the most similar first). */ static int score_compare(const void *a_, const void *b_) { @@ -254,9 +252,9 @@ void diffcore_rename(int detect_rename, else locate_rename_dst(p->two, 1); else if (!DIFF_FILE_VALID(p->two)) - locate_rename_src(p->one, 1); - else if (1 < detect_rename) /* find copy, too */ - locate_rename_src(p->one, 1); + register_rename_src(p->one, 0); + else if (detect_rename == DIFF_DETECT_COPY) + register_rename_src(p->one, 1); } if (rename_dst_nr == 0) goto cleanup; /* nothing to do */ @@ -280,7 +278,7 @@ void diffcore_rename(int detect_rename, * doing the delta matrix altogether. */ if (renq.nr == rename_dst_nr) - goto flush_rest; + goto cleanup; num_create = (rename_dst_nr - renq.nr); num_src = rename_src_nr; @@ -307,37 +305,30 @@ void diffcore_rename(int detect_rename, if (dst->pair) continue; /* already done, either exact or fuzzy. */ if (mx[i].score < minimum_score) - break; /* there is not any more diffs applicable. */ + break; /* there is no more usable pair. */ record_rename_pair(&renq, mx[i].dst, mx[i].src, mx[i].score); } free(mx); diff_debug_queue("done detecting fuzzy", &renq); - flush_rest: + cleanup: /* At this point, we have found some renames and copies and they * are kept in renq. The original list is still in *q. - * - * Scan the original list and move them into the outq; we will sort - * outq and swap it into the queue supplied to pass that to - * downstream, so we assign the sort keys in this loop. - * - * See comments at the top of record_rename_pair for numbers used - * to assign rename_rank. */ outq.queue = NULL; outq.nr = outq.alloc = 0; for (i = 0; i < q->nr; i++) { struct diff_filepair *p = q->queue[i]; - struct diff_rename_src *src = locate_rename_src(p->one, 0); struct diff_rename_dst *dst = locate_rename_dst(p->two, 0); struct diff_filepair *pair_to_free = NULL; if (dst) { /* creation */ if (dst->pair) { - /* renq has rename/copy already to produce - * this file, so we do not emit the creation - * record in the output. + /* renq has rename/copy to produce + * this file already, so we do not + * emit the creation record in the + * output. */ diff_q(&outq, dst->pair); pair_to_free = p; @@ -349,17 +340,12 @@ void diffcore_rename(int detect_rename, diff_q(&outq, p); } else if (!diff_unmodified_pair(p)) - /* all the other cases need to be recorded as is */ + /* all the usual ones need to be kept */ diff_q(&outq, p); - else { - /* unmodified pair needs to be recorded only if - * it is used as the source of rename/copy - */ - if (src && src->src_used) - diff_q(&outq, p); - else - pair_to_free = p; - } + else + /* no need to keep unmodified pairs */ + pair_to_free = p; + if (pair_to_free) diff_free_filepair(pair_to_free); } @@ -370,7 +356,6 @@ void diffcore_rename(int detect_rename, *q = outq; diff_debug_queue("done collapsing", q); - cleanup: free(rename_dst); rename_dst = NULL; rename_dst_nr = rename_dst_alloc = 0; diff --git a/diffcore.h b/diffcore.h --- a/diffcore.h +++ b/diffcore.h @@ -39,8 +39,11 @@ extern void diff_free_filespec_data(stru struct diff_filepair { struct diff_filespec *one; struct diff_filespec *two; - int score; /* only valid when one and two are different paths */ - int status; /* M C R N D U (see Documentation/diff-format.txt) */ + unsigned short int score; /* only valid when one and two are + * different paths + */ + char source_stays; /* all of R/C are copies */ + char status; /* M C R N D U (see Documentation/diff-format.txt) */ }; #define DIFF_PAIR_UNMERGED(p) \ (!DIFF_FILE_VALID((p)->one) && !DIFF_FILE_VALID((p)->two)) diff --git a/t/t4007-rename-3.sh b/t/t4007-rename-3.sh new file mode 100644 --- /dev/null +++ b/t/t4007-rename-3.sh @@ -0,0 +1,103 @@ +#!/bin/sh +# +# Copyright (c) 2005 Junio C Hamano +# + +test_description='Rename interaction with pathspec. + +' +. ./test-lib.sh + +_x40='[0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f]' +_x40="$_x40$_x40$_x40$_x40$_x40$_x40$_x40$_x40" +sanitize_diff_raw='s/ \('"$_x40"'\) \1 \([CR]\)[0-9]* / \1 \1 \2# /' +compare_diff_raw () { + # When heuristics are improved, the score numbers would change. + # Ignore them while comparing. + # Also we do not check SHA1 hash generation in this test, which + # is a job for t0000-basic.sh + + sed -e "$sanitize_diff_raw" <"$1" >.tmp-1 + sed -e "$sanitize_diff_raw" <"$2" >.tmp-2 + diff -u .tmp-1 .tmp-2 && rm -f .tmp-1 .tmp-2 +} + +test_expect_success \ + 'prepare reference tree' \ + 'mkdir path0 path1 && + cp ../../COPYING path0/COPYING && + git-update-cache --add path0/COPYING && + tree=$(git-write-tree) && + echo $tree' + +test_expect_success \ + 'prepare work tree' \ + 'cp path0/COPYING path1/COPYING && + git-update-cache --add --remove path0/COPYING path1/COPYING' + +# In the tree, there is only path0/COPYING. In the cache, path0 and +# path1 both have COPYING and the latter is a copy of path0/COPYING. +# Comparing the full tree with cache should tell us so. + +git-diff-cache -C $tree >current + +cat >expected <<\EOF +:100644 100644 6ff87c4664981e4397625791c8ea3bbb5f2279a3 6ff87c4664981e4397625791c8ea3bbb5f2279a3 C100 path0/COPYING path1/COPYING +EOF + +test_expect_success \ + 'validate the result' \ + 'compare_diff_raw current expected' + +# In the tree, there is only path0/COPYING. In the cache, path0 and +# path1 both have COPYING and the latter is a copy of path0/COPYING. +# When we omit output from path0 it should still be able to tell us +# that path1/COPYING is result from a copy from path0/COPYING, not +# rename, which would imply path0/COPYING is now gone. + +git-diff-cache -C $tree path1 >current + +cat >expected <<\EOF +:100644 100644 6ff87c4664981e4397625791c8ea3bbb5f2279a3 6ff87c4664981e4397625791c8ea3bbb5f2279a3 C100 path0/COPYING path1/COPYING +EOF + +test_expect_success \ + 'validate the result' \ + 'compare_diff_raw current expected' + +test_expect_success \ + 'tweak work tree' \ + 'rm -f path0/COPYING && + git-update-cache --remove path0/COPYING' + +# In the tree, there is only path0/COPYING. In the cache, path0 does +# not have COPYING anymore and path1 has COPYING which is a copy of +# path0/COPYING. Showing the full tree with cache should tell us about +# the rename. + +git-diff-cache -C $tree >current + +cat >expected <<\EOF +:100644 100644 6ff87c4664981e4397625791c8ea3bbb5f2279a3 6ff87c4664981e4397625791c8ea3bbb5f2279a3 R100 path0/COPYING path1/COPYING +EOF + +test_expect_success \ + 'validate the result' \ + 'compare_diff_raw current expected' + +# In the tree, there is only path0/COPYING. In the cache, path0 does +# not have COPYING anymore and path1 has COPYING which is a copy of +# path0/COPYING. Even if we restrict the output to path1, it still +# should show us the rename. + +git-diff-cache -C $tree path1 >current + +cat >expected <<\EOF +:100644 100644 6ff87c4664981e4397625791c8ea3bbb5f2279a3 6ff87c4664981e4397625791c8ea3bbb5f2279a3 R100 path0/COPYING path1/COPYING +EOF + +test_expect_success \ + 'validate the result' \ + 'compare_diff_raw current expected' + +test_done ------------------------------------------------ ^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH 11/12] Move pathspec to the beginning of the diffcore chain. 2005-05-27 22:43 ` [PATCH 00/12] Diff updates Junio C Hamano ` (9 preceding siblings ...) 2005-05-27 22:55 ` [PATCH 10/12] Fix the way diffcore-rename records unremoved source Junio C Hamano @ 2005-05-27 22:56 ` Junio C Hamano 2005-05-27 22:56 ` [PATCH 12/12] Optimize diff-tree -[CM] --stdin Junio C Hamano 2005-05-27 23:03 ` [PATCH 00/12] Diff updates Junio C Hamano 12 siblings, 0 replies; 45+ messages in thread From: Junio C Hamano @ 2005-05-27 22:56 UTC (permalink / raw) To: Linus Torvalds; +Cc: Git Mailing List This changes the way how pathspec is used in the three diff-* brothers. Earlier, they tried to grab as much information from the original input and used pathspec to limit the output. This version uses pathspec upfront to narrow the world diffcore operates in, so "git-diff-* <arguments> some-directory" does not look at things outside the specified subtree when finding rename/copy or running pickaxe. Since diff-tree already takes this view and does not feed anything outside the specified directotires to begin with, this patch does not have to touch that command. Signed-off-by: Junio C Hamano <junkio@cox.net> --- diff-cache.c | 4 ++-- diff-files.c | 4 ++-- diff-helper.c | 8 ++++---- t/t4007-rename-3.sh | 22 +++++++++++----------- 4 files changed, 19 insertions(+), 19 deletions(-) diff --git a/diff-cache.c b/diff-cache.c --- a/diff-cache.c +++ b/diff-cache.c @@ -240,12 +240,12 @@ int main(int argc, const char **argv) die("unable to read tree object %s", tree_name); ret = diff_cache(active_cache, active_nr); + if (pathspec) + diffcore_pathspec(pathspec); if (detect_rename) diffcore_rename(detect_rename, diff_score_opt); if (pickaxe) diffcore_pickaxe(pickaxe, pickaxe_opts); - if (pathspec) - diffcore_pathspec(pathspec); diff_flush(diff_output_format, 1); return ret; } diff --git a/diff-files.c b/diff-files.c --- a/diff-files.c +++ b/diff-files.c @@ -116,12 +116,12 @@ int main(int argc, const char **argv) show_modified(oldmode, mode, ce->sha1, null_sha1, ce->name); } + if (1 < argc) + diffcore_pathspec(argv + 1); if (detect_rename) diffcore_rename(detect_rename, diff_score_opt); if (pickaxe) diffcore_pickaxe(pickaxe, pickaxe_opts); - if (1 < argc) - diffcore_pathspec(argv + 1); diff_flush(diff_output_format, 1); return 0; } diff --git a/diff-helper.c b/diff-helper.c --- a/diff-helper.c +++ b/diff-helper.c @@ -129,17 +129,17 @@ int main(int ac, const char **av) { new_path); continue; } - if (pickaxe) - diffcore_pickaxe(pickaxe, pickaxe_opts); if (1 < ac) diffcore_pathspec(av + 1); + if (pickaxe) + diffcore_pickaxe(pickaxe, pickaxe_opts); diff_flush(DIFF_FORMAT_PATCH, 0); printf("%s\n", sb.buf); } - if (pickaxe) - diffcore_pickaxe(pickaxe, pickaxe_opts); if (1 < ac) diffcore_pathspec(av + 1); + if (pickaxe) + diffcore_pickaxe(pickaxe, pickaxe_opts); diff_flush(DIFF_FORMAT_PATCH, 0); return 0; } diff --git a/t/t4007-rename-3.sh b/t/t4007-rename-3.sh --- a/t/t4007-rename-3.sh +++ b/t/t4007-rename-3.sh @@ -46,23 +46,23 @@ cat >expected <<\EOF EOF test_expect_success \ - 'validate the result' \ + 'validate the result (#1)' \ 'compare_diff_raw current expected' # In the tree, there is only path0/COPYING. In the cache, path0 and # path1 both have COPYING and the latter is a copy of path0/COPYING. -# When we omit output from path0 it should still be able to tell us -# that path1/COPYING is result from a copy from path0/COPYING, not -# rename, which would imply path0/COPYING is now gone. +# However when we say we care only about path1, we should just see +# path1/COPYING suddenly appearing from nowhere, not detected as +# a copy from path0/COPYING. git-diff-cache -C $tree path1 >current cat >expected <<\EOF -:100644 100644 6ff87c4664981e4397625791c8ea3bbb5f2279a3 6ff87c4664981e4397625791c8ea3bbb5f2279a3 C100 path0/COPYING path1/COPYING +:000000 100644 0000000000000000000000000000000000000000 6ff87c4664981e4397625791c8ea3bbb5f2279a3 N path1/COPYING EOF test_expect_success \ - 'validate the result' \ + 'validate the result (#2)' \ 'compare_diff_raw current expected' test_expect_success \ @@ -82,22 +82,22 @@ cat >expected <<\EOF EOF test_expect_success \ - 'validate the result' \ + 'validate the result (#3)' \ 'compare_diff_raw current expected' # In the tree, there is only path0/COPYING. In the cache, path0 does # not have COPYING anymore and path1 has COPYING which is a copy of -# path0/COPYING. Even if we restrict the output to path1, it still -# should show us the rename. +# path0/COPYING. When we say we care only about path1, we should just +# see path1/COPYING appearing from nowhere. git-diff-cache -C $tree path1 >current cat >expected <<\EOF -:100644 100644 6ff87c4664981e4397625791c8ea3bbb5f2279a3 6ff87c4664981e4397625791c8ea3bbb5f2279a3 R100 path0/COPYING path1/COPYING +:000000 100644 0000000000000000000000000000000000000000 6ff87c4664981e4397625791c8ea3bbb5f2279a3 N path1/COPYING EOF test_expect_success \ - 'validate the result' \ + 'validate the result (#4)' \ 'compare_diff_raw current expected' test_done ------------------------------------------------ ^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH 12/12] Optimize diff-tree -[CM] --stdin 2005-05-27 22:43 ` [PATCH 00/12] Diff updates Junio C Hamano ` (10 preceding siblings ...) 2005-05-27 22:56 ` [PATCH 11/12] Move pathspec to the beginning of the diffcore chain Junio C Hamano @ 2005-05-27 22:56 ` Junio C Hamano 2005-06-04 2:17 ` Yoichi Yuasa 2005-05-27 23:03 ` [PATCH 00/12] Diff updates Junio C Hamano 12 siblings, 1 reply; 45+ messages in thread From: Junio C Hamano @ 2005-05-27 22:56 UTC (permalink / raw) To: Linus Torvalds; +Cc: Git Mailing List This attempts to optimize "diff-tree -[CM] --stdin", which compares successible tree pairs. This optimization does not make much sense for other commands in the diff-* brothers. When reading from --stdin and using rename/copy detection, the patch makes diff-tree to read the current index file first. This is done to reuse the optimization used by diff-cache in the non-cached case. Similarity estimator can avoid expanding a blob if the index says what is in the work tree has an exact copy of that blob already expanded. Another optimization the patch makes is to check only file sizes first to terminate similarity estimation early. In order for this to work, it needs a way to tell the size of the blob without expanding it. Since an obvious way of doing it, which is to keep all the blobs previously used in the memory, is too costly, it does so by keeping the filesize for each object it has already seen in memory. Signed-off-by: Junio C Hamano <junkio@cox.net> --- diff-tree.c | 3 + diff.c | 83 +++++++++++++++++++++++++++++++++++++++++++++++++++-- diff.h | 2 + diffcore-pickaxe.c | 2 - diffcore-rename.c | 19 ++++++++---- diffcore.h | 2 - 6 files changed, 102 insertions(+), 9 deletions(-) diff --git a/diff-tree.c b/diff-tree.c --- a/diff-tree.c +++ b/diff-tree.c @@ -578,6 +578,9 @@ int main(int argc, const char **argv) if (!read_stdin) return 0; + if (detect_rename) + diff_setup_opt |= (DIFF_SETUP_USE_SIZE_CACHE | + DIFF_SETUP_USE_CACHE); while (fgets(line, sizeof(line), stdin)) diff_tree_stdin(line); diff --git a/diff.c b/diff.c --- a/diff.c +++ b/diff.c @@ -12,6 +12,7 @@ static const char *diff_opts = "-pu"; static unsigned char null_sha1[20] = { 0, }; static int reverse_diff; +static int use_size_cache; static const char *external_diff(void) { @@ -222,12 +223,60 @@ static int work_tree_matches(const char return 1; } +static struct sha1_size_cache { + unsigned char sha1[20]; + unsigned long size; +} **sha1_size_cache; +static int sha1_size_cache_nr, sha1_size_cache_alloc; + +static struct sha1_size_cache *locate_size_cache(unsigned char *sha1, + unsigned long size) +{ + int first, last; + struct sha1_size_cache *e; + + first = 0; + last = sha1_size_cache_nr; + while (last > first) { + int next = (last + first) >> 1; + e = sha1_size_cache[next]; + int cmp = memcmp(e->sha1, sha1, 20); + if (!cmp) + return e; + if (cmp < 0) { + last = next; + continue; + } + first = next+1; + } + /* not found */ + if (size == UINT_MAX) + return NULL; + /* insert to make it at "first" */ + if (sha1_size_cache_alloc <= sha1_size_cache_nr) { + sha1_size_cache_alloc = alloc_nr(sha1_size_cache_alloc); + sha1_size_cache = xrealloc(sha1_size_cache, + sha1_size_cache_alloc * + sizeof(*sha1_size_cache)); + } + sha1_size_cache_nr++; + if (first < sha1_size_cache_nr) + memmove(sha1_size_cache + first + 1, sha1_size_cache + first, + (sha1_size_cache_nr - first - 1) * + sizeof(*sha1_size_cache)); + e = xmalloc(sizeof(struct sha1_size_cache)); + sha1_size_cache[first] = e; + memcpy(e->sha1, sha1, 20); + e->size = size; + return e; +} + /* * While doing rename detection and pickaxe operation, we may need to * grab the data for the blob (or file) for our own in-core comparison. * diff_filespec has data and size fields for this purpose. */ -int diff_populate_filespec(struct diff_filespec *s) +int diff_populate_filespec(struct diff_filespec *s, int size_only) { int err = 0; if (!DIFF_FILE_VALID(s)) @@ -235,6 +284,9 @@ int diff_populate_filespec(struct diff_f if (S_ISDIR(s->mode)) return -1; + if (!use_size_cache) + size_only = 0; + if (s->data) return err; if (!s->sha1_valid || @@ -254,6 +306,8 @@ int diff_populate_filespec(struct diff_f s->size = st.st_size; if (!s->size) goto empty; + if (size_only) + return 0; if (S_ISLNK(st.st_mode)) { int ret; s->data = xmalloc(s->size); @@ -273,9 +327,21 @@ int diff_populate_filespec(struct diff_f close(fd); } else { + /* We cannot do size only for SHA1 blobs */ char type[20]; + struct sha1_size_cache *e; + + if (size_only) { + e = locate_size_cache(s->sha1, UINT_MAX); + if (e) { + s->size = e->size; + return 0; + } + } s->data = read_sha1_file(s->sha1, type, &s->size); s->should_free = 1; + if (s->data && size_only) + locate_size_cache(s->sha1, s->size); } return 0; } @@ -361,7 +427,7 @@ static void prepare_temp_file(const char return; } else { - if (diff_populate_filespec(one)) + if (diff_populate_filespec(one, 0)) die("cannot read data blob for %s", one->path); prep_temp_blob(temp, one->data, one->size, one->sha1, one->mode); @@ -496,6 +562,19 @@ void diff_setup(int flags) { if (flags & DIFF_SETUP_REVERSE) reverse_diff = 1; + if (flags & DIFF_SETUP_USE_CACHE) { + if (!active_cache) + /* read-cache does not die even when it fails + * so it is safe for us to do this here. Also + * it does not smudge active_cache or active_nr + * when it fails, so we do not have to worry about + * cleaning it up oufselves either. + */ + read_cache(); + } + if (flags & DIFF_SETUP_USE_SIZE_CACHE) + use_size_cache = 1; + } struct diff_queue_struct diff_queued_diff; diff --git a/diff.h b/diff.h --- a/diff.h +++ b/diff.h @@ -29,6 +29,8 @@ extern void diff_unmerge(const char *pat extern int diff_scoreopt_parse(const char *opt); #define DIFF_SETUP_REVERSE 1 +#define DIFF_SETUP_USE_CACHE 2 +#define DIFF_SETUP_USE_SIZE_CACHE 4 extern void diff_setup(int flags); #define DIFF_DETECT_RENAME 1 diff --git a/diffcore-pickaxe.c b/diffcore-pickaxe.c --- a/diffcore-pickaxe.c +++ b/diffcore-pickaxe.c @@ -11,7 +11,7 @@ static int contains(struct diff_filespec { unsigned long offset, sz; const char *data; - if (diff_populate_filespec(one)) + if (diff_populate_filespec(one, 0)) return 0; sz = one->size; data = one->data; diff --git a/diffcore-rename.c b/diffcore-rename.c --- a/diffcore-rename.c +++ b/diffcore-rename.c @@ -99,8 +99,11 @@ static int is_exact_match(struct diff_fi if (src->sha1_valid && dst->sha1_valid && !memcmp(src->sha1, dst->sha1, 20)) return 1; - if (diff_populate_filespec(src) || diff_populate_filespec(dst)) - /* this is an error but will be caught downstream */ + if (diff_populate_filespec(src, 1) || diff_populate_filespec(dst, 1)) + return 0; + if (src->size != dst->size) + return 0; + if (diff_populate_filespec(src, 0) || diff_populate_filespec(dst, 0)) return 0; if (src->size == dst->size && !memcmp(src->data, dst->data, src->size)) @@ -125,9 +128,11 @@ static int estimate_similarity(struct di * dst, and then some edit has been applied to dst. * * Compare them and return how similar they are, representing - * the score as an integer between 0 and 10000, except - * where they match exactly it is considered better than anything - * else. + * the score as an integer between 0 and MAX_SCORE. + * + * When there is an exact match, it is considered a better + * match than anything else; the destination does not even + * call into this function in that case. */ void *delta; unsigned long delta_size, base_size; @@ -147,6 +152,7 @@ static int estimate_similarity(struct di /* We would not consider edits that change the file size so * drastically. delta_size must be smaller than * (MAX_SCORE-minimum_score)/MAX_SCORE * min(src->size, dst->size). + * * Note that base_size == 0 case is handled here already * and the final score computation below would not have a * divide-by-zero issue. @@ -154,6 +160,9 @@ static int estimate_similarity(struct di if (base_size * (MAX_SCORE-minimum_score) < delta_size * MAX_SCORE) return 0; + if (diff_populate_filespec(src, 0) || diff_populate_filespec(dst, 0)) + return 0; /* error but caught downstream */ + delta = diff_delta(src->data, src->size, dst->data, dst->size, &delta_size); diff --git a/diffcore.h b/diffcore.h --- a/diffcore.h +++ b/diffcore.h @@ -33,7 +33,7 @@ extern struct diff_filespec *alloc_files extern void fill_filespec(struct diff_filespec *, const unsigned char *, unsigned short); -extern int diff_populate_filespec(struct diff_filespec *); +extern int diff_populate_filespec(struct diff_filespec *, int); extern void diff_free_filespec_data(struct diff_filespec *); struct diff_filepair { ------------------------------------------------ ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 12/12] Optimize diff-tree -[CM] --stdin 2005-05-27 22:56 ` [PATCH 12/12] Optimize diff-tree -[CM] --stdin Junio C Hamano @ 2005-06-04 2:17 ` Yoichi Yuasa 0 siblings, 0 replies; 45+ messages in thread From: Yoichi Yuasa @ 2005-06-04 2:17 UTC (permalink / raw) To: Junio C Hamano; +Cc: yuasa, torvalds, git Hi, On Fri, 27 May 2005 15:56:38 -0700 Junio C Hamano <junkio@cox.net> wrote: > <snip> > diff --git a/diff.c b/diff.c > --- a/diff.c > +++ b/diff.c > @@ -12,6 +12,7 @@ static const char *diff_opts = "-pu"; > static unsigned char null_sha1[20] = { 0, }; > > static int reverse_diff; > +static int use_size_cache; > > static const char *external_diff(void) > { > @@ -222,12 +223,60 @@ static int work_tree_matches(const char > return 1; > } > > +static struct sha1_size_cache { > + unsigned char sha1[20]; > + unsigned long size; > +} **sha1_size_cache; > +static int sha1_size_cache_nr, sha1_size_cache_alloc; > + > +static struct sha1_size_cache *locate_size_cache(unsigned char *sha1, > + unsigned long size) > +{ > + int first, last; > + struct sha1_size_cache *e; > + > + first = 0; > + last = sha1_size_cache_nr; > + while (last > first) { > + int next = (last + first) >> 1; > + e = sha1_size_cache[next]; > + int cmp = memcmp(e->sha1, sha1, 20); I got build error in diff.c Please apply the following patch. Thanks, Yoichi Signed-off-by: Yoichi Yuasa <yuasa@hh.iij4u.or.jp> diff --git a/diff.c b/diff.c --- a/diff.c +++ b/diff.c @@ -244,9 +244,9 @@ static struct sha1_size_cache *locate_si first = 0; last = sha1_size_cache_nr; while (last > first) { - int next = (last + first) >> 1; + int cmp, next = (last + first) >> 1; e = sha1_size_cache[next]; - int cmp = memcmp(e->sha1, sha1, 20); + cmp = memcmp(e->sha1, sha1, 20); if (!cmp) return e; if (cmp < 0) { ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 00/12] Diff updates 2005-05-27 22:43 ` [PATCH 00/12] Diff updates Junio C Hamano ` (11 preceding siblings ...) 2005-05-27 22:56 ` [PATCH 12/12] Optimize diff-tree -[CM] --stdin Junio C Hamano @ 2005-05-27 23:03 ` Junio C Hamano 2005-05-28 10:11 ` [PATCH] Do not show empty diff in diff-cache uncached Junio C Hamano 12 siblings, 1 reply; 45+ messages in thread From: Junio C Hamano @ 2005-05-27 23:03 UTC (permalink / raw) To: Linus Torvalds; +Cc: Git Mailing List I've said "I am done with diff" twice on this list already. Aside from unevitable bug reports ;-) I think this time I am done. At least the major ones I wanted to do. Except one thing. What do you think about the current behaviour of "diff-cache -p (uncached") in a work tree which was freshly checked-out, unmodified but you "touch"ed some files to make them stat-dirty? I think ancient diff-cache did not report those files, but with the new "diff --git" headers it will show the "diff --git" header mentioning those files followed by no content nor mode changes. Admittedly this matches the diff-raw output behaviour more closely, but I find it a bit distracting. Do you care about cleaning this up? ^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH] Do not show empty diff in diff-cache uncached. 2005-05-27 23:03 ` [PATCH 00/12] Diff updates Junio C Hamano @ 2005-05-28 10:11 ` Junio C Hamano 2005-05-28 19:22 ` [PATCH] Diff: two fixes Junio C Hamano 2005-05-29 18:53 ` [PATCH] Do not show empty diff in diff-cache uncached Linus Torvalds 0 siblings, 2 replies; 45+ messages in thread From: Junio C Hamano @ 2005-05-28 10:11 UTC (permalink / raw) To: Linus Torvalds; +Cc: Git Mailing List Pre- "diff --git" built-in diff did not add any extended header on its own, so it did not show anything for unmodified but stat-dirty file from diff-cache command without --cached flag. Recent diff-cache produces "diff --git" header internally before calling the "diff" command, which results in an empty diff for such a file, cluttering the output. This patch fixes this. Signed-off-by: Junio C Hamano <junkio@cox.net> --- diff.c | 26 ++++++++++++++++++++++++++ diffcore-rename.c | 17 ----------------- diffcore.h | 2 ++ 3 files changed, 28 insertions(+), 17 deletions(-) diff --git a/diff.c b/diff.c --- a/diff.c +++ b/diff.c @@ -684,6 +684,23 @@ int diff_unmodified_pair(struct diff_fil return 0; } +int is_exact_match(struct diff_filespec *src, struct diff_filespec *dst) +{ + if (src->sha1_valid && dst->sha1_valid && + !memcmp(src->sha1, dst->sha1, 20)) + return 1; + if (diff_populate_filespec(src, 1) || diff_populate_filespec(dst, 1)) + return 0; + if (src->size != dst->size) + return 0; + if (diff_populate_filespec(src, 0) || diff_populate_filespec(dst, 0)) + return 0; + if (src->size == dst->size && + !memcmp(src->data, dst->data, src->size)) + return 1; + return 0; +} + static void diff_flush_patch(struct diff_filepair *p) { const char *name, *other; @@ -692,6 +709,15 @@ static void diff_flush_patch(struct diff if (diff_unmodified_pair(p)) return; + /* When diff-raw would have said "look at the filesystem", we + * need to see if the two are the same and if so not to emit + * anything at all. Avoid is_exact_match() comparison when it + * does not matter. + */ + if ((DIFF_FILE_VALID(p->two) && !p->two->sha1_valid) && + is_exact_match(p->one, p->two)) + return; + name = p->one->path; other = (strcmp(name, p->two->path) ? p->two->path : NULL); if ((DIFF_FILE_VALID(p->one) && S_ISDIR(p->one->mode)) || diff --git a/diffcore-rename.c b/diffcore-rename.c --- a/diffcore-rename.c +++ b/diffcore-rename.c @@ -94,23 +94,6 @@ static struct diff_rename_src *register_ return &(rename_src[first]); } -static int is_exact_match(struct diff_filespec *src, struct diff_filespec *dst) -{ - if (src->sha1_valid && dst->sha1_valid && - !memcmp(src->sha1, dst->sha1, 20)) - return 1; - if (diff_populate_filespec(src, 1) || diff_populate_filespec(dst, 1)) - return 0; - if (src->size != dst->size) - return 0; - if (diff_populate_filespec(src, 0) || diff_populate_filespec(dst, 0)) - return 0; - if (src->size == dst->size && - !memcmp(src->data, dst->data, src->size)) - return 1; - return 0; -} - struct diff_score { int src; /* index in rename_src */ int dst; /* index in rename_dst */ diff --git a/diffcore.h b/diffcore.h --- a/diffcore.h +++ b/diffcore.h @@ -29,6 +29,8 @@ struct diff_filespec { unsigned should_munmap : 1; /* data should be munmap()'ed */ }; +extern int is_exact_match(struct diff_filespec *, struct diff_filespec *); + extern struct diff_filespec *alloc_filespec(const char *); extern void fill_filespec(struct diff_filespec *, const unsigned char *, unsigned short); ------------------------------------------------ ^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH] Diff: two fixes. 2005-05-28 10:11 ` [PATCH] Do not show empty diff in diff-cache uncached Junio C Hamano @ 2005-05-28 19:22 ` Junio C Hamano 2005-05-29 4:20 ` [PATCH] diff-helper: fix R/C score parsing under -z flag Junio C Hamano 2005-05-29 18:53 ` [PATCH] Do not show empty diff in diff-cache uncached Linus Torvalds 1 sibling, 1 reply; 45+ messages in thread From: Junio C Hamano @ 2005-05-28 19:22 UTC (permalink / raw) To: Linus Torvalds; +Cc: Git Mailing List The count-delta routine sometimes overcounted the copied source material which resulted in unsigned int wraparound. The latest diff-cache (uncached) fix to eliminate empty diffs from the output revealed that is_exact_match() was not careful enough, which resulted in a sanity check routine triggering when a file is added to an index. Signed-off-by: Junio C Hamano <junkio@cox.net> --- count-delta.c | 3 +++ diff.c | 3 +++ 2 files changed, 6 insertions(+) diff --git a/count-delta.c b/count-delta.c --- a/count-delta.c +++ b/count-delta.c @@ -88,5 +88,8 @@ unsigned long count_delta(void *delta_bu /* delete size is what was _not_ copied from source. * edit size is that and literal additions. */ + if (src_size + added_literal < copied_from_source) + /* we ended up overcounting and underflowed */ + return 0; return (src_size - copied_from_source) + added_literal; } diff --git a/diff.c b/diff.c --- a/diff.c +++ b/diff.c @@ -689,6 +689,9 @@ int is_exact_match(struct diff_filespec if (src->sha1_valid && dst->sha1_valid && !memcmp(src->sha1, dst->sha1, 20)) return 1; + /* if either is invalid they cannot match */ + if (!DIFF_FILE_VALID(src) || !DIFF_FILE_VALID(dst)) + return 0; if (diff_populate_filespec(src, 1) || diff_populate_filespec(dst, 1)) return 0; if (src->size != dst->size) ------------------------------------------------ ^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH] diff-helper: fix R/C score parsing under -z flag. 2005-05-28 19:22 ` [PATCH] Diff: two fixes Junio C Hamano @ 2005-05-29 4:20 ` Junio C Hamano 2005-05-29 5:23 ` [PATCH] diff-cache: diff-patch (-p) format fixes Junio C Hamano 0 siblings, 1 reply; 45+ messages in thread From: Junio C Hamano @ 2005-05-29 4:20 UTC (permalink / raw) To: Linus Torvalds; +Cc: Git Mailing List The score number that follows R/C status letter was parsed but the parse pointer was not updated, causing the entire line to become unrecognized. This patch fixes this problem. Signed-off-by: Junio C Hamano <junkio@cox.net> --- diff-helper.c | 17 ++++++++--------- 1 files changed, 8 insertions(+), 9 deletions(-) diff --git a/diff-helper.c b/diff-helper.c --- a/diff-helper.c +++ b/diff-helper.c @@ -80,17 +80,16 @@ int main(int ac, const char **av) { if (!strchr("MCRNDU", status)) break; two_paths = score = 0; - if (status == 'R' || status == 'C') { + if (status == 'R' || status == 'C') two_paths = 1; - sscanf(cp, "%d", &score); - if (line_termination) { - cp = strchr(cp, - inter_name_termination); - if (!cp) - break; - } - } + /* pick up score if exists */ + if (sscanf(cp, "%d", &score) != 1) + score = 0; + cp = strchr(cp, + inter_name_termination); + if (!cp) + break; if (*cp++ != inter_name_termination) break; ------------------------------------------------ ^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH] diff-cache: diff-patch (-p) format fixes. 2005-05-29 4:20 ` [PATCH] diff-helper: fix R/C score parsing under -z flag Junio C Hamano @ 2005-05-29 5:23 ` Junio C Hamano 2005-05-29 9:10 ` [PATCH] diff: code clean-up Junio C Hamano 0 siblings, 1 reply; 45+ messages in thread From: Junio C Hamano @ 2005-05-29 5:23 UTC (permalink / raw) To: Linus Torvalds; +Cc: Git Mailing List This fixes two more bugs. - When diff-cache is run with -R and without --cached, a stat-dirty but otherwise unmodified file still produced an empty "diff --git" header (earlier fix was only for the case without -R). - When diff-cache is run without --cached, an unmodifed file that has different mode bits from the tree incorrectly picked up the mode bits from the filesystem, when it should have used what was recorded in the tree object. Signed-off-by: Junio C Hamano <junkio@cox.net> --- diff.c | 26 ++++++++++++++++++++++---- 1 files changed, 22 insertions(+), 4 deletions(-) diff --git a/diff.c b/diff.c --- a/diff.c +++ b/diff.c @@ -183,6 +183,9 @@ void fill_filespec(struct diff_filespec * Given a name and sha1 pair, if the dircache tells us the file in * the work tree has that object contents, return true, so that * prepare_temp_file() does not have to inflate and extract. + * + * NOTE: this function does not use the mode bits, so diff_filespec + * users must be careful about mode handling! */ static int work_tree_matches(const char *name, const unsigned char *sha1) { @@ -394,6 +397,10 @@ static void prepare_temp_file(const char if (!one->sha1_valid || work_tree_matches(name, one->sha1)) { + /* NOTE: we only say the matching file has the same + * contents. It may have a different mode, in which + * case we should not pick it up from the filesystem! + */ struct stat st; if (lstat(name, &st) < 0) { if (errno == ENOENT) @@ -421,8 +428,15 @@ static void prepare_temp_file(const char strcpy(temp->hex, sha1_to_hex(null_sha1)); else strcpy(temp->hex, sha1_to_hex(one->sha1)); - sprintf(temp->mode, "%06o", - S_IFREG |ce_permissions(st.st_mode)); + /* even though we borrow the contents from the + * work tree, we want our mode if we are not told + * to look at the filesystem. + */ + if (one->sha1_valid) + sprintf(temp->mode, "%06o", one->mode); + else + sprintf(temp->mode, "%06o", + S_IFREG | ce_permissions(st.st_mode)); } return; } @@ -716,9 +730,13 @@ static void diff_flush_patch(struct diff * need to see if the two are the same and if so not to emit * anything at all. Avoid is_exact_match() comparison when it * does not matter. + * Notice that "look at the filesystem" can happen on p->one + * if we are operating under reverse-diff option. */ - if ((DIFF_FILE_VALID(p->two) && !p->two->sha1_valid) && - is_exact_match(p->one, p->two)) + if ( ((DIFF_FILE_VALID(p->one) && !p->one->sha1_valid) || + (DIFF_FILE_VALID(p->two) && !p->two->sha1_valid)) && + is_exact_match(p->one, p->two) && + (p->one->mode == p->two->mode) ) return; name = p->one->path; ------------------------------------------------ ^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH] diff: code clean-up. 2005-05-29 5:23 ` [PATCH] diff-cache: diff-patch (-p) format fixes Junio C Hamano @ 2005-05-29 9:10 ` Junio C Hamano 0 siblings, 0 replies; 45+ messages in thread From: Junio C Hamano @ 2005-05-29 9:10 UTC (permalink / raw) To: Linus Torvalds; +Cc: Git Mailing List - The previous fix for the "diff-cache -p mode bits problem" had an unnecessary conditional statement not to use the mode that came from the calling program when we get 0{40} SHA1. This was totally unnecessary, since the caller gives the valid mode bits even when calling us with 0{40} SHA1. This mistake did not break anything, since the mode bits we got ourselves were the same one taken from the filesystem, but it was doing unnecessary work. This has been fixed. - DIFF_PAIR_RENAME() macro is introduced to distinguish a filepair that is a rename/copy (the definition of which is src and dst are different paths, of course). This removes the hack used in the record_rename_pair() to always put some non-zero value in the score field. Signed-off-by: Junio C Hamano <junkio@cox.net> --- diff.c | 17 +++++++---------- diffcore-rename.c | 2 +- diffcore.h | 6 +++--- 3 files changed, 11 insertions(+), 14 deletions(-) diff --git a/diff.c b/diff.c --- a/diff.c +++ b/diff.c @@ -429,14 +429,11 @@ static void prepare_temp_file(const char else strcpy(temp->hex, sha1_to_hex(one->sha1)); /* even though we borrow the contents from the - * work tree, we want our mode if we are not told - * to look at the filesystem. + * work tree, we always want our mode. mode is + * trustworthy even when !(one->sha1_valid), as + * long as DIFF_FILE_VALID(one). */ - if (one->sha1_valid) - sprintf(temp->mode, "%06o", one->mode); - else - sprintf(temp->mode, "%06o", - S_IFREG | ce_permissions(st.st_mode)); + sprintf(temp->mode, "%06o", one->mode); } return; } @@ -843,7 +840,7 @@ static void diff_resolve_rename_copy(voi for (j = 0; j < q->nr; j++) { pp = q->queue[j]; if (!strcmp(p->one->path, pp->one->path) && - pp->score) { + DIFF_PAIR_RENAME(pp)) { /* rename/copy are always valid * so we do not say DIFF_FILE_VALID() * on pp->one and pp->two. @@ -862,7 +859,7 @@ static void diff_resolve_rename_copy(voi * whose both sides are valid and of the same type, i.e. * either in-place edit or rename/copy edit. */ - else if (p->score) { + else if (DIFF_PAIR_RENAME(p)) { if (p->source_stays) { p->status = 'C'; continue; @@ -875,7 +872,7 @@ static void diff_resolve_rename_copy(voi pp = q->queue[j]; if (strcmp(pp->one->path, p->one->path)) continue; /* not us */ - if (!pp->score) + if (!DIFF_PAIR_RENAME(pp)) continue; /* not a rename/copy */ /* pp is a rename/copy from the same source */ p->status = 'C'; diff --git a/diffcore-rename.c b/diffcore-rename.c --- a/diffcore-rename.c +++ b/diffcore-rename.c @@ -190,7 +190,7 @@ static void record_rename_pair(struct di fill_filespec(two, dst->sha1, dst->mode); dp = diff_queue(renq, one, two); - dp->score = score ? : 1; /* make sure it is at least 1 */ + dp->score = score; dp->source_stays = rename_src[src_index].src_stays; rename_dst[dst_index].pair = dp; } diff --git a/diffcore.h b/diffcore.h --- a/diffcore.h +++ b/diffcore.h @@ -42,15 +42,15 @@ extern void diff_free_filespec_data(stru struct diff_filepair { struct diff_filespec *one; struct diff_filespec *two; - unsigned short int score; /* only valid when one and two are - * different paths - */ + unsigned short int score; char source_stays; /* all of R/C are copies */ char status; /* M C R N D U (see Documentation/diff-format.txt) */ }; #define DIFF_PAIR_UNMERGED(p) \ (!DIFF_FILE_VALID((p)->one) && !DIFF_FILE_VALID((p)->two)) +#define DIFF_PAIR_RENAME(p) (strcmp((p)->one->path, (p)->two->path)) + #define DIFF_PAIR_TYPE_CHANGED(p) \ ((S_IFMT & (p)->one->mode) != (S_IFMT & (p)->two->mode)) ------------------------------------------------ ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH] Do not show empty diff in diff-cache uncached. 2005-05-28 10:11 ` [PATCH] Do not show empty diff in diff-cache uncached Junio C Hamano 2005-05-28 19:22 ` [PATCH] Diff: two fixes Junio C Hamano @ 2005-05-29 18:53 ` Linus Torvalds 2005-05-29 20:09 ` Junio C Hamano 2005-06-11 23:27 ` [PATCH] apply.c: tolerate diff from a dirty but unchanged path Junio C Hamano 1 sibling, 2 replies; 45+ messages in thread From: Linus Torvalds @ 2005-05-29 18:53 UTC (permalink / raw) To: Junio C Hamano; +Cc: Git Mailing List On Sat, 28 May 2005, Junio C Hamano wrote: > > Pre- "diff --git" built-in diff did not add any extended header > on its own, so it did not show anything for unmodified but > stat-dirty file from diff-cache command without --cached flag. > > Recent diff-cache produces "diff --git" header internally before > calling the "diff" command, which results in an empty diff for > such a file, cluttering the output. This patch fixes this. I'm not sure I like this. I actually _expect_ that "git-diff-files" will show files that don't match the index, even if they happen to have the exact content that the index points to. It's how I know whether the index is up-to-date or not. The exact same thing is trye of git-diff-cache. If something isn't up-to-date in the cache, you should show it, since certain operations depend on the cache being updated. Linus ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH] Do not show empty diff in diff-cache uncached. 2005-05-29 18:53 ` [PATCH] Do not show empty diff in diff-cache uncached Linus Torvalds @ 2005-05-29 20:09 ` Junio C Hamano 2005-05-29 21:52 ` Junio C Hamano 2005-06-11 23:27 ` [PATCH] apply.c: tolerate diff from a dirty but unchanged path Junio C Hamano 1 sibling, 1 reply; 45+ messages in thread From: Junio C Hamano @ 2005-05-29 20:09 UTC (permalink / raw) To: Linus Torvalds; +Cc: Git Mailing List >>>>> "LT" == Linus Torvalds <torvalds@osdl.org> writes: LT> I'm not sure I like this. LT> I actually _expect_ that "git-diff-files" will show files that don't match LT> the index, even if they happen to have the exact content that the index LT> points to. It's how I know whether the index is up-to-date or not. LT> The exact same thing is trye of git-diff-cache. If something isn't LT> up-to-date in the cache, you should show it, since certain operations LT> depend on the cache being updated. Let me make sure that we are on the same page. I am only talking about '-p' (diff-patch) case in this patch. The code should continue to show the 0{40} SHA1 on the right hand side in diff-raw output. Do you still want to see the header in that case to match the diff-raw exactly? ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH] Do not show empty diff in diff-cache uncached. 2005-05-29 20:09 ` Junio C Hamano @ 2005-05-29 21:52 ` Junio C Hamano 2005-05-29 23:41 ` [PATCH 0/3] Leftover bits after 12-series Junio C Hamano 2005-05-30 5:34 ` [PATCH] Do not show empty diff in diff-cache uncached Linus Torvalds 0 siblings, 2 replies; 45+ messages in thread From: Junio C Hamano @ 2005-05-29 21:52 UTC (permalink / raw) To: Linus Torvalds; +Cc: Git Mailing List >>>>> "JCH" == Junio C Hamano <junkio@cox.net> writes: JCH> Let me make sure that we are on the same page. Linus, now I've examined what are and what aren't merged in your tree, I think I know the answer to that question. The set you picked look sensible to me. I'm taking a look at the resulting tree to see if there are fixes and cleanups that I still think should be merged. I'll feed them afresh to you later, if there are any, after rebasing the patch to the tip of your tree. Please disregard the patches you have already discarded so far; this request-to-discard includes -O and -B enhancements. ^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH 0/3] Leftover bits after 12-series 2005-05-29 21:52 ` Junio C Hamano @ 2005-05-29 23:41 ` Junio C Hamano 2005-05-29 23:54 ` [PATCH 1/3] diff-helper: Fix R/C score parsing under -z flag Junio C Hamano ` (3 more replies) 2005-05-30 5:34 ` [PATCH] Do not show empty diff in diff-cache uncached Linus Torvalds 1 sibling, 4 replies; 45+ messages in thread From: Junio C Hamano @ 2005-05-29 23:41 UTC (permalink / raw) To: Linus Torvalds; +Cc: Git Mailing List >>>>> "JCH" == Junio C Hamano <junkio@cox.net> writes: JCH> I'm taking a look at the resulting tree to see if there are JCH> fixes and cleanups that I still think should be merged. I'll JCH> feed them afresh to you later, if there are any, after rebasing JCH> the patch to the tip of your tree. I'll be submitting a set of cleanup and bugfix patches. The first one is a real bugfix. The latter two are cleanups. [PATCH 1/3] diff-helper: Fix R/C score parsing under -z flag. [PATCH 2/3] diff: consolidate various calls into diffcore. [PATCH 3/3] diff: code clean-up. ^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH 1/3] diff-helper: Fix R/C score parsing under -z flag. 2005-05-29 23:41 ` [PATCH 0/3] Leftover bits after 12-series Junio C Hamano @ 2005-05-29 23:54 ` Junio C Hamano 2005-05-29 23:56 ` [PATCH 2/3] diff: consolidate various calls into diffcore Junio C Hamano ` (2 subsequent siblings) 3 siblings, 0 replies; 45+ messages in thread From: Junio C Hamano @ 2005-05-29 23:54 UTC (permalink / raw) To: Linus Torvalds; +Cc: Git Mailing List The score number that follow R/C status were parsed but the parse pointer was not updated, causing the entire line to become unrecognized. This patch fixes this problem. There was a test missing to catch this breakage, which this commit adds as t4009-diff-rename-4.sh. The diff-raw tests used in related t4005-diff-rename-2.sh (the same test without -z) and t4007-rename-3.sh were stricter than necessarily, despite that the comment for the tests said otherwise. This patch also corrects them. The documentation is updated to say that the status can optionally be followed by a number called "score"; it does not have to stay similarity index forever and there is no reason to limit it only to C and R. Signed-off-by: Junio C Hamano <junkio@cox.net> --- Documentation/diff-format.txt | 2 diff-helper.c | 17 +-- t/t4005-diff-rename-2.sh | 20 ++-- t/t4007-rename-3.sh | 2 t/t4009-diff-rename-4.sh | 196 ++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 219 insertions(+), 18 deletions(-) new file (100755): t/t4009-diff-rename-4.sh diff --git a/Documentation/diff-format.txt b/Documentation/diff-format.txt --- a/Documentation/diff-format.txt +++ b/Documentation/diff-format.txt @@ -36,7 +36,7 @@ That is, from the left to the right: (6) sha1 for "src"; 0{40} if creation or unmerged. (7) a space. (8) sha1 for "dst"; 0{40} if creation, unmerged or "look at work tree". - (9) status, followed by similarlity index number only for C and R. + (9) status, followed by optional "score" number. (10) a tab or a NUL when '-z' option is used. (11) path for "src" (12) a tab or a NUL when '-z' option is used; only exists for C or R. diff --git a/diff-helper.c b/diff-helper.c --- a/diff-helper.c +++ b/diff-helper.c @@ -80,17 +80,16 @@ int main(int ac, const char **av) { if (!strchr("MCRNDU", status)) break; two_paths = score = 0; - if (status == 'R' || status == 'C') { + if (status == 'R' || status == 'C') two_paths = 1; - sscanf(cp, "%d", &score); - if (line_termination) { - cp = strchr(cp, - inter_name_termination); - if (!cp) - break; - } - } + /* pick up score if exists */ + if (sscanf(cp, "%d", &score) != 1) + score = 0; + cp = strchr(cp, + inter_name_termination); + if (!cp) + break; if (*cp++ != inter_name_termination) break; diff --git a/t/t4005-diff-rename-2.sh b/t/t4005-diff-rename-2.sh --- a/t/t4005-diff-rename-2.sh +++ b/t/t4005-diff-rename-2.sh @@ -8,11 +8,17 @@ test_description='Same rename detection ' . ./test-lib.sh +_x40='[0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f]' +_x40="$_x40$_x40$_x40$_x40$_x40$_x40$_x40$_x40" +sanitize_diff_raw='s/ '"$_x40"' '"$_x40"' \([A-Z]\)[0-9]* / X X \1# /' compare_diff_raw () { # When heuristics are improved, the score numbers would change. # Ignore them while comparing. - sed -e 's/ \([CR]\)[0-9]* /\1#/' <"$1" >.tmp-1 - sed -e 's/ \([CR]\)[0-9]* /\1#/' <"$2" >.tmp-2 + # Also we do not check SHA1 hash generation in this test, which + # is a job for t0000-basic.sh + + sed -e "$sanitize_diff_raw" <"$1" >.tmp-1 + sed -e "$sanitize_diff_raw" <"$2" >.tmp-2 diff -u .tmp-1 .tmp-2 && rm -f .tmp-1 .tmp-2 } @@ -109,11 +115,6 @@ test_expect_success \ 'validate output from rename/copy detection (#2)' \ 'compare_diff_raw current expected' -test_expect_success \ - 'prepare work tree once again' \ - 'cat ../../COPYING >COPYING && - git-update-cache --add --remove COPYING COPYING.1' - # make sure diff-helper can grok it. mv expected diff-raw GIT_DIFF_OPTS=--unified=0 git-diff-helper <diff-raw >current @@ -151,6 +152,11 @@ test_expect_success \ # anything about rezrov nor COPYING, since the revised again diff-raw # nows how to say Copy. +test_expect_success \ + 'prepare work tree once again' \ + 'cat ../../COPYING >COPYING && + git-update-cache --add --remove COPYING COPYING.1' + git-diff-cache -C $tree >current cat >expected <<\EOF :100644 100644 6ff87c4664981e4397625791c8ea3bbb5f2279a3 0603b3238a076dc6c8022aedc6648fa523a17178 C1234 COPYING COPYING.1 diff --git a/t/t4007-rename-3.sh b/t/t4007-rename-3.sh --- a/t/t4007-rename-3.sh +++ b/t/t4007-rename-3.sh @@ -10,7 +10,7 @@ test_description='Rename interaction wit _x40='[0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f]' _x40="$_x40$_x40$_x40$_x40$_x40$_x40$_x40$_x40" -sanitize_diff_raw='s/ \('"$_x40"'\) \1 \([CR]\)[0-9]* / \1 \1 \2# /' +sanitize_diff_raw='s/ '"$_x40"' '"$_x40"' \([A-Z]\)[0-9]* / X X \1# /' compare_diff_raw () { # When heuristics are improved, the score numbers would change. # Ignore them while comparing. diff --git a/t/t4009-diff-rename-4.sh b/t/t4009-diff-rename-4.sh new file mode 100755 --- /dev/null +++ b/t/t4009-diff-rename-4.sh @@ -0,0 +1,196 @@ +#!/bin/sh +# +# Copyright (c) 2005 Junio C Hamano +# + +test_description='Same rename detection as t4003 but testing diff-raw -z. + +' +. ./test-lib.sh + +_x40='[0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f]' +_x40="$_x40$_x40$_x40$_x40$_x40$_x40$_x40$_x40" +sanitize_diff_raw='/^:/s/ '"$_x40"' '"$_x40"' \([A-Z]\)[0-9]*$/ X X \1#/' +compare_diff_raw () { + # When heuristics are improved, the score numbers would change. + # Ignore them while comparing. + # Also we do not check SHA1 hash generation in this test, which + # is a job for t0000-basic.sh + + tr '\0' '\012' <"$1" | sed -e "$sanitize_diff_raw" >.tmp-1 + tr '\0' '\012' <"$2" | sed -e "$sanitize_diff_raw" >.tmp-2 + diff -u .tmp-1 .tmp-2 && rm -f .tmp-1 .tmp-2 +} + +compare_diff_patch () { + # When heuristics are improved, the score numbers would change. + # Ignore them while comparing. + sed -e '/^similarity index [0-9]*%$/d' <"$1" >.tmp-1 + sed -e '/^similarity index [0-9]*%$/d' <"$2" >.tmp-2 + diff -u .tmp-1 .tmp-2 && rm -f .tmp-1 .tmp-2 +} + +test_expect_success \ + 'prepare reference tree' \ + 'cat ../../COPYING >COPYING && + echo frotz >rezrov && + git-update-cache --add COPYING rezrov && + tree=$(git-write-tree) && + echo $tree' + +test_expect_success \ + 'prepare work tree' \ + 'sed -e 's/HOWEVER/However/' <COPYING >COPYING.1 && + sed -e 's/GPL/G.P.L/g' <COPYING >COPYING.2 && + rm -f COPYING && + git-update-cache --add --remove COPYING COPYING.?' + +# tree has COPYING and rezrov. work tree has COPYING.1 and COPYING.2, +# both are slightly edited, and unchanged rezrov. We say COPYING.1 +# and COPYING.2 are based on COPYING, and do not say anything about +# rezrov. + +git-diff-cache -z -M $tree >current + +cat >expected <<\EOF +:100644 100644 6ff87c4664981e4397625791c8ea3bbb5f2279a3 0603b3238a076dc6c8022aedc6648fa523a17178 C1234 +COPYING +COPYING.1 +:100644 100644 6ff87c4664981e4397625791c8ea3bbb5f2279a3 06c67961bbaed34a127f76d261f4c0bf73eda471 R1234 +COPYING +COPYING.2 +EOF + +test_expect_success \ + 'validate output from rename/copy detection (#1)' \ + 'compare_diff_raw current expected' + +# make sure diff-helper can grok it. +mv current diff-raw +GIT_DIFF_OPTS=--unified=0 git-diff-helper -z <diff-raw >current +cat >expected <<\EOF +diff --git a/COPYING b/COPYING.1 +copy from COPYING +copy to COPYING.1 +--- a/COPYING ++++ b/COPYING.1 +@@ -6 +6 @@ +- HOWEVER, in order to allow a migration to GPLv3 if that seems like ++ However, in order to allow a migration to GPLv3 if that seems like +diff --git a/COPYING b/COPYING.2 +rename old COPYING +rename new COPYING.2 +--- a/COPYING ++++ b/COPYING.2 +@@ -2 +2 @@ +- Note that the only valid version of the GPL as far as this project ++ Note that the only valid version of the G.P.L as far as this project +@@ -6 +6 @@ +- HOWEVER, in order to allow a migration to GPLv3 if that seems like ++ HOWEVER, in order to allow a migration to G.P.Lv3 if that seems like +@@ -12 +12 @@ +- This file is licensed under the GPL v2, or a later version ++ This file is licensed under the G.P.L v2, or a later version +EOF + +test_expect_success \ + 'validate output from diff-helper (#1)' \ + 'compare_diff_patch current expected' + +################################################################ + +test_expect_success \ + 'prepare work tree again' \ + 'mv COPYING.2 COPYING && + git-update-cache --add --remove COPYING COPYING.1 COPYING.2' + +# tree has COPYING and rezrov. work tree has COPYING and COPYING.1, +# both are slightly edited, and unchanged rezrov. We say COPYING.1 +# is based on COPYING and COPYING is still there, and do not say anything +# about rezrov. + +git-diff-cache -z -C $tree >current +cat >expected <<\EOF +:100644 100644 6ff87c4664981e4397625791c8ea3bbb5f2279a3 06c67961bbaed34a127f76d261f4c0bf73eda471 M +COPYING +:100644 100644 6ff87c4664981e4397625791c8ea3bbb5f2279a3 0603b3238a076dc6c8022aedc6648fa523a17178 C1234 +COPYING +COPYING.1 +EOF + +test_expect_success \ + 'validate output from rename/copy detection (#2)' \ + 'compare_diff_raw current expected' + +# make sure diff-helper can grok it. +mv current diff-raw +GIT_DIFF_OPTS=--unified=0 git-diff-helper -z <diff-raw >current +cat >expected <<\EOF +diff --git a/COPYING b/COPYING +--- a/COPYING ++++ b/COPYING +@@ -2 +2 @@ +- Note that the only valid version of the GPL as far as this project ++ Note that the only valid version of the G.P.L as far as this project +@@ -6 +6 @@ +- HOWEVER, in order to allow a migration to GPLv3 if that seems like ++ HOWEVER, in order to allow a migration to G.P.Lv3 if that seems like +@@ -12 +12 @@ +- This file is licensed under the GPL v2, or a later version ++ This file is licensed under the G.P.L v2, or a later version +diff --git a/COPYING b/COPYING.1 +copy from COPYING +copy to COPYING.1 +--- a/COPYING ++++ b/COPYING.1 +@@ -6 +6 @@ +- HOWEVER, in order to allow a migration to GPLv3 if that seems like ++ However, in order to allow a migration to GPLv3 if that seems like +EOF + +test_expect_success \ + 'validate output from diff-helper (#2)' \ + 'compare_diff_patch current expected' + +################################################################ + +# tree has COPYING and rezrov. work tree has the same COPYING and +# copy-edited COPYING.1, and unchanged rezrov. We should not say +# anything about rezrov nor COPYING, since the revised again diff-raw +# nows how to say Copy. + +test_expect_success \ + 'prepare work tree once again' \ + 'cat ../../COPYING >COPYING && + git-update-cache --add --remove COPYING COPYING.1' + +git-diff-cache -z -C $tree >current +cat >expected <<\EOF +:100644 100644 6ff87c4664981e4397625791c8ea3bbb5f2279a3 0603b3238a076dc6c8022aedc6648fa523a17178 C1234 +COPYING +COPYING.1 +EOF + +test_expect_success \ + 'validate output from rename/copy detection (#3)' \ + 'compare_diff_raw current expected' + +# make sure diff-helper can grok it. +mv current diff-raw +GIT_DIFF_OPTS=--unified=0 git-diff-helper -z <diff-raw >current +cat >expected <<\EOF +diff --git a/COPYING b/COPYING.1 +copy from COPYING +copy to COPYING.1 +--- a/COPYING ++++ b/COPYING.1 +@@ -6 +6 @@ +- HOWEVER, in order to allow a migration to GPLv3 if that seems like ++ However, in order to allow a migration to GPLv3 if that seems like +EOF + +test_expect_success \ + 'validate output from diff-helper (#3)' \ + 'compare_diff_patch current expected' + +test_done ------------------------------------------------ ^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH 2/3] diff: consolidate various calls into diffcore. 2005-05-29 23:41 ` [PATCH 0/3] Leftover bits after 12-series Junio C Hamano 2005-05-29 23:54 ` [PATCH 1/3] diff-helper: Fix R/C score parsing under -z flag Junio C Hamano @ 2005-05-29 23:56 ` Junio C Hamano 2005-05-29 23:56 ` [PATCH 3/3] diff: code clean-up and removal of rename hack Junio C Hamano 2005-05-30 6:58 ` [PATCH 0/4] Junio C Hamano 3 siblings, 0 replies; 45+ messages in thread From: Junio C Hamano @ 2005-05-29 23:56 UTC (permalink / raw) To: Linus Torvalds; +Cc: Git Mailing List The three diff-* brothers had a sequence of calls into diffcore that were almost identical. Introduce a new diffcore_std() function that takes all the necessary arguments to consolidate it. This will make later enhancements and changing the order of diffcore application simpler. Signed-off-by: Junio C Hamano <junkio@cox.net> --- diff-cache.c | 9 +++------ diff-files.c | 9 +++------ diff-tree.c | 7 +++---- diff.c | 12 ++++++++++++ diff.h | 4 ++++ 5 files changed, 25 insertions(+), 16 deletions(-) diff --git a/diff-cache.c b/diff-cache.c --- a/diff-cache.c +++ b/diff-cache.c @@ -240,12 +240,9 @@ int main(int argc, const char **argv) die("unable to read tree object %s", tree_name); ret = diff_cache(active_cache, active_nr); - if (pathspec) - diffcore_pathspec(pathspec); - if (detect_rename) - diffcore_rename(detect_rename, diff_score_opt); - if (pickaxe) - diffcore_pickaxe(pickaxe, pickaxe_opts); + diffcore_std(pathspec, + detect_rename, diff_score_opt, + pickaxe, pickaxe_opts); diff_flush(diff_output_format, 1); return ret; } diff --git a/diff-files.c b/diff-files.c --- a/diff-files.c +++ b/diff-files.c @@ -116,12 +116,9 @@ int main(int argc, const char **argv) show_modified(oldmode, mode, ce->sha1, null_sha1, ce->name); } - if (1 < argc) - diffcore_pathspec(argv + 1); - if (detect_rename) - diffcore_rename(detect_rename, diff_score_opt); - if (pickaxe) - diffcore_pickaxe(pickaxe, pickaxe_opts); + diffcore_std(argv + 1, + detect_rename, diff_score_opt, + pickaxe, pickaxe_opts); diff_flush(diff_output_format, 1); return 0; } diff --git a/diff-tree.c b/diff-tree.c --- a/diff-tree.c +++ b/diff-tree.c @@ -261,10 +261,9 @@ static void call_diff_setup(void) static int call_diff_flush(void) { - if (detect_rename) - diffcore_rename(detect_rename, diff_score_opt); - if (pickaxe) - diffcore_pickaxe(pickaxe, pickaxe_opts); + diffcore_std(0, + detect_rename, diff_score_opt, + pickaxe, pickaxe_opts); if (diff_queue_is_empty()) { diff_flush(DIFF_FORMAT_NO_OUTPUT, 0); return 0; diff --git a/diff.c b/diff.c --- a/diff.c +++ b/diff.c @@ -887,6 +887,18 @@ void diff_flush(int diff_output_style, i q->nr = q->alloc = 0; } +void diffcore_std(const char **paths, + int detect_rename, int rename_score, + const char *pickaxe, int pickaxe_opts) +{ + if (paths && paths[0]) + diffcore_pathspec(paths); + if (detect_rename) + diffcore_rename(detect_rename, rename_score); + if (pickaxe) + diffcore_pickaxe(pickaxe, pickaxe_opts); +} + void diff_addremove(int addremove, unsigned mode, const unsigned char *sha1, const char *base, const char *path) diff --git a/diff.h b/diff.h --- a/diff.h +++ b/diff.h @@ -43,6 +43,10 @@ extern void diffcore_pickaxe(const char extern void diffcore_pathspec(const char **pathspec); +extern void diffcore_std(const char **paths, + int detect_rename, int rename_score, + const char *pickaxe, int pickaxe_opts); + extern int diff_queue_is_empty(void); #define DIFF_FORMAT_HUMAN 0 ^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH 3/3] diff: code clean-up and removal of rename hack. 2005-05-29 23:41 ` [PATCH 0/3] Leftover bits after 12-series Junio C Hamano 2005-05-29 23:54 ` [PATCH 1/3] diff-helper: Fix R/C score parsing under -z flag Junio C Hamano 2005-05-29 23:56 ` [PATCH 2/3] diff: consolidate various calls into diffcore Junio C Hamano @ 2005-05-29 23:56 ` Junio C Hamano 2005-05-30 6:58 ` [PATCH 0/4] Junio C Hamano 3 siblings, 0 replies; 45+ messages in thread From: Junio C Hamano @ 2005-05-29 23:56 UTC (permalink / raw) To: Linus Torvalds; +Cc: Git Mailing List A new macro, DIFF_PAIR_RENAME(), is introduced to distinguish a filepair that is a rename/copy (the definition of which is src and dst are different paths, of course). This removes the hack used in the record_rename_pair() to always put a non-zero value in the score field. Signed-off-by: Junio C Hamano <junkio@cox.net> --- diff.c | 6 +++--- diffcore-rename.c | 2 +- diffcore.h | 6 +++--- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/diff.c b/diff.c --- a/diff.c +++ b/diff.c @@ -796,7 +796,7 @@ static void diff_resolve_rename_copy(voi for (j = 0; j < q->nr; j++) { pp = q->queue[j]; if (!strcmp(p->one->path, pp->one->path) && - pp->score) { + DIFF_PAIR_RENAME(pp)) { /* rename/copy are always valid * so we do not say DIFF_FILE_VALID() * on pp->one and pp->two. @@ -815,7 +815,7 @@ static void diff_resolve_rename_copy(voi * whose both sides are valid and of the same type, i.e. * either in-place edit or rename/copy edit. */ - else if (p->score) { + else if (DIFF_PAIR_RENAME(p)) { if (p->source_stays) { p->status = 'C'; continue; @@ -828,7 +828,7 @@ static void diff_resolve_rename_copy(voi pp = q->queue[j]; if (strcmp(pp->one->path, p->one->path)) continue; /* not us */ - if (!pp->score) + if (!DIFF_PAIR_RENAME(pp)) continue; /* not a rename/copy */ /* pp is a rename/copy from the same source */ p->status = 'C'; diff --git a/diffcore-rename.c b/diffcore-rename.c --- a/diffcore-rename.c +++ b/diffcore-rename.c @@ -207,7 +207,7 @@ static void record_rename_pair(struct di fill_filespec(two, dst->sha1, dst->mode); dp = diff_queue(renq, one, two); - dp->score = score ? : 1; /* make sure it is at least 1 */ + dp->score = score; dp->source_stays = rename_src[src_index].src_stays; rename_dst[dst_index].pair = dp; } diff --git a/diffcore.h b/diffcore.h --- a/diffcore.h +++ b/diffcore.h @@ -39,15 +39,15 @@ extern void diff_free_filespec_data(stru struct diff_filepair { struct diff_filespec *one; struct diff_filespec *two; - unsigned short int score; /* only valid when one and two are - * different paths - */ + unsigned short int score; char source_stays; /* all of R/C are copies */ char status; /* M C R N D U (see Documentation/diff-format.txt) */ }; #define DIFF_PAIR_UNMERGED(p) \ (!DIFF_FILE_VALID((p)->one) && !DIFF_FILE_VALID((p)->two)) +#define DIFF_PAIR_RENAME(p) (strcmp((p)->one->path, (p)->two->path)) + #define DIFF_PAIR_TYPE_CHANGED(p) \ ((S_IFMT & (p)->one->mode) != (S_IFMT & (p)->two->mode)) ^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH 0/4] 2005-05-29 23:41 ` [PATCH 0/3] Leftover bits after 12-series Junio C Hamano ` (2 preceding siblings ...) 2005-05-29 23:56 ` [PATCH 3/3] diff: code clean-up and removal of rename hack Junio C Hamano @ 2005-05-30 6:58 ` Junio C Hamano 2005-05-30 7:07 ` [PATCH 1/4] diff: further cleanup Junio C Hamano ` (3 more replies) 3 siblings, 4 replies; 45+ messages in thread From: Junio C Hamano @ 2005-05-30 6:58 UTC (permalink / raw) To: Linus Torvalds; +Cc: Git Mailing List Linus, as promised, I am sending a couple of further cleanup, and a pair of new diffcore routines. I am assuming that you would have applied the three clean-up patches I sent after you rejected some of my 12-piece set, before you use these four. They come on top of the previous three. Although I do not think they depend on the previous three, I am letting you know that that was how I tested these four: [PATCH 1/4] diff: further cleanup. [PATCH 2/4] diff: fix the culling of unneeded delete record. [PATCH 3/4] Add -B flag to diff-* brothers. [PATCH 4/4] Add -O<orderfile> option to diff-* brothers. The third one is the gem of this series. I think I covered the basics I can think of in the new test script, but there could still be cases that rename/copy detector does something interesting when broken pairs are involved. Please give it a good beating before you use it for anything important. This being diff routine, it obviously cannot corrupt your data, though. The fourth one was what both you and Petr expressed reluctance, although Thomas was supportive. I admit it is of "nice to have" category not "great we need to have it inside" category, but it is my favorite. Oh, before I forget, I was wondering if you want me to mark broken pair in any special way, just line I mark matched rename/copy pairs. Something along the lines of: diff --git a/foo b/foo dissimilarity index 100% deleted file mode 100644 --- a/foo +++ /dev/null @@ ... diff --git a/foo b/foo dissimilarity index 100% new file mode 100644 --- /dev/null +++ a/foo @@ ... ^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH 1/4] diff: further cleanup. 2005-05-30 6:58 ` [PATCH 0/4] Junio C Hamano @ 2005-05-30 7:07 ` Junio C Hamano 2005-05-30 7:08 ` [PATCH 2/4] diff: fix the culling of unneeded delete record Junio C Hamano ` (2 subsequent siblings) 3 siblings, 0 replies; 45+ messages in thread From: Junio C Hamano @ 2005-05-30 7:07 UTC (permalink / raw) To: Linus Torvalds; +Cc: Git Mailing List When preparing data to feed the external diff, we should give the mode we obtained from the caller, even when we are dealing with a file with 0{40} SHA1 (i.e. the caller said "look at the filesystem"), since the mode passed by the caller via diff_addremove() or diff_change() is always trustworthy. This is _not_ a bugfix --- the existing code stat() on the file ifself and does the same computation on st.st_mode to compute the mode the same way the caller did to give the original mode. We cannot remove the stat() call from here, but the extra computation to create the mode value is unnecessary. Signed-off-by: Junio C Hamano <junkio@cox.net> --- diff.c | 9 +++++++-- 1 files changed, 7 insertions(+), 2 deletions(-) 9c88ea020142346a56e36cea87e349814675f3f5 (from f96e0e2250f29f5ba2ae06c6b401a83fa1b828b4) diff --git a/diff.c b/diff.c --- a/diff.c +++ b/diff.c @@ -421,8 +421,13 @@ static void prepare_temp_file(const char strcpy(temp->hex, sha1_to_hex(null_sha1)); else strcpy(temp->hex, sha1_to_hex(one->sha1)); - sprintf(temp->mode, "%06o", - S_IFREG |ce_permissions(st.st_mode)); + /* Even though we may sometimes borrow the + * contents from the work tree, we always want + * one->mode. mode is trustworthy even when + * !(one->sha1_valid), as long as + * DIFF_FILE_VALID(one). + */ + sprintf(temp->mode, "%06o", one->mode); } return; } ^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH 2/4] diff: fix the culling of unneeded delete record. 2005-05-30 6:58 ` [PATCH 0/4] Junio C Hamano 2005-05-30 7:07 ` [PATCH 1/4] diff: further cleanup Junio C Hamano @ 2005-05-30 7:08 ` Junio C Hamano 2005-05-30 7:08 ` [PATCH 3/4] Add -B flag to diff-* brothers Junio C Hamano 2005-05-30 7:09 ` [PATCH 4/4] Add -O<orderfile> option " Junio C Hamano 3 siblings, 0 replies; 45+ messages in thread From: Junio C Hamano @ 2005-05-30 7:08 UTC (permalink / raw) To: Linus Torvalds; +Cc: Git Mailing List The commit 15d061b435a7e3b6bead39df3889f4af78c4b00a [PATCH] Fix the way diffcore-rename records unremoved source. still leaves unneeded delete records in its output stream by mistake, which was covered up by having an extra check to turn such a delete into a no-op downstream. Fix the check in the diffcore-rename to simplify the output routine. Signed-off-by: Junio C Hamano <junkio@cox.net> --- diff.c | 23 ++--------------------- diffcore-rename.c | 44 +++++++++++++++++++++++++++++++++----------- 2 files changed, 35 insertions(+), 32 deletions(-) 3fde726e95a828e680c95297f185d6d25fcf853a (from 9c88ea020142346a56e36cea87e349814675f3f5) diff --git a/diff.c b/diff.c --- a/diff.c +++ b/diff.c @@ -792,27 +792,8 @@ static void diff_resolve_rename_copy(voi p->status = 'U'; else if (!DIFF_FILE_VALID(p->one)) p->status = 'N'; - else if (!DIFF_FILE_VALID(p->two)) { - /* Deleted entry may have been picked up by - * another rename-copy entry. So we scan the - * queue and if we find one that uses us as the - * source we do not say delete for this entry. - */ - for (j = 0; j < q->nr; j++) { - pp = q->queue[j]; - if (!strcmp(p->one->path, pp->one->path) && - DIFF_PAIR_RENAME(pp)) { - /* rename/copy are always valid - * so we do not say DIFF_FILE_VALID() - * on pp->one and pp->two. - */ - p->status = 'X'; - break; - } - } - if (!p->status) - p->status = 'D'; - } + else if (!DIFF_FILE_VALID(p->two)) + p->status = 'D'; else if (DIFF_PAIR_TYPE_CHANGED(p)) p->status = 'T'; diff --git a/diffcore-rename.c b/diffcore-rename.c --- a/diffcore-rename.c +++ b/diffcore-rename.c @@ -328,26 +328,48 @@ void diffcore_rename(int detect_rename, outq.nr = outq.alloc = 0; for (i = 0; i < q->nr; i++) { struct diff_filepair *p = q->queue[i]; - struct diff_rename_dst *dst = locate_rename_dst(p->two, 0); struct diff_filepair *pair_to_free = NULL; - if (dst) { - /* creation */ - if (dst->pair) { - /* renq has rename/copy to produce - * this file already, so we do not - * emit the creation record in the - * output. - */ + if (!DIFF_FILE_VALID(p->one) && DIFF_FILE_VALID(p->two)) { + /* + * Creation + * + * We would output this create record if it has + * not been turned into a rename/copy already. + */ + struct diff_rename_dst *dst = + locate_rename_dst(p->two, 0); + if (dst && dst->pair) { diff_q(&outq, dst->pair); pair_to_free = p; } else - /* no matching rename/copy source, so record - * this as a creation. + /* no matching rename/copy source, so + * record this as a creation. */ diff_q(&outq, p); } + else if (DIFF_FILE_VALID(p->one) && !DIFF_FILE_VALID(p->two)) { + /* + * Deletion + * + * We would output this delete record if renq + * does not have a rename/copy to move + * p->one->path out. + */ + for (j = 0; j < renq.nr; j++) + if (!strcmp(renq.queue[j]->one->path, + p->one->path)) + break; + if (j < renq.nr) + /* this path remains */ + pair_to_free = p; + + if (pair_to_free) + ; + else + diff_q(&outq, p); + } else if (!diff_unmodified_pair(p)) /* all the usual ones need to be kept */ diff_q(&outq, p); ^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH 3/4] Add -B flag to diff-* brothers. 2005-05-30 6:58 ` [PATCH 0/4] Junio C Hamano 2005-05-30 7:07 ` [PATCH 1/4] diff: further cleanup Junio C Hamano 2005-05-30 7:08 ` [PATCH 2/4] diff: fix the culling of unneeded delete record Junio C Hamano @ 2005-05-30 7:08 ` Junio C Hamano 2005-05-30 7:09 ` [PATCH 4/4] Add -O<orderfile> option " Junio C Hamano 3 siblings, 0 replies; 45+ messages in thread From: Junio C Hamano @ 2005-05-30 7:08 UTC (permalink / raw) To: Linus Torvalds; +Cc: Git Mailing List A new diffcore transformation, diffcore-break.c, is introduced. When the -B flag is given, a patch that represents a complete rewrite is broken into a deletion followed by a creation. This makes it easier to review such a complete rewrite patch. The -B flag takes the same syntax as the -M and -C flags to specify the minimum amount of non-source material the resulting file needs to have to be considered a complete rewrite, and defaults to 99% if not specified. As the new test t4008-diff-break-rewrite.sh demonstrates, if a file is a complete rewrite, it is broken into a delete/create pair, which can further be subjected to the usual rename detection if -M or -C is used. For example, if file0 gets completely rewritten to make it as if it were rather based on file1 which itself disappeared, the following happens: The original change looks like this: file0 --> file0' (quite different from file0) file1 --> /dev/null After diffcore-break runs, it would become this: file0 --> /dev/null /dev/null --> file0' file1 --> /dev/null Then diffcore-rename matches them up: file1 --> file0' The internal score values are finer grained now. Earlier maximum of 10000 has been raised to 60000; there is no user visible changes but there is no reason to waste available bits. Signed-off-by: Junio C Hamano <junkio@cox.net> --- Documentation/git-diff-cache.txt | 5 Documentation/git-diff-files.txt | 5 Documentation/git-diff-tree.txt | 5 Makefile | 3 diff-cache.c | 11 +- diff-files.c | 8 + diff-tree.c | 8 + diff.c | 21 +++ diff.h | 5 diffcore-break.c | 127 +++++++++++++++++++++++ diffcore-rename.c | 45 ++++++-- diffcore.h | 12 +- t/t4008-diff-break-rewrite.sh | 207 +++++++++++++++++++++++++++++++++++++++ 13 files changed, 433 insertions(+), 29 deletions(-) 96c0ba825059e65743bfce718e54413c4583ff35 (from 3fde726e95a828e680c95297f185d6d25fcf853a) diff --git a/Documentation/git-diff-cache.txt b/Documentation/git-diff-cache.txt --- a/Documentation/git-diff-cache.txt +++ b/Documentation/git-diff-cache.txt @@ -9,7 +9,7 @@ git-diff-cache - Compares content and mo SYNOPSIS -------- -'git-diff-cache' [-p] [-r] [-z] [-m] [-M] [-R] [-C] [-S<string>] [--pickaxe-all] [--cached] <tree-ish> [<path>...] +'git-diff-cache' [-p] [-r] [-z] [-m] [-B] [-M] [-R] [-C] [-S<string>] [--pickaxe-all] [--cached] <tree-ish> [<path>...] DESCRIPTION ----------- @@ -35,6 +35,9 @@ OPTIONS -z:: \0 line termination on output +-B:: + Break complete rewrite changes into pairs of delete and create. + -M:: Detect renames. diff --git a/Documentation/git-diff-files.txt b/Documentation/git-diff-files.txt --- a/Documentation/git-diff-files.txt +++ b/Documentation/git-diff-files.txt @@ -9,7 +9,7 @@ git-diff-files - Compares files in the w SYNOPSIS -------- -'git-diff-files' [-p] [-q] [-r] [-z] [-M] [-C] [-R] [-S<string>] [--pickaxe-all] [<pattern>...] +'git-diff-files' [-p] [-q] [-r] [-z] [-B] [-M] [-C] [-R] [-S<string>] [--pickaxe-all] [<pattern>...] DESCRIPTION ----------- @@ -29,6 +29,9 @@ OPTIONS -R:: Output diff in reverse. +-B:: + Break complete rewrite changes into pairs of delete and create. + -M:: Detect renames. diff --git a/Documentation/git-diff-tree.txt b/Documentation/git-diff-tree.txt --- a/Documentation/git-diff-tree.txt +++ b/Documentation/git-diff-tree.txt @@ -9,7 +9,7 @@ git-diff-tree - Compares the content and SYNOPSIS -------- -'git-diff-tree' [-p] [-r] [-z] [--stdin] [-M] [-R] [-C] [-S<string>] [--pickaxe-all] [-m] [-s] [-v] [-t] <tree-ish> <tree-ish> [<pattern>]\* +'git-diff-tree' [-p] [-r] [-z] [--stdin] [-B] [-M] [-R] [-C] [-S<string>] [--pickaxe-all] [-m] [-s] [-v] [-t] <tree-ish> <tree-ish> [<pattern>]\* DESCRIPTION ----------- @@ -33,6 +33,9 @@ OPTIONS generate patch (see section on generating patches). For git-diff-tree, this flag implies '-r' as well. +-B:: + Break complete rewrite changes into pairs of delete and create. + -M:: Detect renames. diff --git a/Makefile b/Makefile --- a/Makefile +++ b/Makefile @@ -48,7 +48,7 @@ LIB_OBJS += strbuf.o LIB_H += diff.h count-delta.h LIB_OBJS += diff.o diffcore-rename.o diffcore-pickaxe.o diffcore-pathspec.o \ - count-delta.o + count-delta.o diffcore-break.o LIB_OBJS += gitenv.o @@ -130,6 +130,7 @@ diff.o: $(LIB_H) diffcore.h diffcore-rename.o : $(LIB_H) diffcore.h diffcore-pathspec.o : $(LIB_H) diffcore.h diffcore-pickaxe.o : $(LIB_H) diffcore.h +diffcore-break.o : $(LIB_H) diffcore.h test: all $(MAKE) -C t/ all diff --git a/diff-cache.c b/diff-cache.c --- a/diff-cache.c +++ b/diff-cache.c @@ -9,6 +9,7 @@ static int diff_setup_opt = 0; static int diff_score_opt = 0; static const char *pickaxe = NULL; static int pickaxe_opts = 0; +static int diff_break_opt = -1; /* A file entry went away or appeared */ static void show_file(const char *prefix, struct cache_entry *ce, unsigned char *sha1, unsigned int mode) @@ -188,6 +189,10 @@ int main(int argc, const char **argv) diff_output_format = DIFF_FORMAT_PATCH; continue; } + if (!strncmp(arg, "-B", 2)) { + diff_break_opt = diff_scoreopt_parse(arg); + continue; + } if (!strncmp(arg, "-M", 2)) { detect_rename = DIFF_DETECT_RENAME; diff_score_opt = diff_scoreopt_parse(arg); @@ -240,9 +245,11 @@ int main(int argc, const char **argv) die("unable to read tree object %s", tree_name); ret = diff_cache(active_cache, active_nr); - diffcore_std(pathspec, + + diffcore_std(pathspec ? : NULL, detect_rename, diff_score_opt, - pickaxe, pickaxe_opts); + pickaxe, pickaxe_opts, + diff_break_opt); diff_flush(diff_output_format, 1); return ret; } diff --git a/diff-files.c b/diff-files.c --- a/diff-files.c +++ b/diff-files.c @@ -15,6 +15,7 @@ static int diff_setup_opt = 0; static int diff_score_opt = 0; static const char *pickaxe = NULL; static int pickaxe_opts = 0; +static int diff_break_opt = -1; static int silent = 0; static void show_unmerge(const char *path) @@ -57,6 +58,8 @@ int main(int argc, const char **argv) pickaxe = argv[1] + 2; else if (!strcmp(argv[1], "--pickaxe-all")) pickaxe_opts = DIFF_PICKAXE_ALL; + else if (!strncmp(argv[1], "-B", 2)) + diff_break_opt = diff_scoreopt_parse(argv[1]); else if (!strncmp(argv[1], "-M", 2)) { diff_score_opt = diff_scoreopt_parse(argv[1]); detect_rename = DIFF_DETECT_RENAME; @@ -116,9 +119,10 @@ int main(int argc, const char **argv) show_modified(oldmode, mode, ce->sha1, null_sha1, ce->name); } - diffcore_std(argv + 1, + diffcore_std((1 < argc) ? argv + 1 : NULL, detect_rename, diff_score_opt, - pickaxe, pickaxe_opts); + pickaxe, pickaxe_opts, + diff_break_opt); diff_flush(diff_output_format, 1); return 0; } diff --git a/diff-tree.c b/diff-tree.c --- a/diff-tree.c +++ b/diff-tree.c @@ -14,6 +14,7 @@ static int diff_setup_opt = 0; static int diff_score_opt = 0; static const char *pickaxe = NULL; static int pickaxe_opts = 0; +static int diff_break_opt = -1; static const char *header = NULL; static const char *header_prefix = ""; @@ -263,7 +264,8 @@ static int call_diff_flush(void) { diffcore_std(0, detect_rename, diff_score_opt, - pickaxe, pickaxe_opts); + pickaxe, pickaxe_opts, + diff_break_opt); if (diff_queue_is_empty()) { diff_flush(DIFF_FORMAT_NO_OUTPUT, 0); return 0; @@ -523,6 +525,10 @@ int main(int argc, const char **argv) diff_score_opt = diff_scoreopt_parse(arg); continue; } + if (!strncmp(arg, "-B", 2)) { + diff_break_opt = diff_scoreopt_parse(arg); + continue; + } if (!strcmp(arg, "-z")) { diff_output_format = DIFF_FORMAT_MACHINE; continue; diff --git a/diff.c b/diff.c --- a/diff.c +++ b/diff.c @@ -603,6 +603,7 @@ struct diff_filepair *diff_queue(struct dp->two = two; dp->score = 0; dp->source_stays = 0; + dp->broken_pair = 0; diff_q(queue, dp); return dp; } @@ -637,6 +638,16 @@ static void diff_flush_raw(struct diff_f sprintf(status, "%c%03d", p->status, (int)(0.5 + p->score * 100.0/MAX_SCORE)); break; + case 'N': case 'D': + two_paths = 0; + if (p->score) + sprintf(status, "%c%03d", p->status, + (int)(0.5 + p->score * 100.0/MAX_SCORE)); + else { + status[0] = p->status; + status[1] = 0; + } + break; default: two_paths = 0; status[0] = p->status; @@ -760,8 +771,9 @@ void diff_debug_filepair(const struct di { diff_debug_filespec(p->one, i, "one"); diff_debug_filespec(p->two, i, "two"); - fprintf(stderr, "score %d, status %c source_stays %d\n", - p->score, p->status ? : '?', p->source_stays); + fprintf(stderr, "score %d, status %c stays %d broken %d\n", + p->score, p->status ? : '?', + p->source_stays, p->broken_pair); } void diff_debug_queue(const char *msg, struct diff_queue_struct *q) @@ -875,10 +887,13 @@ void diff_flush(int diff_output_style, i void diffcore_std(const char **paths, int detect_rename, int rename_score, - const char *pickaxe, int pickaxe_opts) + const char *pickaxe, int pickaxe_opts, + int break_opt) { if (paths && paths[0]) diffcore_pathspec(paths); + if (0 <= break_opt) + diffcore_break(break_opt); if (detect_rename) diffcore_rename(detect_rename, rename_score); if (pickaxe) diff --git a/diff.h b/diff.h --- a/diff.h +++ b/diff.h @@ -43,9 +43,12 @@ extern void diffcore_pickaxe(const char extern void diffcore_pathspec(const char **pathspec); +extern void diffcore_break(int); + extern void diffcore_std(const char **paths, int detect_rename, int rename_score, - const char *pickaxe, int pickaxe_opts); + const char *pickaxe, int pickaxe_opts, + int break_opt); extern int diff_queue_is_empty(void); diff --git a/diffcore-break.c b/diffcore-break.c new file mode 100644 --- /dev/null +++ b/diffcore-break.c @@ -0,0 +1,127 @@ +/* + * Copyright (C) 2005 Junio C Hamano + */ +#include "cache.h" +#include "diff.h" +#include "diffcore.h" +#include "delta.h" +#include "count-delta.h" + +static int very_different(struct diff_filespec *src, + struct diff_filespec *dst, + int min_score) +{ + /* dst is recorded as a modification of src. Are they so + * different that we are better off recording this as a pair + * of delete and create? min_score is the minimum amount of + * new material that must exist in the dst and not in src for + * the pair to be considered a complete rewrite, and recommended + * to be set to a very high value, 99% or so. + * + * The value we return represents the amount of new material + * that is in dst and not in src. We return 0 when we do not + * want to get the filepair broken. + */ + void *delta; + unsigned long delta_size, base_size; + + if (!S_ISREG(src->mode) || !S_ISREG(dst->mode)) + return 0; /* leave symlink rename alone */ + + if (diff_populate_filespec(src, 1) || diff_populate_filespec(dst, 1)) + return 0; /* error but caught downstream */ + + delta_size = ((src->size < dst->size) ? + (dst->size - src->size) : (src->size - dst->size)); + + /* Notice that we use max of src and dst as the base size, + * unlike rename similarity detection. This is so that we do + * not mistake a large addition as a complete rewrite. + */ + base_size = ((src->size < dst->size) ? dst->size : src->size); + + /* + * If file size difference is too big compared to the + * base_size, we declare this a complete rewrite. + */ + if (base_size * min_score < delta_size * MAX_SCORE) + return MAX_SCORE; + + if (diff_populate_filespec(src, 0) || diff_populate_filespec(dst, 0)) + return 0; /* error but caught downstream */ + + delta = diff_delta(src->data, src->size, + dst->data, dst->size, + &delta_size); + + /* A delta that has a lot of literal additions would have + * big delta_size no matter what else it does. + */ + if (base_size * min_score < delta_size * MAX_SCORE) + return MAX_SCORE; + + /* Estimate the edit size by interpreting delta. */ + delta_size = count_delta(delta, delta_size); + free(delta); + if (delta_size == UINT_MAX) + return 0; /* error in delta computation */ + + if (base_size < delta_size) + return MAX_SCORE; + + return delta_size * MAX_SCORE / base_size; +} + +void diffcore_break(int min_score) +{ + struct diff_queue_struct *q = &diff_queued_diff; + struct diff_queue_struct outq; + int i; + + if (!min_score) + min_score = DEFAULT_BREAK_SCORE; + + outq.nr = outq.alloc = 0; + outq.queue = NULL; + + for (i = 0; i < q->nr; i++) { + struct diff_filepair *p = q->queue[i]; + int score; + + /* We deal only with in-place edit of non directory. + * We do not break anything else. + */ + if (DIFF_FILE_VALID(p->one) && DIFF_FILE_VALID(p->two) && + !S_ISDIR(p->one->mode) && !S_ISDIR(p->two->mode) && + !strcmp(p->one->path, p->two->path)) { + score = very_different(p->one, p->two, min_score); + if (min_score <= score) { + /* Split this into delete and create */ + struct diff_filespec *null_one, *null_two; + struct diff_filepair *dp; + + /* deletion of one */ + null_one = alloc_filespec(p->one->path); + dp = diff_queue(&outq, p->one, null_one); + dp->score = score; + dp->broken_pair = 1; + + /* creation of two */ + null_two = alloc_filespec(p->two->path); + dp = diff_queue(&outq, null_two, p->two); + dp->score = score; + dp->broken_pair = 1; + + free(p); /* not diff_free_filepair(), we are + * reusing one and two here. + */ + continue; + } + } + diff_q(&outq, p); + } + free(q->queue); + *q = outq; + + return; +} diff --git a/diffcore-rename.c b/diffcore-rename.c --- a/diffcore-rename.c +++ b/diffcore-rename.c @@ -225,8 +225,8 @@ static int score_compare(const void *a_, int diff_scoreopt_parse(const char *opt) { int diglen, num, scale, i; - if (opt[0] != '-' || (opt[1] != 'M' && opt[1] != 'C')) - return -1; /* that is not a -M nor -C option */ + if (opt[0] != '-' || (opt[1] != 'M' && opt[1] != 'C' && opt[1] != 'B')) + return -1; /* that is not a -M, -C nor -B option */ diglen = strspn(opt+2, "0123456789"); if (diglen == 0 || strlen(opt+2) != diglen) return 0; /* use default */ @@ -249,7 +249,7 @@ void diffcore_rename(int detect_rename, int num_create, num_src, dst_cnt; if (!minimum_score) - minimum_score = DEFAULT_MINIMUM_SCORE; + minimum_score = DEFAULT_RENAME_SCORE; renq.queue = NULL; renq.nr = renq.alloc = 0; @@ -353,17 +353,36 @@ void diffcore_rename(int detect_rename, /* * Deletion * - * We would output this delete record if renq - * does not have a rename/copy to move - * p->one->path out. + * We would output this delete record if: + * + * (1) this is a broken delete and the counterpart + * broken create remains in the output; or + * (2) this is not a broken delete, and renq does + * not have a rename/copy to move p->one->path + * out. + * + * Otherwise, the counterpart broken create + * has been turned into a rename-edit; or + * delete did not have a matching create to + * begin with. */ - for (j = 0; j < renq.nr; j++) - if (!strcmp(renq.queue[j]->one->path, - p->one->path)) - break; - if (j < renq.nr) - /* this path remains */ - pair_to_free = p; + if (DIFF_PAIR_BROKEN(p)) { + /* broken delete */ + struct diff_rename_dst *dst = + locate_rename_dst(p->one, 0); + if (dst && dst->pair) + /* counterpart is now rename/copy */ + pair_to_free = p; + } + else { + for (j = 0; j < renq.nr; j++) + if (!strcmp(renq.queue[j]->one->path, + p->one->path)) + break; + if (j < renq.nr) + /* this path remains */ + pair_to_free = p; + } if (pair_to_free) ; diff --git a/diffcore.h b/diffcore.h --- a/diffcore.h +++ b/diffcore.h @@ -8,8 +8,9 @@ * (e.g. diffcore-rename, diffcore-pickaxe). Never include this header * in anything else. */ -#define MAX_SCORE 10000 -#define DEFAULT_MINIMUM_SCORE 5000 +#define MAX_SCORE 60000 +#define DEFAULT_RENAME_SCORE 30000 /* rename/copy similarity minimum (50%) */ +#define DEFAULT_BREAK_SCORE 59400 /* minimum for break to happen (99%)*/ #define RENAME_DST_MATCHED 01 @@ -40,14 +41,19 @@ struct diff_filepair { struct diff_filespec *one; struct diff_filespec *two; unsigned short int score; - char source_stays; /* all of R/C are copies */ char status; /* M C R N D U (see Documentation/diff-format.txt) */ + unsigned source_stays : 1; /* all of R/C are copies */ + unsigned broken_pair : 1; }; #define DIFF_PAIR_UNMERGED(p) \ (!DIFF_FILE_VALID((p)->one) && !DIFF_FILE_VALID((p)->two)) #define DIFF_PAIR_RENAME(p) (strcmp((p)->one->path, (p)->two->path)) +#define DIFF_PAIR_BROKEN(p) \ + ( (!DIFF_FILE_VALID((p)->one) != !DIFF_FILE_VALID((p)->two)) && \ + ((p)->broken_pair != 0) ) + #define DIFF_PAIR_TYPE_CHANGED(p) \ ((S_IFMT & (p)->one->mode) != (S_IFMT & (p)->two->mode)) diff --git a/t/t4008-diff-break-rewrite.sh b/t/t4008-diff-break-rewrite.sh new file mode 100755 --- /dev/null +++ b/t/t4008-diff-break-rewrite.sh @@ -0,0 +1,207 @@ +#!/bin/sh +# +# Copyright (c) 2005 Junio C Hamano +# + +test_description='Break and then rename + +We have two very different files, file0 and file1, registered in a tree. + +We update file1 so drastically that it is more similar to file0, and +then remove file0. With -B, changes to file1 should be broken into +separate delete and create, resulting in removal of file0, removal of +original file1 and creation of completely rewritten file1. + +Further, with -B and -M together, these three modifications should +turn into rename-edit of file0 into file1. + +Starting from the same two files in the tree, we swap file0 and file1. +With -B, this should be detected as two complete rewrites, resulting in +four changes in total. + +Further, with -B and -M together, these should turn into two renames. +' +. ./test-lib.sh + +_x40='[0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f]' +_x40="$_x40$_x40$_x40$_x40$_x40$_x40$_x40$_x40" +sanitize_diff_raw='s/ '"$_x40"' '"$_x40"' \([CDNR]\)[0-9]* / X X \1# /' +compare_diff_raw () { + # When heuristics are improved, the score numbers would change. + # Ignore them while comparing. + # Also we do not check SHA1 hash generation in this test, which + # is a job for t0000-basic.sh + + sed -e "$sanitize_diff_raw" <"$1" >.tmp-1 + sed -e "$sanitize_diff_raw" <"$2" >.tmp-2 + diff -u .tmp-1 .tmp-2 && rm -f .tmp-1 .tmp-2 +} + +test_expect_success \ + setup \ + 'cat ../../README >file0 && + cat ../../COPYING >file1 && + git-update-cache --add file0 file1 && + tree=$(git-write-tree) && + echo "$tree"' + +test_expect_success \ + 'change file1 with copy-edit of file0 and remove file0' \ + 'sed -e "s/git/GIT/" file0 >file1 && + rm -f file0 && + git-update-cache --remove file0 file1' + +test_expect_success \ + 'run diff with -B' \ + 'git-diff-cache -B --cached "$tree" >current' + +cat >expected <<\EOF +:100644 000000 f5deac7be59e7eeab8657fd9ae706fd6a57daed2 0000000000000000000000000000000000000000 D file0 +:100644 000000 6ff87c4664981e4397625791c8ea3bbb5f2279a3 0000000000000000000000000000000000000000 D100 file1 +:000000 100644 0000000000000000000000000000000000000000 11e331465a89c394dc25c780de230043750c1ec8 N100 file1 +EOF + +test_expect_success \ + 'validate result of -B (#1)' \ + 'compare_diff_raw current expected' + +test_expect_success \ + 'run diff with -B and -M' \ + 'git-diff-cache -B -M "$tree" >current' + +cat >expected <<\EOF +:100644 100644 f5deac7be59e7eeab8657fd9ae706fd6a57daed2 08bb2fb671deff4c03a4d4a0a1315dff98d5732c R100 file0 file1 +EOF + +test_expect_success \ + 'validate result of -B -M (#2)' \ + 'compare_diff_raw current expected' + +test_expect_success \ + 'swap file0 and file1' \ + 'rm -f file0 file1 && + git-read-tree -m $tree && + git-checkout-cache -f -u -a && + mv file0 tmp && + mv file1 file0 && + mv tmp file1 && + git-update-cache file0 file1' + +test_expect_success \ + 'run diff with -B' \ + 'git-diff-cache -B "$tree" >current' + +cat >expected <<\EOF +:100644 000000 f5deac7be59e7eeab8657fd9ae706fd6a57daed2 0000000000000000000000000000000000000000 D100 file0 +:000000 100644 0000000000000000000000000000000000000000 6ff87c4664981e4397625791c8ea3bbb5f2279a3 N100 file0 +:100644 000000 6ff87c4664981e4397625791c8ea3bbb5f2279a3 0000000000000000000000000000000000000000 D100 file1 +:000000 100644 0000000000000000000000000000000000000000 f5deac7be59e7eeab8657fd9ae706fd6a57daed2 N100 file1 +EOF + +test_expect_success \ + 'validate result of -B (#3)' \ + 'compare_diff_raw current expected' + +test_expect_success \ + 'run diff with -B and -M' \ + 'git-diff-cache -B -M "$tree" >current' + +cat >expected <<\EOF +:100644 100644 6ff87c4664981e4397625791c8ea3bbb5f2279a3 6ff87c4664981e4397625791c8ea3bbb5f2279a3 R100 file1 file0 +:100644 100644 f5deac7be59e7eeab8657fd9ae706fd6a57daed2 f5deac7be59e7eeab8657fd9ae706fd6a57daed2 R100 file0 file1 +EOF + +test_expect_success \ + 'validate result of -B -M (#4)' \ + 'compare_diff_raw current expected' + +test_expect_success \ + 'make file0 into something completely different' \ + 'rm -f file0 && + ln -s frotz file0 && + git-update-cache file0 file1' + +test_expect_success \ + 'run diff with -B' \ + 'git-diff-cache -B "$tree" >current' + +cat >expected <<\EOF +:100644 120000 f5deac7be59e7eeab8657fd9ae706fd6a57daed2 67be421f88824578857624f7b3dc75e99a8a1481 T file0 +:100644 000000 6ff87c4664981e4397625791c8ea3bbb5f2279a3 0000000000000000000000000000000000000000 D100 file1 +:000000 100644 0000000000000000000000000000000000000000 f5deac7be59e7eeab8657fd9ae706fd6a57daed2 N100 file1 +EOF + +test_expect_success \ + 'validate result of -B (#5)' \ + 'compare_diff_raw current expected' + +test_expect_success \ + 'run diff with -B' \ + 'git-diff-cache -B -M "$tree" >current' + +# This should not mistake file0 as the copy source of new file1 +# due to type differences. +cat >expected <<\EOF +:100644 120000 f5deac7be59e7eeab8657fd9ae706fd6a57daed2 67be421f88824578857624f7b3dc75e99a8a1481 T file0 +:100644 000000 6ff87c4664981e4397625791c8ea3bbb5f2279a3 0000000000000000000000000000000000000000 D100 file1 +:000000 100644 0000000000000000000000000000000000000000 f5deac7be59e7eeab8657fd9ae706fd6a57daed2 N100 file1 +EOF + +test_expect_success \ + 'validate result of -B -M (#6)' \ + 'compare_diff_raw current expected' + +test_expect_success \ + 'run diff with -M' \ + 'git-diff-cache -M "$tree" >current' + +# This should not mistake file0 as the copy source of new file1 +# due to type differences. +cat >expected <<\EOF +:100644 120000 f5deac7be59e7eeab8657fd9ae706fd6a57daed2 67be421f88824578857624f7b3dc75e99a8a1481 T file0 +:100644 100644 6ff87c4664981e4397625791c8ea3bbb5f2279a3 f5deac7be59e7eeab8657fd9ae706fd6a57daed2 M file1 +EOF + +test_expect_success \ + 'validate result of -M (#7)' \ + 'compare_diff_raw current expected' + +test_expect_success \ + 'file1 edited to look like file0 and file0 rename-edited to file2' \ + 'rm -f file0 file1 && + git-read-tree -m $tree && + git-checkout-cache -f -u -a && + sed -e "s/git/GIT/" file0 >file1 && + sed -e "s/git/GET/" file0 >file2 && + rm -f file0 + git-update-cache --add --remove file0 file1 file2' + +test_expect_success \ + 'run diff with -B' \ + 'git-diff-cache -B "$tree" >current' + +cat >expected <<\EOF +:100644 000000 f5deac7be59e7eeab8657fd9ae706fd6a57daed2 0000000000000000000000000000000000000000 D file0 +:100644 000000 6ff87c4664981e4397625791c8ea3bbb5f2279a3 0000000000000000000000000000000000000000 D100 file1 +:000000 100644 0000000000000000000000000000000000000000 08bb2fb671deff4c03a4d4a0a1315dff98d5732c N100 file1 +:000000 100644 0000000000000000000000000000000000000000 f5deac7be59e7eeab8657fd9ae706fd6a57daed2 N file2 +EOF + +test_expect_success \ + 'validate result of -B (#8)' \ + 'compare_diff_raw current expected' + +test_expect_success \ + 'run diff with -B -M' \ + 'git-diff-cache -B -M "$tree" >current' + +cat >expected <<\EOF +:100644 100644 f5deac7be59e7eeab8657fd9ae706fd6a57daed2 08bb2fb671deff4c03a4d4a0a1315dff98d5732c C095 file0 file1 +:100644 100644 f5deac7be59e7eeab8657fd9ae706fd6a57daed2 59f832e5c8b3f7e486be15ad0cd3e95ba9af8998 R095 file0 file2 +EOF + +test_expect_success \ + 'validate result of -B -M (#9)' \ + 'compare_diff_raw current expected' + +test_done ^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH 4/4] Add -O<orderfile> option to diff-* brothers. 2005-05-30 6:58 ` [PATCH 0/4] Junio C Hamano ` (2 preceding siblings ...) 2005-05-30 7:08 ` [PATCH 3/4] Add -B flag to diff-* brothers Junio C Hamano @ 2005-05-30 7:09 ` Junio C Hamano 3 siblings, 0 replies; 45+ messages in thread From: Junio C Hamano @ 2005-05-30 7:09 UTC (permalink / raw) To: Linus Torvalds; +Cc: Git Mailing List A new diffcore filter diffcore-order is introduced. This takes a text file each of whose line is a shell glob pattern. Patches that match a glob pattern on an earlier line in the file are output before patches that match a later line, and patches that do not match any glob pattern are output last. A typical orderfile for git project probably should look like this: README Makefile Documentation *.h *.c Signed-off-by: Junio C Hamano <junkio@cox.net> --- Documentation/git-diff-cache.txt | 6 + Documentation/git-diff-files.txt | 6 + Documentation/git-diff-tree.txt | 6 + Makefile | 3 diff-cache.c | 8 ++ diff-files.c | 6 + diff-tree.c | 8 ++ diff.c | 5 + diff.h | 7 +- diffcore-order.c | 122 +++++++++++++++++++++++++++++++++++++++ 10 files changed, 167 insertions(+), 10 deletions(-) 6575b8d66ea48891aaba3fba3eb8a4fddc596c2c (from 96c0ba825059e65743bfce718e54413c4583ff35) diff --git a/Documentation/git-diff-cache.txt b/Documentation/git-diff-cache.txt --- a/Documentation/git-diff-cache.txt +++ b/Documentation/git-diff-cache.txt @@ -9,7 +9,7 @@ git-diff-cache - Compares content and mo SYNOPSIS -------- -'git-diff-cache' [-p] [-r] [-z] [-m] [-B] [-M] [-R] [-C] [-S<string>] [--pickaxe-all] [--cached] <tree-ish> [<path>...] +'git-diff-cache' [-p] [-r] [-z] [-m] [-B] [-M] [-R] [-C] [-O<orderfile>] [-S<string>] [--pickaxe-all] [--cached] <tree-ish> [<path>...] DESCRIPTION ----------- @@ -52,6 +52,10 @@ OPTIONS changeset, not just the files that contains the change in <string>. +-O<orderfile>:: + Output the patch in the order specified in the + <orderfile>, which has one shell glob pattern per line. + -R:: Output diff in reverse. diff --git a/Documentation/git-diff-files.txt b/Documentation/git-diff-files.txt --- a/Documentation/git-diff-files.txt +++ b/Documentation/git-diff-files.txt @@ -9,7 +9,7 @@ git-diff-files - Compares files in the w SYNOPSIS -------- -'git-diff-files' [-p] [-q] [-r] [-z] [-B] [-M] [-C] [-R] [-S<string>] [--pickaxe-all] [<pattern>...] +'git-diff-files' [-p] [-q] [-r] [-z] [-B] [-M] [-C] [-R] [-O<orderfile>] [-S<string>] [--pickaxe-all] [<pattern>...] DESCRIPTION ----------- @@ -46,6 +46,10 @@ OPTIONS changeset, not just the files that contains the change in <string>. +-O<orderfile>:: + Output the patch in the order specified in the + <orderfile>, which has one shell glob pattern per line. + -r:: This flag does not mean anything. It is there only to match git-diff-tree. Unlike git-diff-tree, git-diff-files always looks diff --git a/Documentation/git-diff-tree.txt b/Documentation/git-diff-tree.txt --- a/Documentation/git-diff-tree.txt +++ b/Documentation/git-diff-tree.txt @@ -9,7 +9,7 @@ git-diff-tree - Compares the content and SYNOPSIS -------- -'git-diff-tree' [-p] [-r] [-z] [--stdin] [-B] [-M] [-R] [-C] [-S<string>] [--pickaxe-all] [-m] [-s] [-v] [-t] <tree-ish> <tree-ish> [<pattern>]\* +'git-diff-tree' [-p] [-r] [-z] [--stdin] [-B] [-M] [-R] [-C] [-O<orderfile>] [-S<string>] [--pickaxe-all] [-m] [-s] [-v] [-t] <tree-ish> <tree-ish> [<pattern>]\* DESCRIPTION ----------- @@ -53,6 +53,10 @@ OPTIONS changeset, not just the files that contains the change in <string>. +-O<orderfile>:: + Output the patch in the order specified in the + <orderfile>, which has one shell glob pattern per line. + -r:: recurse diff --git a/Makefile b/Makefile --- a/Makefile +++ b/Makefile @@ -48,7 +48,7 @@ LIB_OBJS += strbuf.o LIB_H += diff.h count-delta.h LIB_OBJS += diff.o diffcore-rename.o diffcore-pickaxe.o diffcore-pathspec.o \ - count-delta.o diffcore-break.o + count-delta.o diffcore-break.o diffcore-order.o LIB_OBJS += gitenv.o @@ -131,6 +131,7 @@ diffcore-rename.o : $(LIB_H) diffcore.h diffcore-pathspec.o : $(LIB_H) diffcore.h diffcore-pickaxe.o : $(LIB_H) diffcore.h diffcore-break.o : $(LIB_H) diffcore.h +diffcore-order.o : $(LIB_H) diffcore.h test: all $(MAKE) -C t/ all diff --git a/diff-cache.c b/diff-cache.c --- a/diff-cache.c +++ b/diff-cache.c @@ -10,6 +10,7 @@ static int diff_score_opt = 0; static const char *pickaxe = NULL; static int pickaxe_opts = 0; static int diff_break_opt = -1; +static const char *orderfile = NULL; /* A file entry went away or appeared */ static void show_file(const char *prefix, struct cache_entry *ce, unsigned char *sha1, unsigned int mode) @@ -215,6 +216,10 @@ int main(int argc, const char **argv) pickaxe = arg + 2; continue; } + if (!strncmp(arg, "-O", 2)) { + orderfile = arg + 2; + continue; + } if (!strcmp(arg, "--pickaxe-all")) { pickaxe_opts = DIFF_PICKAXE_ALL; continue; @@ -249,7 +254,8 @@ int main(int argc, const char **argv) diffcore_std(pathspec ? : NULL, detect_rename, diff_score_opt, pickaxe, pickaxe_opts, - diff_break_opt); + diff_break_opt, + orderfile); diff_flush(diff_output_format, 1); return ret; } diff --git a/diff-files.c b/diff-files.c --- a/diff-files.c +++ b/diff-files.c @@ -16,6 +16,7 @@ static int diff_score_opt = 0; static const char *pickaxe = NULL; static int pickaxe_opts = 0; static int diff_break_opt = -1; +static const char *orderfile = NULL; static int silent = 0; static void show_unmerge(const char *path) @@ -56,6 +57,8 @@ int main(int argc, const char **argv) diff_setup_opt |= DIFF_SETUP_REVERSE; else if (!strncmp(argv[1], "-S", 2)) pickaxe = argv[1] + 2; + else if (!strncmp(argv[1], "-O", 2)) + orderfile = argv[1] + 2; else if (!strcmp(argv[1], "--pickaxe-all")) pickaxe_opts = DIFF_PICKAXE_ALL; else if (!strncmp(argv[1], "-B", 2)) @@ -122,7 +125,8 @@ int main(int argc, const char **argv) diffcore_std((1 < argc) ? argv + 1 : NULL, detect_rename, diff_score_opt, pickaxe, pickaxe_opts, - diff_break_opt); + diff_break_opt, + orderfile); diff_flush(diff_output_format, 1); return 0; } diff --git a/diff-tree.c b/diff-tree.c --- a/diff-tree.c +++ b/diff-tree.c @@ -15,6 +15,7 @@ static int diff_score_opt = 0; static const char *pickaxe = NULL; static int pickaxe_opts = 0; static int diff_break_opt = -1; +static const char *orderfile = NULL; static const char *header = NULL; static const char *header_prefix = ""; @@ -265,7 +266,8 @@ static int call_diff_flush(void) diffcore_std(0, detect_rename, diff_score_opt, pickaxe, pickaxe_opts, - diff_break_opt); + diff_break_opt, + orderfile); if (diff_queue_is_empty()) { diff_flush(DIFF_FORMAT_NO_OUTPUT, 0); return 0; @@ -511,6 +513,10 @@ int main(int argc, const char **argv) pickaxe = arg + 2; continue; } + if (!strncmp(arg, "-O", 2)) { + orderfile = arg + 2; + continue; + } if (!strcmp(arg, "--pickaxe-all")) { pickaxe_opts = DIFF_PICKAXE_ALL; continue; diff --git a/diff.c b/diff.c --- a/diff.c +++ b/diff.c @@ -888,7 +888,8 @@ void diff_flush(int diff_output_style, i void diffcore_std(const char **paths, int detect_rename, int rename_score, const char *pickaxe, int pickaxe_opts, - int break_opt) + int break_opt, + const char *orderfile) { if (paths && paths[0]) diffcore_pathspec(paths); @@ -898,6 +899,8 @@ void diffcore_std(const char **paths, diffcore_rename(detect_rename, rename_score); if (pickaxe) diffcore_pickaxe(pickaxe, pickaxe_opts); + if (orderfile) + diffcore_order(orderfile); } void diff_addremove(int addremove, unsigned mode, diff --git a/diff.h b/diff.h --- a/diff.h +++ b/diff.h @@ -43,12 +43,15 @@ extern void diffcore_pickaxe(const char extern void diffcore_pathspec(const char **pathspec); -extern void diffcore_break(int); +extern void diffcore_order(const char *orderfile); + +extern void diffcore_break(int max_score); extern void diffcore_std(const char **paths, int detect_rename, int rename_score, const char *pickaxe, int pickaxe_opts, - int break_opt); + int break_opt, + const char *orderfile); extern int diff_queue_is_empty(void); diff --git a/diffcore-order.c b/diffcore-order.c new file mode 100644 --- /dev/null +++ b/diffcore-order.c @@ -0,0 +1,122 @@ +/* + * Copyright (C) 2005 Junio C Hamano + */ +#include "cache.h" +#include "diff.h" +#include "diffcore.h" +#include <fnmatch.h> + +static char **order; +static int order_cnt; + +static void prepare_order(const char *orderfile) +{ + int fd, cnt, pass; + void *map; + char *cp, *endp; + struct stat st; + + if (order) + return; + + fd = open(orderfile, O_RDONLY); + if (fd < 0) + return; + if (fstat(fd, &st)) { + close(fd); + return; + } + map = mmap(NULL, st.st_size, PROT_READ|PROT_WRITE, MAP_PRIVATE, fd, 0); + close(fd); + if (-1 == (int)(long)map) + return; + endp = map + st.st_size; + for (pass = 0; pass < 2; pass++) { + cnt = 0; + cp = map; + while (cp < endp) { + char *ep; + for (ep = cp; ep < endp && *ep != '\n'; ep++) + ; + /* cp to ep has one line */ + if (*cp == '\n' || *cp == '#') + ; /* comment */ + else if (pass == 0) + cnt++; + else { + if (*ep == '\n') { + *ep = 0; + order[cnt] = cp; + } + else { + order[cnt] = xmalloc(ep-cp+1); + memcpy(order[cnt], cp, ep-cp); + order[cnt][ep-cp] = 0; + } + cnt++; + } + if (ep < endp) + ep++; + cp = ep; + } + if (pass == 0) { + order_cnt = cnt; + order = xmalloc(sizeof(*order) * cnt); + } + } +} + +struct pair_order { + struct diff_filepair *pair; + int orig_order; + int order; +}; + +static int match_order(const char *path) +{ + int i; + char p[PATH_MAX]; + + for (i = 0; i < order_cnt; i++) { + strcpy(p, path); + while (p[0]) { + char *cp; + if (!fnmatch(order[i], p, 0)) + return i; + cp = strrchr(p, '/'); + if (!cp) + break; + *cp = 0; + } + } + return order_cnt; +} + +static int compare_pair_order(const void *a_, const void *b_) +{ + struct pair_order const *a, *b; + a = (struct pair_order const *)a_; + b = (struct pair_order const *)b_; + if (a->order != b->order) + return a->order - b->order; + return a->orig_order - b->orig_order; +} + +void diffcore_order(const char *orderfile) +{ + struct diff_queue_struct *q = &diff_queued_diff; + struct pair_order *o = xmalloc(sizeof(*o) * q->nr); + int i; + + prepare_order(orderfile); + for (i = 0; i < q->nr; i++) { + o[i].pair = q->queue[i]; + o[i].orig_order = i; + o[i].order = match_order(o[i].pair->two->path); + } + qsort(o, q->nr, sizeof(*o), compare_pair_order); + for (i = 0; i < q->nr; i++) + q->queue[i] = o[i].pair; + free(o); + return; +} ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH] Do not show empty diff in diff-cache uncached. 2005-05-29 21:52 ` Junio C Hamano 2005-05-29 23:41 ` [PATCH 0/3] Leftover bits after 12-series Junio C Hamano @ 2005-05-30 5:34 ` Linus Torvalds 2005-05-30 5:53 ` Junio C Hamano 1 sibling, 1 reply; 45+ messages in thread From: Linus Torvalds @ 2005-05-30 5:34 UTC (permalink / raw) To: Junio C Hamano; +Cc: Git Mailing List On Sun, 29 May 2005, Junio C Hamano wrote: > > Please disregard the patches you have already discarded so far; > this request-to-discard includes -O and -B enhancements. I actually like -B, it's just that that patch depended on -O and also came with a separate patch that was the reason I liked -B in the first place.. IO, if we have /* start out with files "a" and "b" */ mv b c mv a b git-update-cache --add --remove a b c git-diff-cache -B HEAD then I htink you're 100% right that it should show up as two renames in a diffs, and "-B" would catch it. I think that's a great thing. Linus ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH] Do not show empty diff in diff-cache uncached. 2005-05-30 5:34 ` [PATCH] Do not show empty diff in diff-cache uncached Linus Torvalds @ 2005-05-30 5:53 ` Junio C Hamano 0 siblings, 0 replies; 45+ messages in thread From: Junio C Hamano @ 2005-05-30 5:53 UTC (permalink / raw) To: Linus Torvalds; +Cc: Git Mailing List >>>>> "LT" == Linus Torvalds <torvalds@osdl.org> writes: LT> On Sun, 29 May 2005, Junio C Hamano wrote: >> >> Please disregard the patches you have already discarded so far; >> this request-to-discard includes -O and -B enhancements. LT> I actually like -B, it's just that that patch depended on -O and also came LT> with a separate patch that was the reason I liked -B in the first place.. Good. I actually just finished testing the complete reorder of the patches and about to start the final review before throwing the bundle at you again. In the new set, -B comes before -O. ^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH] apply.c: tolerate diff from a dirty but unchanged path 2005-05-29 18:53 ` [PATCH] Do not show empty diff in diff-cache uncached Linus Torvalds 2005-05-29 20:09 ` Junio C Hamano @ 2005-06-11 23:27 ` Junio C Hamano 2005-06-12 16:14 ` Linus Torvalds 1 sibling, 1 reply; 45+ messages in thread From: Junio C Hamano @ 2005-06-11 23:27 UTC (permalink / raw) To: Linus Torvalds; +Cc: Git Mailing List I just noticed a problem with empty "diff --git" header a dirty but otherwise unmodified path spits out. The problem is related to an old topic: Re: [PATCH] Do not show empty diff in diff-cache uncached. >>>>> "LT" == Linus Torvalds <torvalds@osdl.org> writes: LT> On Sat, 28 May 2005, Junio C Hamano wrote: >> Recent diff-cache produces "diff --git" header internally before >> calling the "diff" command, which results in an empty diff for >> such a file, cluttering the output. This patch fixes this. LT> I actually _expect_ that "git-diff-files" will show LT> files that don't match the index, even if they happen to LT> have the exact content that the index points to. It's LT> how I know whether the index is up-to-date or not. I now agree with the reasoning and think showing an empty "diff --git" header is a good thing. I had something like this, while fixing a rename/copy bug: ... #endif static void diff_resolve_rename_copy(void) diff --git a/diffcore-break.c b/diffcore-break.c diff --git a/diffcore-rename.c b/diffcore-rename.c --- a/diffcore-rename.c +++ b/diffcore-rename.c @@ -249,8 +249,12 @@ void diffcore_rename(int detect_rename, ... I touched diffcore-break.c and decided I should not change it so I reverted, which is why the empty "diff --git" for that file shows that the cache is dirty. The problem is that currently, "git-apply --stat" seems to barf reading such a patch. Here is a proposed fix. ------------ apply.c: tolerate diff from a dirty but otherwise unchanged cache. For such an entry, "diff --git" line without anything else is output, but the next "diff --git" header is not really treated as an unrecognized diff. Signed-off-by: Junio C Hamano <junkio@cox.net> --- diff --git a/apply.c b/apply.c --- a/apply.c +++ b/apply.c @@ -343,7 +343,7 @@ static int gitdiff_dissimilarity(const c */ static int gitdiff_unrecognized(const char *line, struct patch *patch) { - return -1; + return -2; } static char *git_header_name(char *line) @@ -453,8 +453,12 @@ static int parse_git_header(char *line, int oplen = strlen(p->str); if (len < oplen || memcmp(p->str, line, oplen)) continue; - if (p->fn(line + oplen, patch) < 0) + switch (p->fn(line + oplen, patch)) { + case -1: return offset; + case -2: + return -1; + } break; } } ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH] apply.c: tolerate diff from a dirty but unchanged path 2005-06-11 23:27 ` [PATCH] apply.c: tolerate diff from a dirty but unchanged path Junio C Hamano @ 2005-06-12 16:14 ` Linus Torvalds 2005-06-12 17:05 ` Linus Torvalds 0 siblings, 1 reply; 45+ messages in thread From: Linus Torvalds @ 2005-06-12 16:14 UTC (permalink / raw) To: Junio C Hamano; +Cc: Git Mailing List On Sat, 11 Jun 2005, Junio C Hamano wrote: > > Here is a proposed fix. This is wrong. It will make us reject all patches that don't end the magic sequence of "@@ -", ie it will make us reject pure renames etc. I'll fix it up, Linus ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH] apply.c: tolerate diff from a dirty but unchanged path 2005-06-12 16:14 ` Linus Torvalds @ 2005-06-12 17:05 ` Linus Torvalds 2005-06-12 18:34 ` Junio C Hamano 0 siblings, 1 reply; 45+ messages in thread From: Linus Torvalds @ 2005-06-12 17:05 UTC (permalink / raw) To: Junio C Hamano; +Cc: Git Mailing List On Sun, 12 Jun 2005, Linus Torvalds wrote: > > I'll fix it up, One-liner fix checked in: we should ignore all git headers that are just a single line. A valid git header is _always_ multiple lines: either you have the "---/+++" lines of a diff, or you have the old/new lines of a mode or name change. So the if (git_hdr_len < 0) test of the return value of "parse_git_header()" was changed into if (git_hdr_len <= len) and that cleanly solves the problem. Linus ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH] apply.c: tolerate diff from a dirty but unchanged path 2005-06-12 17:05 ` Linus Torvalds @ 2005-06-12 18:34 ` Junio C Hamano 0 siblings, 0 replies; 45+ messages in thread From: Junio C Hamano @ 2005-06-12 18:34 UTC (permalink / raw) To: Linus Torvalds; +Cc: Git Mailing List >>>>> "LT" == Linus Torvalds <torvalds@osdl.org> writes: LT> One-liner fix checked in: we should ignore all git headers that are just a LT> single line. A valid git header is _always_ multiple lines: either you LT> have the "---/+++" lines of a diff, or you have the old/new lines of a LT> mode or name change. Yup. I was about to say "it always amazes me" but by now I should instead say "I am used to see" that your fix to my crapola is always cleaner, simpler, and shorter. By the way, just to help you sort the heap of recent patches from me, here is the list of patches from me not in your tree that I think you _could_ consider. Other patches I posted to the list that have not been merged are mostly earlier attempts that I consider you have already discarded; there _could_ be something I am forgetting, but if even I myself forget, they probably do not matter ;-): ------------ This is a fix for a bug that would embarrass me unless fixed. Subject: [PATCH] Fix rename/copy when dealing with temporarily broken... Date: Sat, 11 Jun 2005 20:55:20 -0700 Message-ID: <7vwtp0p6tz.fsf@assigned-by-dhcp.cox.net> A good test case is to run and compare these two in the GIT repository. The latter case is mishandled with unpatched code: commit=7ef76925d9c19ef74874e1735e2436e56d0c4897 git-diff-tree -C $commit git-diff-tree -B -C $commit ------------ This one adjusts what the tutorial tells the user to expect to the reality with the "corrected" -B behaviour we already have in your tree. We must have this before 1.0 in order not to confuse the reader. Unless you drop -B from git-status-script, that is. Subject: [PATCH 5/4] Tutorial update to adjust for -B fix Date: Fri, 03 Jun 2005 12:11:07 -0700 Message-ID: <7vd5r3l0hg.fsf_-_@assigned-by-dhcp.cox.net> ------------ These two are enhancements that would help writing the ultra-smart three-way merge that would look at not just merge base and two heads, but the changes in the ancestry chain: Message-ID: <7vpsut7k89.fsf@assigned-by-dhcp.cox.net> Subject: [PATCH] diff-tree: --find-copies-harder Date: Fri, 10 Jun 2005 18:31:02 -0700 Message-ID: <7vr7f8p6qu.fsf@assigned-by-dhcp.cox.net> Subject: [PATCH] Add --diff-filter= output restriction to diff-* family. Date: Sat, 11 Jun 2005 20:57:13 -0700 I mentioned them in the reply to this message from you: Date: Thu, 9 Jun 2005 08:15:52 -0700 (PDT) Subject: Re: [PATCH 3/3] read-tree -m 3-way: handle more trivial merges Message-ID: <Pine.LNX.4.58.0506090800580.2286@ppc970.osdl.org> No, I think this is quite possibly wrong for several reasons. ... So I just need a little convincing that this is a good idea. ------------ These are reworked "help carrying forward local changes in 2-way fast forward". Message-ID: <7vd5qt7k2d.fsf@assigned-by-dhcp.cox.net> Subject: [PATCH 1/3] Clean up read-tree two-way tests. Date: Fri, 10 Jun 2005 18:34:34 -0700 Message-ID: <7v7jh17jzr.fsf@assigned-by-dhcp.cox.net> Subject: [PATCH 2/3] read-tree --emu23. Date: Fri, 10 Jun 2005 18:36:08 -0700 Message-ID: <7v1x797jx0.fsf@assigned-by-dhcp.cox.net> Subject: [PATCH 3/3] read-tree: fix too strong index requirement #5ALT Date: Fri, 10 Jun 2005 18:37:47 -0700 Message-ID: <7voeadxlvo.fsf@assigned-by-dhcp.cox.net> Subject: [PATCH 4/3] Finish making --emu23 equivalent to pure 2-way merge. Date: Sat, 11 Jun 2005 02:50:51 -0700 Message-ID: <7vfyvpxlqi.fsf@assigned-by-dhcp.cox.net> Subject: [PATCH 5/3] read-tree: loosen too strict index requirements Date: Sat, 11 Jun 2005 02:53:57 -0700 The first four implement --emu23, a two-tree fast forward emulated internally using the three-way mechanism. This is to help cases where the user has local changes since the old head by not refusing to run in many cases straight 2-tree fast forward would refuse, and letting the user use the usual merge-cache three-way merge postprocessing machinery to carry local changes forward instead. The changes these four introduce do affect 3-way in 3 cases, which you objected, but this set has smaller impact than my earlier attempts. It only resolves 3 extra cases the original 3-way refuses to touch (my earlier one made it to resolve 3 cases that involve removed paths as well, none of which this round touches, and leaves stages in the resulting index). The cases new code internally resolves, instead of refusing to run, are: - only "merged head" created a new path, and the index happened to have the same change; we resolve it internally by taking the "merged head" and keep the index dirty if it was (#2ALT). - only "our head" created a new path, and the cache entry is dirty (i.e. user has local changes in the work tree file); we resolve it internally by taking "our head" and keep the index dirty if it was (#3ALT). - both "merged head" and "our head" created a new path identically, and the cache entry is dirty; we resolve it internally by taking "our head" and keep the index dirty if it was (#5ALT). I think your earlier objection of "closing the door to the clever merge algorithm" would not apply to these three cases. Note that --emu23 does not need 3-way code to resolve the #2ALT case, but needs the other two cases not refused; the patch makes it resolve #2ALT case only to keep things symmetric with #3ALT case --- otherwise users of 3-way merge would get confusing (but still correct) results. The last one [5/3] deals with the case where a path is only changed in the merged head in 3-way case, and the index already happens to have a version from the merged head (probably the user was playing with a patch floating around in the mailing list before initiating the merge). In this case we can make the 3-way merge succeed instead of refusing to run, and that is what this patch is about. This does not change the outcome of the merge, and I think your "closing the door" objection would not apply to it. ------------ This is to enhance the git-*-script suite. I do not mind too much if it stays outside, but I think it would be useful to other people as well, not just me. Message-ID: <7vll5h7k5t.fsf@assigned-by-dhcp.cox.net> Subject: [PATCH] Add script for patch submission via e-mail. Date: Fri, 10 Jun 2005 18:32:30 -0700 ------------ This attempts to split the "too big main() that does it all" which you hated. Purely cosmetic at this point but as you pointed out it is a good discipline to help later maintenance to have a set of smaller single-task functions. Message-ID: <7vu0k56517.fsf_-_@assigned-by-dhcp.cox.net> Subject: [PATCH] diff-stages: unuglify the too big main() function. Date: Fri, 10 Jun 2005 18:44:36 -0700 ^ permalink raw reply [flat|nested] 45+ messages in thread
end of thread, other threads:[~2005-06-12 18:30 UTC | newest] Thread overview: 45+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2005-05-27 0:41 Broken directory pathname pruning Linus Torvalds 2005-05-27 0:42 ` Junio C Hamano 2005-05-27 0:49 ` [PATCH] allow pathspec to end with a slash Junio C Hamano 2005-05-27 0:52 ` [PATCH] allow pathspec to end with a slash (take #2) Junio C Hamano 2005-05-27 6:41 ` [PATCH] Diff updates, fixing pathspec and rename/copy interaction Junio C Hamano 2005-05-27 15:56 ` Linus Torvalds 2005-05-27 18:22 ` Junio C Hamano 2005-05-27 22:43 ` [PATCH 00/12] Diff updates Junio C Hamano 2005-05-27 22:49 ` [PATCH 01/12] Fix math thinko in similarity estimator Junio C Hamano 2005-05-27 22:50 ` [PATCH 02/12] Introduce diff_free_filepair() funcion Junio C Hamano 2005-05-27 22:51 ` [PATCH 03/12] Make pathspec only care about the detination tree Junio C Hamano 2005-05-27 22:52 ` [PATCH 04/12] Remove unused rank field from diff_core structure Junio C Hamano 2005-05-27 22:53 ` [PATCH 05/12] Do not expose internal scaling to diff-helper Junio C Hamano 2005-05-27 22:54 ` [PATCH 06/12] Remove final newline from the value of xfrm_msg variable Junio C Hamano 2005-05-27 22:54 ` [PATCH 07/12] Clean up diff_setup() to make it more extensible Junio C Hamano 2005-05-27 22:55 ` [PATCH 08/12] Remove a function not used anymore Junio C Hamano 2005-05-27 22:55 ` [PATCH 09/12] Add --pickaxe-all to diff-* brothers Junio C Hamano 2005-05-27 22:55 ` [PATCH 10/12] Fix the way diffcore-rename records unremoved source Junio C Hamano 2005-05-27 22:56 ` [PATCH 11/12] Move pathspec to the beginning of the diffcore chain Junio C Hamano 2005-05-27 22:56 ` [PATCH 12/12] Optimize diff-tree -[CM] --stdin Junio C Hamano 2005-06-04 2:17 ` Yoichi Yuasa 2005-05-27 23:03 ` [PATCH 00/12] Diff updates Junio C Hamano 2005-05-28 10:11 ` [PATCH] Do not show empty diff in diff-cache uncached Junio C Hamano 2005-05-28 19:22 ` [PATCH] Diff: two fixes Junio C Hamano 2005-05-29 4:20 ` [PATCH] diff-helper: fix R/C score parsing under -z flag Junio C Hamano 2005-05-29 5:23 ` [PATCH] diff-cache: diff-patch (-p) format fixes Junio C Hamano 2005-05-29 9:10 ` [PATCH] diff: code clean-up Junio C Hamano 2005-05-29 18:53 ` [PATCH] Do not show empty diff in diff-cache uncached Linus Torvalds 2005-05-29 20:09 ` Junio C Hamano 2005-05-29 21:52 ` Junio C Hamano 2005-05-29 23:41 ` [PATCH 0/3] Leftover bits after 12-series Junio C Hamano 2005-05-29 23:54 ` [PATCH 1/3] diff-helper: Fix R/C score parsing under -z flag Junio C Hamano 2005-05-29 23:56 ` [PATCH 2/3] diff: consolidate various calls into diffcore Junio C Hamano 2005-05-29 23:56 ` [PATCH 3/3] diff: code clean-up and removal of rename hack Junio C Hamano 2005-05-30 6:58 ` [PATCH 0/4] Junio C Hamano 2005-05-30 7:07 ` [PATCH 1/4] diff: further cleanup Junio C Hamano 2005-05-30 7:08 ` [PATCH 2/4] diff: fix the culling of unneeded delete record Junio C Hamano 2005-05-30 7:08 ` [PATCH 3/4] Add -B flag to diff-* brothers Junio C Hamano 2005-05-30 7:09 ` [PATCH 4/4] Add -O<orderfile> option " Junio C Hamano 2005-05-30 5:34 ` [PATCH] Do not show empty diff in diff-cache uncached Linus Torvalds 2005-05-30 5:53 ` Junio C Hamano 2005-06-11 23:27 ` [PATCH] apply.c: tolerate diff from a dirty but unchanged path Junio C Hamano 2005-06-12 16:14 ` Linus Torvalds 2005-06-12 17:05 ` Linus Torvalds 2005-06-12 18:34 ` Junio C Hamano
Code repositories for project(s) associated with this public inbox https://80x24.org/mirrors/git.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).