Merge branch 'mg/more-textconv'
authorJunio C Hamano <gitster@pobox.com>
Wed, 23 Oct 2013 20:21:30 +0000 (13:21 -0700)
committerJunio C Hamano <gitster@pobox.com>
Wed, 23 Oct 2013 20:21:31 +0000 (13:21 -0700)
Make "git grep" and "git show" pay attention to --textconv when
dealing with blob objects.

* mg/more-textconv:
grep: honor --textconv for the case rev:path
grep: allow to use textconv filters
t7008: demonstrate behavior of grep with textconv
cat-file: do not die on --textconv without textconv filters
show: honor --textconv for blobs
diff_opt: track whether flags have been set explicitly
t4030: demonstrate behavior of show with textconv

14 files changed:
Documentation/git-grep.txt
Documentation/technical/api-diff.txt
builtin/cat-file.c
builtin/grep.c
builtin/log.c
diff.c
diff.h
grep.c
grep.h
object.c
object.h
t/t4030-diff-textconv.sh
t/t7008-grep-binary.sh
t/t8007-cat-file-textconv.sh
index 8497aa4494778134cad5ca2c2b11fb1c824e2a24..f83733490f5a881f6f197b3b62000337c500ef9d 100644 (file)
@@ -9,7 +9,7 @@ git-grep - Print lines matching a pattern
 SYNOPSIS
 --------
 [verse]
-'git grep' [-a | --text] [-I] [-i | --ignore-case] [-w | --word-regexp]
+'git grep' [-a | --text] [-I] [--textconv] [-i | --ignore-case] [-w | --word-regexp]
           [-v | --invert-match] [-h|-H] [--full-name]
           [-E | --extended-regexp] [-G | --basic-regexp]
           [-P | --perl-regexp]
@@ -80,6 +80,13 @@ OPTIONS
 --text::
        Process binary files as if they were text.
 
+--textconv::
+       Honor textconv filter settings.
+
+--no-textconv::
+       Do not honor textconv filter settings.
+       This is the default.
+
 -i::
 --ignore-case::
        Ignore case differences between the patterns and the
index 2d2ebc04b74ef2b1bf39df892d303d37076325d1..8b001de0db3628d4f02c2890d5614e067e701ed4 100644 (file)
@@ -28,7 +28,8 @@ Calling sequence
 
 * Call `diff_setup_done()`; this inspects the options set up so far for
   internal consistency and make necessary tweaking to it (e.g. if
-  textual patch output was asked, recursive behaviour is turned on).
+  textual patch output was asked, recursive behaviour is turned on);
+  the callback set_default in diff_options can be used to tweak this more.
 
 * As you find different pairs of files, call `diff_change()` to feed
   modified files, `diff_addremove()` to feed created or deleted files,
@@ -115,6 +116,13 @@ Notable members are:
        operation, but some do not have anything to do with the diffcore
        library.
 
+`touched_flags`::
+       Records whether a flag has been changed due to user request
+       (rather than just set/unset by default).
+
+`set_default`::
+       Callback which allows tweaking the options in diff_setup_done().
+
 BINARY, TEXT;;
        Affects the way how a file that is seemingly binary is treated.
 
index 41afaa534b02d8f8089973f3949ba507bd3cf94b..b2ca775a80f54fcceba344d5545fe3cb34ba0184 100644 (file)
@@ -45,6 +45,14 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name)
        case 'e':
                return !has_sha1_file(sha1);
 
+       case 'c':
+               if (!obj_context.path[0])
+                       die("git cat-file --textconv %s: <object> must be <sha1:path>",
+                           obj_name);
+
+               if (textconv_object(obj_context.path, obj_context.mode, sha1, 1, &buf, &size))
+                       break;
+
        case 'p':
                type = sha1_object_info(sha1, NULL);
                if (type < 0)
@@ -67,16 +75,6 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name)
                /* otherwise just spit out the data */
                break;
 
-       case 'c':
-               if (!obj_context.path[0])
-                       die("git cat-file --textconv %s: <object> must be <sha1:path>",
-                           obj_name);
-
-               if (!textconv_object(obj_context.path, obj_context.mode, sha1, 1, &buf, &size))
-                       die("git cat-file --textconv: unable to run textconv on %s",
-                           obj_name);
-               break;
-
        case 0:
                if (type_from_string(exp_type) == OBJ_BLOB) {
                        unsigned char blob_sha1[20];
index 03bc442e3f96ba12ec389f0fc40349a57e073dc7..63f86032d91f00fc607f7d3b26ec941bb7a4c76c 100644 (file)
@@ -458,10 +458,10 @@ static int grep_tree(struct grep_opt *opt, const struct pathspec *pathspec,
 }
 
 static int grep_object(struct grep_opt *opt, const struct pathspec *pathspec,
-                      struct object *obj, const char *name)
+                      struct object *obj, const char *name, struct object_context *oc)
 {
        if (obj->type == OBJ_BLOB)
-               return grep_sha1(opt, obj->sha1, name, 0, NULL);
+               return grep_sha1(opt, obj->sha1, name, 0, oc ? oc->path : NULL);
        if (obj->type == OBJ_COMMIT || obj->type == OBJ_TREE) {
                struct tree_desc tree;
                void *data;
@@ -503,7 +503,7 @@ static int grep_objects(struct grep_opt *opt, const struct pathspec *pathspec,
        for (i = 0; i < nr; i++) {
                struct object *real_obj;
                real_obj = deref_tag(list->objects[i].item, NULL, 0);
-               if (grep_object(opt, pathspec, real_obj, list->objects[i].name)) {
+               if (grep_object(opt, pathspec, real_obj, list->objects[i].name, list->objects[i].context)) {
                        hit = 1;
                        if (opt->status_only)
                                break;
@@ -658,6 +658,8 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
                OPT_SET_INT('I', NULL, &opt.binary,
                        N_("don't match patterns in binary files"),
                        GREP_BINARY_NOMATCH),
+               OPT_BOOL(0, "textconv", &opt.allow_textconv,
+                        N_("process binary files with textconv filters")),
                { OPTION_INTEGER, 0, "max-depth", &opt.max_depth, N_("depth"),
                        N_("descend at most <depth> levels"), PARSE_OPT_NONEG,
                        NULL, 1 },
@@ -817,12 +819,13 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
        for (i = 0; i < argc; i++) {
                const char *arg = argv[i];
                unsigned char sha1[20];
+               struct object_context oc;
                /* Is it a rev? */
-               if (!get_sha1(arg, sha1)) {
+               if (!get_sha1_with_context(arg, 0, sha1, &oc)) {
                        struct object *object = parse_object_or_die(sha1, arg);
                        if (!seen_dashdash)
                                verify_non_filename(prefix, arg);
-                       add_object_array(object, arg, &list);
+                       add_object_array_with_context(object, arg, &list, xmemdupz(&oc, sizeof(struct object_context)));
                        continue;
                }
                if (!strcmp(arg, "--")) {
index 77d0f5f3fdbcb48d75aee35a67ed57dd134c907e..b708517a3506517a25d4c05f3c9c92a660688f8d 100644 (file)
@@ -111,6 +111,7 @@ static void cmd_log_init_defaults(struct rev_info *rev)
 
        if (default_date_mode)
                rev->date_mode = parse_date_format(default_date_mode);
+       rev->diffopt.touched_flags = 0;
 }
 
 static void cmd_log_init_finish(int argc, const char **argv, const char *prefix,
@@ -436,10 +437,29 @@ static void show_tagger(char *buf, int len, struct rev_info *rev)
        strbuf_release(&out);
 }
 
-static int show_blob_object(const unsigned char *sha1, struct rev_info *rev)
+static int show_blob_object(const unsigned char *sha1, struct rev_info *rev, const char *obj_name)
 {
+       unsigned char sha1c[20];
+       struct object_context obj_context;
+       char *buf;
+       unsigned long size;
+
        fflush(stdout);
-       return stream_blob_to_fd(1, sha1, NULL, 0);
+       if (!DIFF_OPT_TOUCHED(&rev->diffopt, ALLOW_TEXTCONV) ||
+           !DIFF_OPT_TST(&rev->diffopt, ALLOW_TEXTCONV))
+               return stream_blob_to_fd(1, sha1, NULL, 0);
+
+       if (get_sha1_with_context(obj_name, 0, sha1c, &obj_context))
+               die("Not a valid object name %s", obj_name);
+       if (!obj_context.path[0] ||
+           !textconv_object(obj_context.path, obj_context.mode, sha1c, 1, &buf, &size))
+               return stream_blob_to_fd(1, sha1, NULL, 0);
+
+       if (!buf)
+               die("git show %s: bad file", obj_name);
+
+       write_or_die(1, buf, size);
+       return 0;
 }
 
 static int show_tag_object(const unsigned char *sha1, struct rev_info *rev)
@@ -525,7 +545,7 @@ int cmd_show(int argc, const char **argv, const char *prefix)
                const char *name = objects[i].name;
                switch (o->type) {
                case OBJ_BLOB:
-                       ret = show_blob_object(o->sha1, NULL);
+                       ret = show_blob_object(o->sha1, &rev, name);
                        break;
                case OBJ_TAG: {
                        struct tag *t = (struct tag *)o;
diff --git a/diff.c b/diff.c
index a04a34d0487f231481a7c9c475ac4a3e92b5ed2d..e34bf971207f7737c3b1a10968acd8ffb3ecf93d 100644 (file)
--- a/diff.c
+++ b/diff.c
@@ -3219,6 +3219,9 @@ void diff_setup_done(struct diff_options *options)
 {
        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)
diff --git a/diff.h b/diff.h
index 44092c2176468257644a19f787038a1ff3033eaa..e34232501ee8e56a1d245da1c4f95ed8b928a837 100644 (file)
--- a/diff.h
+++ b/diff.h
@@ -88,8 +88,9 @@ typedef struct strbuf *(*diff_prefix_fn_t)(struct diff_options *opt, void *data)
 #define DIFF_OPT_PICKAXE_IGNORE_CASE (1 << 30)
 
 #define DIFF_OPT_TST(opts, flag)    ((opts)->flags & DIFF_OPT_##flag)
-#define DIFF_OPT_SET(opts, flag)    ((opts)->flags |= DIFF_OPT_##flag)
-#define DIFF_OPT_CLR(opts, flag)    ((opts)->flags &= ~DIFF_OPT_##flag)
+#define DIFF_OPT_TOUCHED(opts, flag)    ((opts)->touched_flags & DIFF_OPT_##flag)
+#define DIFF_OPT_SET(opts, flag)    (((opts)->flags |= DIFF_OPT_##flag),((opts)->touched_flags |= DIFF_OPT_##flag))
+#define DIFF_OPT_CLR(opts, flag)    (((opts)->flags &= ~DIFF_OPT_##flag),((opts)->touched_flags |= DIFF_OPT_##flag))
 #define DIFF_XDL_TST(opts, flag)    ((opts)->xdl_opts & XDF_##flag)
 #define DIFF_XDL_SET(opts, flag)    ((opts)->xdl_opts |= XDF_##flag)
 #define DIFF_XDL_CLR(opts, flag)    ((opts)->xdl_opts &= ~XDF_##flag)
@@ -109,6 +110,7 @@ struct diff_options {
        const char *single_follow;
        const char *a_prefix, *b_prefix;
        unsigned flags;
+       unsigned touched_flags;
 
        /* diff-filter bits */
        unsigned int filter;
@@ -149,6 +151,8 @@ struct diff_options {
        /* to support internal diff recursion by --follow hack*/
        int found_follow;
 
+       void (*set_default)(struct diff_options *);
+
        FILE *file;
        int close_file;
 
diff --git a/grep.c b/grep.c
index bb548cae69d1eece8832c97332a91daac5f8dceb..c668034739258d0cd1e7108357da80d44891221f 100644 (file)
--- a/grep.c
+++ b/grep.c
@@ -2,6 +2,8 @@
 #include "grep.h"
 #include "userdiff.h"
 #include "xdiff-interface.h"
+#include "diff.h"
+#include "diffcore.h"
 
 static int grep_source_load(struct grep_source *gs);
 static int grep_source_is_binary(struct grep_source *gs);
@@ -1322,6 +1324,58 @@ static void std_output(struct grep_opt *opt, const void *buf, size_t size)
        fwrite(buf, size, 1, stdout);
 }
 
+static int fill_textconv_grep(struct userdiff_driver *driver,
+                             struct grep_source *gs)
+{
+       struct diff_filespec *df;
+       char *buf;
+       size_t size;
+
+       if (!driver || !driver->textconv)
+               return grep_source_load(gs);
+
+       /*
+        * The textconv interface is intimately tied to diff_filespecs, so we
+        * have to pretend to be one. If we could unify the grep_source
+        * and diff_filespec structs, this mess could just go away.
+        */
+       df = alloc_filespec(gs->path);
+       switch (gs->type) {
+       case GREP_SOURCE_SHA1:
+               fill_filespec(df, gs->identifier, 1, 0100644);
+               break;
+       case GREP_SOURCE_FILE:
+               fill_filespec(df, null_sha1, 0, 0100644);
+               break;
+       default:
+               die("BUG: attempt to textconv something without a path?");
+       }
+
+       /*
+        * fill_textconv is not remotely thread-safe; it may load objects
+        * behind the scenes, and it modifies the global diff tempfile
+        * structure.
+        */
+       grep_read_lock();
+       size = fill_textconv(driver, df, &buf);
+       grep_read_unlock();
+       free_filespec(df);
+
+       /*
+        * The normal fill_textconv usage by the diff machinery would just keep
+        * the textconv'd buf separate from the diff_filespec. But much of the
+        * grep code passes around a grep_source and assumes that its "buf"
+        * pointer is the beginning of the thing we are searching. So let's
+        * install our textconv'd version into the grep_source, taking care not
+        * to leak any existing buffer.
+        */
+       grep_source_clear_data(gs);
+       gs->buf = buf;
+       gs->size = size;
+
+       return 0;
+}
+
 static int grep_source_1(struct grep_opt *opt, struct grep_source *gs, int collect_hits)
 {
        char *bol;
@@ -1332,6 +1386,7 @@ static int grep_source_1(struct grep_opt *opt, struct grep_source *gs, int colle
        unsigned count = 0;
        int try_lookahead = 0;
        int show_function = 0;
+       struct userdiff_driver *textconv = NULL;
        enum grep_context ctx = GREP_CONTEXT_HEAD;
        xdemitconf_t xecfg;
 
@@ -1353,19 +1408,36 @@ static int grep_source_1(struct grep_opt *opt, struct grep_source *gs, int colle
        }
        opt->last_shown = 0;
 
-       switch (opt->binary) {
-       case GREP_BINARY_DEFAULT:
-               if (grep_source_is_binary(gs))
-                       binary_match_only = 1;
-               break;
-       case GREP_BINARY_NOMATCH:
-               if (grep_source_is_binary(gs))
-                       return 0; /* Assume unmatch */
-               break;
-       case GREP_BINARY_TEXT:
-               break;
-       default:
-               die("bug: unknown binary handling mode");
+       if (opt->allow_textconv) {
+               grep_source_load_driver(gs);
+               /*
+                * We might set up the shared textconv cache data here, which
+                * is not thread-safe.
+                */
+               grep_attr_lock();
+               textconv = userdiff_get_textconv(gs->driver);
+               grep_attr_unlock();
+       }
+
+       /*
+        * We know the result of a textconv is text, so we only have to care
+        * about binary handling if we are not using it.
+        */
+       if (!textconv) {
+               switch (opt->binary) {
+               case GREP_BINARY_DEFAULT:
+                       if (grep_source_is_binary(gs))
+                               binary_match_only = 1;
+                       break;
+               case GREP_BINARY_NOMATCH:
+                       if (grep_source_is_binary(gs))
+                               return 0; /* Assume unmatch */
+                       break;
+               case GREP_BINARY_TEXT:
+                       break;
+               default:
+                       die("bug: unknown binary handling mode");
+               }
        }
 
        memset(&xecfg, 0, sizeof(xecfg));
@@ -1373,7 +1445,7 @@ static int grep_source_1(struct grep_opt *opt, struct grep_source *gs, int colle
 
        try_lookahead = should_lookahead(opt);
 
-       if (grep_source_load(gs) < 0)
+       if (fill_textconv_grep(textconv, gs) < 0)
                return 0;
 
        bol = gs->buf;
diff --git a/grep.h b/grep.h
index e4a1df56a423548af4cba728811eb21b62518e15..eaaced19737dfd46e718779cecadc352ff8d5f69 100644 (file)
--- a/grep.h
+++ b/grep.h
@@ -107,6 +107,7 @@ struct grep_opt {
 #define GREP_BINARY_NOMATCH    1
 #define GREP_BINARY_TEXT       2
        int binary;
+       int allow_textconv;
        int extended;
        int use_reflog_filter;
        int pcre;
index 5f792cbfd38c0b30d763902ba5f9399992ddab89..584f7acb36b68d17678b3fd6251105241794ea48 100644 (file)
--- a/object.c
+++ b/object.c
@@ -262,18 +262,16 @@ int object_list_contains(struct object_list *list, struct object *obj)
        return 0;
 }
 
-void add_object_array(struct object *obj, const char *name, struct object_array *array)
-{
-       add_object_array_with_mode(obj, name, array, S_IFINVALID);
-}
-
 /*
  * A zero-length string to which object_array_entry::name can be
  * initialized without requiring a malloc/free.
  */
 static char object_array_slopbuf[1];
 
-void add_object_array_with_mode(struct object *obj, const char *name, struct object_array *array, unsigned mode)
+static void add_object_array_with_mode_context(struct object *obj, const char *name,
+                                              struct object_array *array,
+                                              unsigned mode,
+                                              struct object_context *context)
 {
        unsigned nr = array->nr;
        unsigned alloc = array->alloc;
@@ -296,9 +294,28 @@ void add_object_array_with_mode(struct object *obj, const char *name, struct obj
        else
                entry->name = xstrdup(name);
        entry->mode = mode;
+       entry->context = context;
        array->nr = ++nr;
 }
 
+void add_object_array(struct object *obj, const char *name, struct object_array *array)
+{
+       add_object_array_with_mode(obj, name, array, S_IFINVALID);
+}
+
+void add_object_array_with_mode(struct object *obj, const char *name, struct object_array *array, unsigned mode)
+{
+       add_object_array_with_mode_context(obj, name, array, mode, NULL);
+}
+
+void add_object_array_with_context(struct object *obj, const char *name, struct object_array *array, struct object_context *context)
+{
+       if (context)
+               add_object_array_with_mode_context(obj, name, array, context->mode, context);
+       else
+               add_object_array_with_mode_context(obj, name, array, S_IFINVALID, context);
+}
+
 void object_array_filter(struct object_array *array,
                         object_array_each_func_t want, void *cb_data)
 {
index 2ff68c52dd48842a188eb8f0c9b1e12ff27fae37..dc5df8ce1d9579a28477f91300aa7a49f37ad4e4 100644 (file)
--- a/object.h
+++ b/object.h
@@ -19,6 +19,7 @@ struct object_array {
                 */
                char *name;
                unsigned mode;
+               struct object_context *context;
        } *objects;
 };
 
@@ -91,6 +92,7 @@ int object_list_contains(struct object_list *list, struct object *obj);
 /* Object array handling .. */
 void add_object_array(struct object *obj, const char *name, struct object_array *array);
 void add_object_array_with_mode(struct object *obj, const char *name, struct object_array *array, unsigned mode);
+void add_object_array_with_context(struct object *obj, const char *name, struct object_array *array, struct object_context *context);
 
 typedef int (*object_array_each_func_t)(struct object_array_entry *, void *);
 
index f75f46f92d22451522d4676ff4ed709b49419481..aad6c7f78db34703b2f9b1ed72239cd3b4005016 100755 (executable)
@@ -58,6 +58,12 @@ test_expect_success 'diff produces text' '
        test_cmp expect.text actual
 '
 
+test_expect_success 'show commit produces text' '
+       git show HEAD >diff &&
+       find_diff <diff >actual &&
+       test_cmp expect.text actual
+'
+
 test_expect_success 'diff-tree produces binary' '
        git diff-tree -p HEAD^ HEAD >diff &&
        find_diff <diff >actual &&
@@ -84,6 +90,24 @@ test_expect_success 'status -v produces text' '
        git reset --soft HEAD@{1}
 '
 
+test_expect_success 'show blob produces binary' '
+       git show HEAD:file >actual &&
+       printf "\\0\\n\\01\\n" >expect &&
+       test_cmp expect actual
+'
+
+test_expect_success 'show --textconv blob produces text' '
+       git show --textconv HEAD:file >actual &&
+       printf "0\\n1\\n" >expect &&
+       test_cmp expect actual
+'
+
+test_expect_success 'show --no-textconv blob produces binary' '
+       git show --no-textconv HEAD:file >actual &&
+       printf "\\0\\n\\01\\n" >expect &&
+       test_cmp expect actual
+'
+
 test_expect_success 'grep-diff (-G) operates on textconv data (add)' '
        echo one >expect &&
        git log --root --format=%s -G0 >actual &&
index 26f831984d603a959e2141641269268a2f0d78c8..b146406e9c0912cdeb1a9c5bc93d41b981e971ca 100755 (executable)
@@ -145,4 +145,35 @@ test_expect_success 'grep respects not-binary diff attribute' '
        test_cmp expect actual
 '
 
+cat >nul_to_q_textconv <<'EOF'
+#!/bin/sh
+"$PERL_PATH" -pe 'y/\000/Q/' < "$1"
+EOF
+chmod +x nul_to_q_textconv
+
+test_expect_success 'setup textconv filters' '
+       echo a diff=foo >.gitattributes &&
+       git config diff.foo.textconv "\"$(pwd)\""/nul_to_q_textconv
+'
+
+test_expect_success 'grep does not honor textconv' '
+       test_must_fail git grep Qfile
+'
+
+test_expect_success 'grep --textconv honors textconv' '
+       echo "a:binaryQfile" >expect &&
+       git grep --textconv Qfile >actual &&
+       test_cmp expect actual
+'
+
+test_expect_success 'grep --no-textconv does not honor textconv' '
+       test_must_fail git grep --no-textconv Qfile
+'
+
+test_expect_success 'grep --textconv blob honors textconv' '
+       echo "HEAD:a:binaryQfile" >expect &&
+       git grep --textconv Qfile HEAD:a >actual &&
+       test_cmp expect actual
+'
+
 test_done
index b95e102891db65c184a2ee3137f1b3e20cdab2db..eacd49ade636f5be6166e05d4e200b9a324073be 100755 (executable)
@@ -20,11 +20,11 @@ test_expect_success 'setup ' '
 '
 
 cat >expected <<EOF
-fatal: git cat-file --textconv: unable to run textconv on :one.bin
+bin: test version 2
 EOF
 
 test_expect_success 'no filter specified' '
-       git cat-file --textconv :one.bin 2>result
+       git cat-file --textconv :one.bin >result &&
        test_cmp expected result
 '
 
@@ -34,10 +34,6 @@ test_expect_success 'setup textconv filters' '
        git config diff.test.cachetextconv false
 '
 
-cat >expected <<EOF
-bin: test version 2
-EOF
-
 test_expect_success 'cat-file without --textconv' '
        git cat-file blob :one.bin >result &&
        test_cmp expected result
@@ -71,25 +67,19 @@ test_expect_success 'cat-file --textconv on previous commit' '
 '
 
 test_expect_success 'cat-file without --textconv (symlink)' '
+       printf "%s" "one.bin" >expected &&
        git cat-file blob :symlink.bin >result &&
-       printf "%s" "one.bin" >expected
        test_cmp expected result
 '
 
 
 test_expect_success 'cat-file --textconv on index (symlink)' '
-       ! git cat-file --textconv :symlink.bin 2>result &&
-       cat >expected <<\EOF &&
-fatal: git cat-file --textconv: unable to run textconv on :symlink.bin
-EOF
+       git cat-file --textconv :symlink.bin >result &&
        test_cmp expected result
 '
 
 test_expect_success 'cat-file --textconv on HEAD (symlink)' '
-       ! git cat-file --textconv HEAD:symlink.bin 2>result &&
-       cat >expected <<EOF &&
-fatal: git cat-file --textconv: unable to run textconv on HEAD:symlink.bin
-EOF
+       git cat-file --textconv HEAD:symlink.bin >result &&
        test_cmp expected result
 '