Re: Re: git log -M -- filename is not working?

[Bo Yang <struggleyb.nku@gmail.com>]

Hi,

> But with `-p' it was doing something confusing: I used two files that
> were recently renamed, and the result was the correct log history, but
> the first patch that was shown (the rename) showed the two files as
> added.  (That's even when I added `-C' and `-M'.)  This happens even
> with a single path.  OTOH, using `--follow' with `-p' and a single
> path without your patch produces the expected result where the first
> patch is a rename (even without `-C'/`-M').

Try this one, it only change a little on Jeff's patch, my simple tests
show it works. Apply it on 'next' branch.

diff --git a/builtin/log.c b/builtin/log.c
index 6208703..0142036 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -70,9 +70,10 @@ static void cmd_log_init(int argc, const char
**argv, const char *prefix,
        if (rev->diffopt.pickaxe || rev->diffopt.filter)
                rev->always_show_header = 0;
        if (DIFF_OPT_TST(&rev->diffopt, FOLLOW_RENAMES)) {
-               rev->always_show_header = 0;
-               if (rev->diffopt.nr_paths != 1)
-                       usage("git logs can only follow renames on one
pathname at a time");
+               if (!rev->diffopt.nr_paths)
+                       DIFF_OPT_CLR(&rev->diffopt, FOLLOW_RENAMES);
+               else
+                       rev->always_show_header = 0;
        }
        for (i = 1; i < argc; i++) {
                const char *arg = argv[i];
diff --git a/diffcore.h b/diffcore.h
index 491bea0..9039a06 100644
--- a/diffcore.h
+++ b/diffcore.h
@@ -99,6 +99,10 @@ struct diff_queue_struct {
                (q)->nr = (q)->alloc = 0; \
                (q)->run = 0; \
        } while(0);
+#define DIFF_QUEUE_MARK_RUN(q, r) \
+       do { \
+               (q)->run = (r); \
+       } while(0);

 extern struct diff_queue_struct diff_queued_diff;
 extern struct diff_filepair *diff_queue(struct diff_queue_struct *,
diff --git a/tree-diff.c b/tree-diff.c
index 1fb3e94..dd95f74 100644
--- a/tree-diff.c
+++ b/tree-diff.c
@@ -5,6 +5,7 @@
 #include "diff.h"
 #include "diffcore.h"
 #include "tree.h"
+#include "dir.h"

 static char *malloc_base(const char *base, int baselen, const char
*path, int pathlen)
 {
@@ -322,78 +323,74 @@ int diff_tree(struct tree_desc *t1, struct
tree_desc *t2, const char *base, stru
 }

 /*
- * Does it look like the resulting diff might be due to a rename?
- *  - single entry
- *  - not a valid previous file
+ * Does it look like the resulting diff might be due to a rename? If we have
+ * any files that appeared, then maybe.
  */
 static inline int diff_might_be_rename(void)
 {
-       return diff_queued_diff.nr == 1 &&
-               !DIFF_FILE_VALID(diff_queued_diff.queue[0]->one);
+       int i;
+       for (i = 0; i < diff_queued_diff.nr; i++)
+               if (!DIFF_FILE_VALID(diff_queued_diff.queue[i]->one))
+                       return 1;
+       return 0;
 }

 static void try_to_follow_renames(struct tree_desc *t1, struct
tree_desc *t2, const char *base, struct diff_options *opt)
 {
        struct diff_options diff_opts;
+       struct diff_queue_struct outq;
        struct diff_queue_struct *q = &diff_queued_diff;
-       struct diff_filepair *choice;
-       const char *paths[1];
+       const char **new_paths;
+       int new_paths_num, new_paths_alloc;
        int i;

-       /* Remove the file creation entry from the diff queue, and
remember it */
-       choice = q->queue[0];
+       DIFF_QUEUE_CLEAR(&outq);
        q->nr = 0;

        diff_setup(&diff_opts);
        DIFF_OPT_SET(&diff_opts, RECURSIVE);
        DIFF_OPT_SET(&diff_opts, FIND_COPIES_HARDER);
        diff_opts.output_format = DIFF_FORMAT_NO_OUTPUT;
-       diff_opts.single_follow = opt->paths[0];
        diff_opts.break_opt = opt->break_opt;
-       paths[0] = NULL;
-       diff_tree_setup_paths(paths, &diff_opts);
        if (diff_setup_done(&diff_opts) < 0)
                die("unable to set up diff options to follow renames");
        diff_tree(t1, t2, base, &diff_opts);
        diffcore_std(&diff_opts);
-       diff_tree_release_paths(&diff_opts);

-       /* Go through the new set of filepairing, and see if we find a
more interesting one */
+       new_paths_num = new_paths_alloc = opt->nr_paths;
+       new_paths = xmalloc(opt->nr_paths * sizeof(*new_paths));
+       for (i = 0; i < opt->nr_paths; i++)
+               new_paths[i] = opt->paths[i];
+
+       /* Go through the new set of filepairs, looking for renames. */
        for (i = 0; i < q->nr; i++) {
                struct diff_filepair *p = q->queue[i];

-               /*
-                * Found a source? Not only do we use that for the new
-                * diff_queued_diff, we will also use that as the path in
-                * the future!
-                */
-               if ((p->status == 'R' || p->status == 'C') &&
!strcmp(p->two->path, opt->paths[0])) {
-                       /* Switch the file-pairs around */
-                       q->queue[i] = choice;
-                       choice = p;
-
-                       /* Update the path we use from now on.. */
-                       diff_tree_release_paths(opt);
-                       opt->paths[0] = xstrdup(p->one->path);
-                       diff_tree_setup_paths(opt->paths, opt);
-                       break;
-               }
-       }
+               /* XXX actually this is the slightly different wildcarding
+                * pathspec. We really want to check just prefixes. But
+                * I wonder if we can convince the diff machinery to just
+                * be interested in these paths as destinations, but use
+                * the whole tree as sources */
+               if (!match_pathspec(opt->paths, p->two->path,
+                                  strlen(p->two->path), 0, NULL))
+                       continue;

-       /*
-        * Then, discard all the non-relevant file pairs...
-        */
-       for (i = 0; i < q->nr; i++) {
-               struct diff_filepair *p = q->queue[i];
-               diff_free_filepair(p);
+               diff_q(&outq, p);
+
+               ALLOC_GROW(new_paths, new_paths_num + 1, new_paths_alloc);
+               new_paths[new_paths_num++] = xstrdup(p->one->path);
        }

-       /*
-        * .. and re-instate the one we want (which might be either the
-        * original one, or the rename/copy we found)
-        */
-       q->queue[0] = choice;
-       q->nr = 1;
+       /* now finalize the new paths */
+       ALLOC_GROW(new_paths, new_paths_num + 1, new_paths_alloc);
+       new_paths[new_paths_num] = NULL;
+       diff_tree_release_paths(opt);
+       diff_tree_setup_paths(new_paths, opt);
+
+       /* and restore our old queue */
+       free(q->queue);
+       *q = outq;
+       DIFF_QUEUE_MARK_RUN(q, 1);
 }

 int diff_tree_sha1(const unsigned char *old, const unsigned char
*new, const char *base, struct diff_options *opt)


Regards!
Bo