Merge branch 'tr/diff-submodule-no-reuse-worktree'
authorJunio C Hamano <gitster@pobox.com>
Fri, 14 Mar 2014 21:25:20 +0000 (14:25 -0700)
committerJunio C Hamano <gitster@pobox.com>
Fri, 14 Mar 2014 21:25:20 +0000 (14:25 -0700)
"git diff --external-diff" incorrectly fed the submodule directory
in the working tree to the external diff driver when it knew it is
the same as one of the versions being compared.

* tr/diff-submodule-no-reuse-worktree:
diff: do not reuse_worktree_file for submodules

1  2 
diff.c
t/t4020-diff-external.sh
diff --combined diff.c
index 7c59bfe2d08edd8fe0f13b5487c1af34043d7ef1,a96992a1bdc2a722dd6b03e9a4525dc90eebf3f3..e9a8874d069469f4893add139c4b5d999fc36630
--- 1/diff.c
--- 2/diff.c
+++ b/diff.c
@@@ -30,7 -30,6 +30,7 @@@ static int diff_use_color_default = -1
  static int diff_context_default = 3;
  static const char *diff_word_regex_cfg;
  static const char *external_diff_cmd_cfg;
 +static const char *diff_order_file_cfg;
  int diff_auto_refresh_index = 1;
  static int diff_mnemonic_prefix;
  static int diff_no_prefix;
@@@ -202,8 -201,6 +202,8 @@@ int git_diff_ui_config(const char *var
                return git_config_string(&external_diff_cmd_cfg, var, value);
        if (!strcmp(var, "diff.wordregex"))
                return git_config_string(&diff_word_regex_cfg, var, value);
 +      if (!strcmp(var, "diff.orderfile"))
 +              return git_config_pathname(&diff_order_file_cfg, var, value);
  
        if (!strcmp(var, "diff.ignoresubmodules"))
                handle_ignore_submodules_arg(&default_diff_options, value);
@@@ -238,7 -235,7 +238,7 @@@ int git_diff_basic_config(const char *v
        if (userdiff_config(var, value) < 0)
                return -1;
  
 -      if (!prefixcmp(var, "diff.color.") || !prefixcmp(var, "color.diff.")) {
 +      if (starts_with(var, "diff.color.") || starts_with(var, "color.diff.")) {
                int slot = parse_diff_color_slot(var, 11);
                if (slot < 0)
                        return 0;
                return 0;
        }
  
 -      if (!prefixcmp(var, "submodule."))
 +      if (starts_with(var, "submodule."))
                return parse_submodule_config_option(var, value);
  
        return git_default_config(var, value, cb);
@@@ -672,7 -669,7 +672,7 @@@ static void emit_rewrite_diff(const cha
        memset(&ecbdata, 0, sizeof(ecbdata));
        ecbdata.color_diff = want_color(o->use_color);
        ecbdata.found_changesp = &o->found_changes;
 -      ecbdata.ws_rule = whitespace_rule(name_b ? name_b : name_a);
 +      ecbdata.ws_rule = whitespace_rule(name_b);
        ecbdata.opt = o;
        if (ecbdata.ws_rule & WS_BLANK_AT_EOF) {
                mmfile_t mf1, mf2;
@@@ -1218,7 -1215,7 +1218,7 @@@ static void fn_out_consume(void *priv, 
                        diff_words_append(line, len,
                                          &ecbdata->diff_words->plus);
                        return;
 -              } else if (!prefixcmp(line, "\\ ")) {
 +              } else if (starts_with(line, "\\ ")) {
                        /*
                         * Eat the "no newline at eof" marker as if we
                         * saw a "+" or "-" line with nothing on it,
@@@ -2255,7 -2252,7 +2255,7 @@@ static void builtin_diff(const char *na
                        (!two->mode || S_ISGITLINK(two->mode))) {
                const char *del = diff_get_color_opt(o, DIFF_FILE_OLD);
                const char *add = diff_get_color_opt(o, DIFF_FILE_NEW);
 -              show_submodule_summary(o->file, one ? one->path : two->path,
 +              show_submodule_summary(o->file, one->path ? one->path : two->path,
                                line_prefix,
                                one->sha1, two->sha1, two->dirty_submodule,
                                meta, del, add, reset);
                ecbdata.label_path = lbl;
                ecbdata.color_diff = want_color(o->use_color);
                ecbdata.found_changesp = &o->found_changes;
 -              ecbdata.ws_rule = whitespace_rule(name_b ? name_b : name_a);
 +              ecbdata.ws_rule = whitespace_rule(name_b);
                if (ecbdata.ws_rule & WS_BLANK_AT_EOF)
                        check_blank_at_eof(&mf1, &mf2, &ecbdata);
                ecbdata.opt = o;
                        xdiff_set_find_func(&xecfg, pe->pattern, pe->cflags);
                if (!diffopts)
                        ;
 -              else if (!prefixcmp(diffopts, "--unified="))
 +              else if (starts_with(diffopts, "--unified="))
                        xecfg.ctxlen = strtoul(diffopts + 10, NULL, 10);
 -              else if (!prefixcmp(diffopts, "-u"))
 +              else if (starts_with(diffopts, "-u"))
                        xecfg.ctxlen = strtoul(diffopts + 2, NULL, 10);
                if (o->word_diff)
                        init_diff_words_data(&ecbdata, o, one, two);
@@@ -2845,8 -2842,9 +2845,9 @@@ static struct diff_tempfile *prepare_te
                remove_tempfile_installed = 1;
        }
  
-       if (!one->sha1_valid ||
-           reuse_worktree_file(name, one->sha1, 1)) {
+       if (!S_ISGITLINK(one->mode) &&
+           (!one->sha1_valid ||
+            reuse_worktree_file(name, one->sha1, 1))) {
                struct stat st;
                if (lstat(name, &st) < 0) {
                        if (errno == ENOENT)
@@@ -2902,16 -2900,11 +2903,16 @@@ static void run_external_diff(const cha
                              struct diff_filespec *one,
                              struct diff_filespec *two,
                              const char *xfrm_msg,
 -                            int complete_rewrite)
 +                            int complete_rewrite,
 +                            struct diff_options *o)
  {
        const char *spawn_arg[10];
        int retval;
        const char **arg = &spawn_arg[0];
 +      struct diff_queue_struct *q = &diff_queued_diff;
 +      const char *env[3] = { NULL };
 +      char env_counter[50];
 +      char env_total[50];
  
        if (one && two) {
                struct diff_tempfile *temp_one, *temp_two;
        }
        *arg = NULL;
        fflush(NULL);
 -      retval = run_command_v_opt(spawn_arg, RUN_USING_SHELL);
 +
 +      env[0] = env_counter;
 +      snprintf(env_counter, sizeof(env_counter), "GIT_DIFF_PATH_COUNTER=%d",
 +               ++o->diff_path_counter);
 +      env[1] = env_total;
 +      snprintf(env_total, sizeof(env_total), "GIT_DIFF_PATH_TOTAL=%d", q->nr);
 +
 +      retval = run_command_v_opt_cd_env(spawn_arg, RUN_USING_SHELL, NULL, env);
        remove_tempfile();
        if (retval) {
                fprintf(stderr, "external diff died, stopping at %s.\n", name);
@@@ -3057,7 -3043,7 +3058,7 @@@ static void run_diff_cmd(const char *pg
  
        if (pgm) {
                run_external_diff(pgm, name, other, one, two, xfrm_msg,
 -                                complete_rewrite);
 +                                complete_rewrite, o);
                return;
        }
        if (one && two)
@@@ -3222,8 -3208,6 +3223,8 @@@ void diff_setup(struct diff_options *op
        options->detect_rename = diff_detect_rename_default;
        options->xdl_opts |= diff_algorithm;
  
 +      options->orderfile = diff_order_file_cfg;
 +
        if (diff_no_prefix) {
                options->a_prefix = options->b_prefix = "";
        } else if (!diff_mnemonic_prefix) {
@@@ -3236,9 -3220,6 +3237,9 @@@ void diff_setup_done(struct diff_option
  {
        int count = 0;
  
 +      if (options->set_default)
 +              options->set_default(options);
 +
        if (options->output_format & DIFF_FORMAT_NAME)
                count++;
        if (options->output_format & DIFF_FORMAT_NAME_STATUS)
                options->output_format = DIFF_FORMAT_NO_OUTPUT;
                DIFF_OPT_SET(options, EXIT_WITH_STATUS);
        }
 +
 +      options->diff_path_counter = 0;
  }
  
  static int opt_arg(const char *arg, int arg_short, const char *arg_long, int *val)
@@@ -3410,10 -3389,10 +3411,10 @@@ int parse_long_opt(const char *opt, con
        if (arg[0] != '-' || arg[1] != '-')
                return 0;
        arg += strlen("--");
 -      if (prefixcmp(arg, opt))
 +      if (!starts_with(arg, opt))
                return 0;
        arg += strlen(opt);
 -      if (*arg == '=') { /* sticked form: --option=value */
 +      if (*arg == '=') { /* stuck form: --option=value */
                *optarg = arg + 1;
                return 1;
        }
@@@ -3441,7 -3420,7 +3442,7 @@@ static int stat_opt(struct diff_option
  
        switch (*arg) {
        case '-':
 -              if (!prefixcmp(arg, "-width")) {
 +              if (starts_with(arg, "-width")) {
                        arg += strlen("-width");
                        if (*arg == '=')
                                width = strtoul(arg + 1, &end, 10);
                                width = strtoul(av[1], &end, 10);
                                argcount = 2;
                        }
 -              } else if (!prefixcmp(arg, "-name-width")) {
 +              } else if (starts_with(arg, "-name-width")) {
                        arg += strlen("-name-width");
                        if (*arg == '=')
                                name_width = strtoul(arg + 1, &end, 10);
                                name_width = strtoul(av[1], &end, 10);
                                argcount = 2;
                        }
 -              } else if (!prefixcmp(arg, "-graph-width")) {
 +              } else if (starts_with(arg, "-graph-width")) {
                        arg += strlen("-graph-width");
                        if (*arg == '=')
                                graph_width = strtoul(arg + 1, &end, 10);
                                graph_width = strtoul(av[1], &end, 10);
                                argcount = 2;
                        }
 -              } else if (!prefixcmp(arg, "-count")) {
 +              } else if (starts_with(arg, "-count")) {
                        arg += strlen("-count");
                        if (*arg == '=')
                                count = strtoul(arg + 1, &end, 10);
@@@ -3525,80 -3504,6 +3526,80 @@@ static int parse_submodule_opt(struct d
        return 1;
  }
  
 +static const char diff_status_letters[] = {
 +      DIFF_STATUS_ADDED,
 +      DIFF_STATUS_COPIED,
 +      DIFF_STATUS_DELETED,
 +      DIFF_STATUS_MODIFIED,
 +      DIFF_STATUS_RENAMED,
 +      DIFF_STATUS_TYPE_CHANGED,
 +      DIFF_STATUS_UNKNOWN,
 +      DIFF_STATUS_UNMERGED,
 +      DIFF_STATUS_FILTER_AON,
 +      DIFF_STATUS_FILTER_BROKEN,
 +      '\0',
 +};
 +
 +static unsigned int filter_bit['Z' + 1];
 +
 +static void prepare_filter_bits(void)
 +{
 +      int i;
 +
 +      if (!filter_bit[DIFF_STATUS_ADDED]) {
 +              for (i = 0; diff_status_letters[i]; i++)
 +                      filter_bit[(int) diff_status_letters[i]] = (1 << i);
 +      }
 +}
 +
 +static unsigned filter_bit_tst(char status, const struct diff_options *opt)
 +{
 +      return opt->filter & filter_bit[(int) status];
 +}
 +
 +static int parse_diff_filter_opt(const char *optarg, struct diff_options *opt)
 +{
 +      int i, optch;
 +
 +      prepare_filter_bits();
 +
 +      /*
 +       * If there is a negation e.g. 'd' in the input, and we haven't
 +       * initialized the filter field with another --diff-filter, start
 +       * from full set of bits, except for AON.
 +       */
 +      if (!opt->filter) {
 +              for (i = 0; (optch = optarg[i]) != '\0'; i++) {
 +                      if (optch < 'a' || 'z' < optch)
 +                              continue;
 +                      opt->filter = (1 << (ARRAY_SIZE(diff_status_letters) - 1)) - 1;
 +                      opt->filter &= ~filter_bit[DIFF_STATUS_FILTER_AON];
 +                      break;
 +              }
 +      }
 +
 +      for (i = 0; (optch = optarg[i]) != '\0'; i++) {
 +              unsigned int bit;
 +              int negate;
 +
 +              if ('a' <= optch && optch <= 'z') {
 +                      negate = 1;
 +                      optch = toupper(optch);
 +              } else {
 +                      negate = 0;
 +              }
 +
 +              bit = (0 <= optch && optch <= 'Z') ? filter_bit[optch] : 0;
 +              if (!bit)
 +                      return optarg[i];
 +              if (negate)
 +                      opt->filter &= ~bit;
 +              else
 +                      opt->filter |= bit;
 +      }
 +      return 0;
 +}
 +
  static void enable_patch_output(int *fmt) {
        *fmt &= ~DIFF_FORMAT_NO_OUTPUT;
        *fmt |= DIFF_FORMAT_PATCH;
@@@ -3625,15 -3530,15 +3626,15 @@@ int diff_opt_parse(struct diff_options 
                options->output_format |= DIFF_FORMAT_SHORTSTAT;
        else if (!strcmp(arg, "-X") || !strcmp(arg, "--dirstat"))
                return parse_dirstat_opt(options, "");
 -      else if (!prefixcmp(arg, "-X"))
 +      else if (starts_with(arg, "-X"))
                return parse_dirstat_opt(options, arg + 2);
 -      else if (!prefixcmp(arg, "--dirstat="))
 +      else if (starts_with(arg, "--dirstat="))
                return parse_dirstat_opt(options, arg + 10);
        else if (!strcmp(arg, "--cumulative"))
                return parse_dirstat_opt(options, "cumulative");
        else if (!strcmp(arg, "--dirstat-by-file"))
                return parse_dirstat_opt(options, "files");
 -      else if (!prefixcmp(arg, "--dirstat-by-file=")) {
 +      else if (starts_with(arg, "--dirstat-by-file=")) {
                parse_dirstat_opt(options, "files");
                return parse_dirstat_opt(options, arg + 18);
        }
                options->output_format |= DIFF_FORMAT_NAME_STATUS;
        else if (!strcmp(arg, "-s") || !strcmp(arg, "--no-patch"))
                options->output_format |= DIFF_FORMAT_NO_OUTPUT;
 -      else if (!prefixcmp(arg, "--stat"))
 +      else if (starts_with(arg, "--stat"))
                /* --stat, --stat-width, --stat-name-width, or --stat-count */
                return stat_opt(options, av);
  
        /* renames options */
 -      else if (!prefixcmp(arg, "-B") || !prefixcmp(arg, "--break-rewrites=") ||
 +      else if (starts_with(arg, "-B") || starts_with(arg, "--break-rewrites=") ||
                 !strcmp(arg, "--break-rewrites")) {
                if ((options->break_opt = diff_scoreopt_parse(arg)) == -1)
                        return error("invalid argument to -B: %s", arg+2);
        }
 -      else if (!prefixcmp(arg, "-M") || !prefixcmp(arg, "--find-renames=") ||
 +      else if (starts_with(arg, "-M") || starts_with(arg, "--find-renames=") ||
                 !strcmp(arg, "--find-renames")) {
                if ((options->rename_score = diff_scoreopt_parse(arg)) == -1)
                        return error("invalid argument to -M: %s", arg+2);
        else if (!strcmp(arg, "-D") || !strcmp(arg, "--irreversible-delete")) {
                options->irreversible_delete = 1;
        }
 -      else if (!prefixcmp(arg, "-C") || !prefixcmp(arg, "--find-copies=") ||
 +      else if (starts_with(arg, "-C") || starts_with(arg, "--find-copies=") ||
                 !strcmp(arg, "--find-copies")) {
                if (options->detect_rename == DIFF_DETECT_COPY)
                        DIFF_OPT_SET(options, FIND_COPIES_HARDER);
                DIFF_OPT_CLR(options, RENAME_EMPTY);
        else if (!strcmp(arg, "--relative"))
                DIFF_OPT_SET(options, RELATIVE_NAME);
 -      else if (!prefixcmp(arg, "--relative=")) {
 +      else if (starts_with(arg, "--relative=")) {
                DIFF_OPT_SET(options, RELATIVE_NAME);
                options->prefix = arg + 11;
        }
                DIFF_OPT_CLR(options, FOLLOW_RENAMES);
        else if (!strcmp(arg, "--color"))
                options->use_color = 1;
 -      else if (!prefixcmp(arg, "--color=")) {
 +      else if (starts_with(arg, "--color=")) {
                int value = git_config_colorbool(NULL, arg+8);
                if (value < 0)
                        return error("option `color' expects \"always\", \"auto\", or \"never\"");
                options->use_color = 1;
                options->word_diff = DIFF_WORDS_COLOR;
        }
 -      else if (!prefixcmp(arg, "--color-words=")) {
 +      else if (starts_with(arg, "--color-words=")) {
                options->use_color = 1;
                options->word_diff = DIFF_WORDS_COLOR;
                options->word_regex = arg + 14;
                if (options->word_diff == DIFF_WORDS_NONE)
                        options->word_diff = DIFF_WORDS_PLAIN;
        }
 -      else if (!prefixcmp(arg, "--word-diff=")) {
 +      else if (starts_with(arg, "--word-diff=")) {
                const char *type = arg + 12;
                if (!strcmp(type, "plain"))
                        options->word_diff = DIFF_WORDS_PLAIN;
        else if (!strcmp(arg, "--ignore-submodules")) {
                DIFF_OPT_SET(options, OVERRIDE_SUBMODULE_CONFIG);
                handle_ignore_submodules_arg(options, "all");
 -      } else if (!prefixcmp(arg, "--ignore-submodules=")) {
 +      } else if (starts_with(arg, "--ignore-submodules=")) {
                DIFF_OPT_SET(options, OVERRIDE_SUBMODULE_CONFIG);
                handle_ignore_submodules_arg(options, arg + 20);
        } else if (!strcmp(arg, "--submodule"))
                DIFF_OPT_SET(options, SUBMODULE_LOG);
 -      else if (!prefixcmp(arg, "--submodule="))
 +      else if (starts_with(arg, "--submodule="))
                return parse_submodule_opt(options, arg + 12);
  
        /* misc options */
                return argcount;
        }
        else if ((argcount = parse_long_opt("diff-filter", av, &optarg))) {
 -              options->filter = optarg;
 +              int offending = parse_diff_filter_opt(optarg, options);
 +              if (offending)
 +                      die("unknown change class '%c' in --diff-filter=%s",
 +                          offending, optarg);
                return argcount;
        }
        else if (!strcmp(arg, "--abbrev"))
                options->abbrev = DEFAULT_ABBREV;
 -      else if (!prefixcmp(arg, "--abbrev=")) {
 +      else if (starts_with(arg, "--abbrev=")) {
                options->abbrev = strtoul(arg + 9, NULL, 10);
                if (options->abbrev < MINIMUM_ABBREV)
                        options->abbrev = MINIMUM_ABBREV;
@@@ -3918,15 -3820,15 +3919,15 @@@ static int diff_scoreopt_parse(const ch
        cmd = *opt++;
        if (cmd == '-') {
                /* convert the long-form arguments into short-form versions */
 -              if (!prefixcmp(opt, "break-rewrites")) {
 +              if (starts_with(opt, "break-rewrites")) {
                        opt += strlen("break-rewrites");
                        if (*opt == 0 || *opt++ == '=')
                                cmd = 'B';
 -              } else if (!prefixcmp(opt, "find-copies")) {
 +              } else if (starts_with(opt, "find-copies")) {
                        opt += strlen("find-copies");
                        if (*opt == 0 || *opt++ == '=')
                                cmd = 'C';
 -              } else if (!prefixcmp(opt, "find-renames")) {
 +              } else if (starts_with(opt, "find-renames")) {
                        opt += strlen("find-renames");
                        if (*opt == 0 || *opt++ == '=')
                                cmd = 'M';
@@@ -4131,9 -4033,9 +4132,9 @@@ void diff_debug_filespec(struct diff_fi
                DIFF_FILE_VALID(s) ? "valid" : "invalid",
                s->mode,
                s->sha1_valid ? sha1_to_hex(s->sha1) : "");
 -      fprintf(stderr, "queue[%d] %s size %lu flags %d\n",
 +      fprintf(stderr, "queue[%d] %s size %lu\n",
                x, one ? one : "",
 -              s->size, s->xfrm_flags);
 +              s->size);
  }
  
  void diff_debug_filepair(const struct diff_filepair *p, int i)
@@@ -4336,7 -4238,7 +4337,7 @@@ static void patch_id_consume(void *priv
        int new_len;
  
        /* Ignore line numbers when computing the SHA1 of the patch */
 -      if (!prefixcmp(line, "@@ -"))
 +      if (starts_with(line, "@@ -"))
                return;
  
        new_len = remove_space(line, len);
@@@ -4623,32 -4525,27 +4624,32 @@@ free_queue
        }
  }
  
 -static void diffcore_apply_filter(const char *filter)
 +static int match_filter(const struct diff_options *options, const struct diff_filepair *p)
 +{
 +      return (((p->status == DIFF_STATUS_MODIFIED) &&
 +               ((p->score &&
 +                 filter_bit_tst(DIFF_STATUS_FILTER_BROKEN, options)) ||
 +                (!p->score &&
 +                 filter_bit_tst(DIFF_STATUS_MODIFIED, options)))) ||
 +              ((p->status != DIFF_STATUS_MODIFIED) &&
 +               filter_bit_tst(p->status, options)));
 +}
 +
 +static void diffcore_apply_filter(struct diff_options *options)
  {
        int i;
        struct diff_queue_struct *q = &diff_queued_diff;
        struct diff_queue_struct outq;
 +
        DIFF_QUEUE_CLEAR(&outq);
  
 -      if (!filter)
 +      if (!options->filter)
                return;
  
 -      if (strchr(filter, DIFF_STATUS_FILTER_AON)) {
 +      if (filter_bit_tst(DIFF_STATUS_FILTER_AON, options)) {
                int found;
                for (i = found = 0; !found && i < q->nr; i++) {
 -                      struct diff_filepair *p = q->queue[i];
 -                      if (((p->status == DIFF_STATUS_MODIFIED) &&
 -                           ((p->score &&
 -                             strchr(filter, DIFF_STATUS_FILTER_BROKEN)) ||
 -                            (!p->score &&
 -                             strchr(filter, DIFF_STATUS_MODIFIED)))) ||
 -                          ((p->status != DIFF_STATUS_MODIFIED) &&
 -                           strchr(filter, p->status)))
 +                      if (match_filter(options, q->queue[i]))
                                found++;
                }
                if (found)
                /* Only the matching ones */
                for (i = 0; i < q->nr; i++) {
                        struct diff_filepair *p = q->queue[i];
 -
 -                      if (((p->status == DIFF_STATUS_MODIFIED) &&
 -                           ((p->score &&
 -                             strchr(filter, DIFF_STATUS_FILTER_BROKEN)) ||
 -                            (!p->score &&
 -                             strchr(filter, DIFF_STATUS_MODIFIED)))) ||
 -                          ((p->status != DIFF_STATUS_MODIFIED) &&
 -                           strchr(filter, p->status)))
 +                      if (match_filter(options, p))
                                diff_q(&outq, p);
                        else
                                diff_free_filepair(p);
@@@ -4689,38 -4593,6 +4690,38 @@@ static int diff_filespec_is_identical(s
        return !memcmp(one->data, two->data, one->size);
  }
  
 +static int diff_filespec_check_stat_unmatch(struct diff_filepair *p)
 +{
 +      if (p->done_skip_stat_unmatch)
 +              return p->skip_stat_unmatch_result;
 +
 +      p->done_skip_stat_unmatch = 1;
 +      p->skip_stat_unmatch_result = 0;
 +      /*
 +       * 1. Entries that come from stat info dirtiness
 +       *    always have both sides (iow, not create/delete),
 +       *    one side of the object name is unknown, with
 +       *    the same mode and size.  Keep the ones that
 +       *    do not match these criteria.  They have real
 +       *    differences.
 +       *
 +       * 2. At this point, the file is known to be modified,
 +       *    with the same mode and size, and the object
 +       *    name of one side is unknown.  Need to inspect
 +       *    the identical contents.
 +       */
 +      if (!DIFF_FILE_VALID(p->one) || /* (1) */
 +          !DIFF_FILE_VALID(p->two) ||
 +          (p->one->sha1_valid && p->two->sha1_valid) ||
 +          (p->one->mode != p->two->mode) ||
 +          diff_populate_filespec(p->one, 1) ||
 +          diff_populate_filespec(p->two, 1) ||
 +          (p->one->size != p->two->size) ||
 +          !diff_filespec_is_identical(p->one, p->two)) /* (2) */
 +              p->skip_stat_unmatch_result = 1;
 +      return p->skip_stat_unmatch_result;
 +}
 +
  static void diffcore_skip_stat_unmatch(struct diff_options *diffopt)
  {
        int i;
        for (i = 0; i < q->nr; i++) {
                struct diff_filepair *p = q->queue[i];
  
 -              /*
 -               * 1. Entries that come from stat info dirtiness
 -               *    always have both sides (iow, not create/delete),
 -               *    one side of the object name is unknown, with
 -               *    the same mode and size.  Keep the ones that
 -               *    do not match these criteria.  They have real
 -               *    differences.
 -               *
 -               * 2. At this point, the file is known to be modified,
 -               *    with the same mode and size, and the object
 -               *    name of one side is unknown.  Need to inspect
 -               *    the identical contents.
 -               */
 -              if (!DIFF_FILE_VALID(p->one) || /* (1) */
 -                  !DIFF_FILE_VALID(p->two) ||
 -                  (p->one->sha1_valid && p->two->sha1_valid) ||
 -                  (p->one->mode != p->two->mode) ||
 -                  diff_populate_filespec(p->one, 1) ||
 -                  diff_populate_filespec(p->two, 1) ||
 -                  (p->one->size != p->two->size) ||
 -                  !diff_filespec_is_identical(p->one, p->two)) /* (2) */
 +              if (diff_filespec_check_stat_unmatch(p))
                        diff_q(&outq, p);
                else {
                        /*
@@@ -4785,7 -4677,7 +4786,7 @@@ void diffcore_std(struct diff_options *
        if (!options->found_follow)
                /* See try_to_follow_renames() in tree-diff.c */
                diff_resolve_rename_copy();
 -      diffcore_apply_filter(options->filter);
 +      diffcore_apply_filter(options);
  
        if (diff_queued_diff.nr && !DIFF_OPT_TST(options, DIFF_FROM_CONTENTS))
                DIFF_OPT_SET(options, HAS_CHANGES);
@@@ -4894,7 -4786,6 +4895,7 @@@ void diff_change(struct diff_options *o
                 unsigned old_dirty_submodule, unsigned new_dirty_submodule)
  {
        struct diff_filespec *one, *two;
 +      struct diff_filepair *p;
  
        if (S_ISGITLINK(old_mode) && S_ISGITLINK(new_mode) &&
            is_submodule_ignored(concatpath, options))
        fill_filespec(two, new_sha1, new_sha1_valid, new_mode);
        one->dirty_submodule = old_dirty_submodule;
        two->dirty_submodule = new_dirty_submodule;
 +      p = diff_queue(&diff_queued_diff, one, two);
  
 -      diff_queue(&diff_queued_diff, one, two);
 -      if (!DIFF_OPT_TST(options, DIFF_FROM_CONTENTS))
 -              DIFF_OPT_SET(options, HAS_CHANGES);
 +      if (DIFF_OPT_TST(options, DIFF_FROM_CONTENTS))
 +              return;
 +
 +      if (DIFF_OPT_TST(options, QUICK) && options->skip_stat_unmatch &&
 +          !diff_filespec_check_stat_unmatch(p))
 +              return;
 +
 +      DIFF_OPT_SET(options, HAS_CHANGES);
  }
  
  struct diff_filepair *diff_unmerge(struct diff_options *options, const char *path)
diff --combined t/t4020-diff-external.sh
index bcae35ac1c93aaf79032db5ee09fd2d3400101f5,295a563496dd4c49b8b7a850a7eb2b3d00c32f9b..044620186d135fe5caeb9376665305af1c8acbe5
@@@ -177,7 -177,7 +177,7 @@@ test_expect_success 'no diff with -diff
        git diff | grep Binary
  '
  
 -echo NULZbetweenZwords | "$PERL_PATH" -pe 'y/Z/\000/' > file
 +echo NULZbetweenZwords | perl -pe 'y/Z/\000/' > file
  
  test_expect_success 'force diff with "diff"' '
        echo >.gitattributes "file diff" &&
@@@ -193,19 -193,6 +193,19 @@@ test_expect_success 'GIT_EXTERNAL_DIFF 
        GIT_EXTERNAL_DIFF=echo git diff
  '
  
 +test_expect_success 'GIT_EXTERNAL_DIFF path counter/total' '
 +      write_script external-diff.sh <<-\EOF &&
 +      echo $GIT_DIFF_PATH_COUNTER of $GIT_DIFF_PATH_TOTAL >>counter.txt
 +      EOF
 +      >counter.txt &&
 +      cat >expect <<-\EOF &&
 +      1 of 2
 +      2 of 2
 +      EOF
 +      GIT_EXTERNAL_DIFF=./external-diff.sh git diff &&
 +      test_cmp expect counter.txt
 +'
 +
  test_expect_success 'GIT_EXTERNAL_DIFF generates pretty paths' '
        touch file.ext &&
        git add file.ext &&
@@@ -226,12 -213,13 +226,13 @@@ keep_only_cr () 
  }
  
  test_expect_success 'external diff with autocrlf = true' '
-       git config core.autocrlf true &&
+       test_config core.autocrlf true &&
        GIT_EXTERNAL_DIFF=./fake-diff.sh git diff &&
        test $(wc -l < crlfed.txt) = $(cat crlfed.txt | keep_only_cr | wc -c)
  '
  
  test_expect_success 'diff --cached' '
+       test_config core.autocrlf true &&
        git add file &&
        git update-index --assume-unchanged file &&
        echo second >file &&
        test_cmp "$TEST_DIRECTORY"/t4020/diff.NUL actual
  '
  
+ test_expect_success 'clean up crlf leftovers' '
+       git update-index --no-assume-unchanged file &&
+       rm -f file* &&
+       git reset --hard
+ '
+ test_expect_success 'submodule diff' '
+       git init sub &&
+       ( cd sub && test_commit sub1 ) &&
+       git add sub &&
+       test_tick &&
+       git commit -m "add submodule" &&
+       ( cd sub && test_commit sub2 ) &&
+       write_script gather_pre_post.sh <<-\EOF &&
+       echo "$1 $4" # path, mode
+       cat "$2" # old file
+       cat "$5" # new file
+       EOF
+       GIT_EXTERNAL_DIFF=./gather_pre_post.sh git diff >actual &&
+       cat >expected <<-EOF &&
+       sub 160000
+       Subproject commit $(git rev-parse HEAD:sub)
+       Subproject commit $(cd sub && git rev-parse HEAD)
+       EOF
+       test_cmp expected actual
+ '
  test_done