{"thread":{"id":"24043","subject":"[PATCH v2 0/4] git-gui blame: use textconv","startedAt":"2010-06-08T13:49:14Z","lastAt":"2010-06-09T07:29:05Z","messageCount":10,"participants":["Clément Poulain","Matthieu Moy","Jeff King"],"isPatch":true,"patchVersion":2,"patchTotal":4},"messages":[{"id":"143230","messageId":"1276004958-13540-1-git-send-email-clement.poulain@ensimag.imag.fr","threadId":"24043","inReplyTo":null,"subject":"[PATCH v2 0/4] git-gui blame: use textconv","fromName":"Clément Poulain","fromEmail":"clement.poulain@ensimag.imag.fr","sentAt":"2010-06-08T13:49:14Z","receivedAt":"2010-06-08T13:49:14Z","isPatch":true,"sender":{"key":"clement.poulain@ensimag.imag.fr","avatar":null},"body":"This patch adds support of textconv to git-gui blame.\n\nIt is based on our previous work which adds textconv support to blame:\nhttp://mid.gmane.org/1275921713-3277-1-git-send-email-axel.bonnet@ensimag.imag.fr\nIt also uses a git-gui patch done by Clemens Buchacher which adds textconv\nsupport to git-gui diff: http://mid.gmane.org/20100415193944.GA5848@localhost.\n\ngit-gui blame is based on cat-file to get the content of the file in different\nrevisions, so the patch adds textconv support to cat-file.\nThe first part of this patch adds get_sha1_with_context() in order to know \nthe pathname of the concerned blob, as textconv needs it to work.\n\nClément Poulain (4):\n  sha1_name : creation of get_sha1_with_context\n  cat_file : add textconv support\n  git gui blame : add textconv support\n  test textconv support for cat-file\n\n builtin/blame.c              |    8 ++--\n builtin/cat-file.c           |   32 +++++++++++++++++--\n cache.h                      |   11 ++++++\n git-gui/git-gui.sh           |   28 ++++++++++++++++-\n git-gui/lib/blame.tcl        |   21 +++++++++++-\n git-gui/lib/diff.tcl         |    5 ++-\n git-gui/lib/option.tcl       |    1 +\n sha1_name.c                  |   30 +++++++++++++++---\n t/t8007-cat-file-textconv.sh |   70 ++++++++++++++++++++++++++++++++++++++++++\n 9 files changed, 190 insertions(+), 16 deletions(-)\n create mode 100755 t/t8007-cat-file-textconv.sh\n"},{"id":"143231","messageId":"1276004958-13540-2-git-send-email-clement.poulain@ensimag.imag.fr","threadId":"24043","inReplyTo":"1276004958-13540-1-git-send-email-clement.poulain@ensimag.imag.fr","subject":"[PATCH v2 1/4] sha1_name: add get_sha1_with_context()","fromName":"Clément Poulain","fromEmail":"clement.poulain@ensimag.imag.fr","sentAt":"2010-06-08T13:49:15Z","receivedAt":"2010-06-08T13:49:15Z","isPatch":true,"sender":{"key":"clement.poulain@ensimag.imag.fr","avatar":null},"body":"Textconv is defined by the diff driver, which is associated with a pathname,\nnot a blob. This fonction permits to know the context for the sha1 you're\nlooking for, especially his pathname\n\nSigned-off-by: Clément Poulain <clement.poulain@ensimag.imag.fr>\nSigned-off-by: Diane Gasselin <diane.gasselin@ensimag.imag.fr> \nSigned-off-by: Axel Bonnet <axel.bonnet@ensimag.imag.fr> \n---\n cache.h     |   11 +++++++++++\n sha1_name.c |   30 +++++++++++++++++++++++++-----\n 2 files changed, 36 insertions(+), 5 deletions(-)\n\ndiff --git a/cache.h b/cache.h\nindex 0f4263c..43a8c10 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -730,12 +730,23 @@ static inline unsigned int hexval(unsigned char c)\n #define MINIMUM_ABBREV 4\n #define DEFAULT_ABBREV 7\n \n+struct object_context {\n+\tunsigned char tree[20];\n+\tchar path[PATH_MAX];\n+\tunsigned mode;\n+};\n+#define OBJECT_CONTEXT_INIT  { 0, 0, 0 }\n+\n extern int get_sha1(const char *str, unsigned char *sha1);\n extern int get_sha1_with_mode_1(const char *str, unsigned char *sha1, unsigned *mode, int gently, const char *prefix);\n static inline int get_sha1_with_mode(const char *str, unsigned char *sha1, unsigned *mode)\n {\n \treturn get_sha1_with_mode_1(str, sha1, mode, 1, NULL);\n }\n+static inline int get_sha1_with_context(const char *str, unsigned char *sha1, struct object_context *orc)\n+{\n+\treturn get_sha1_with_context_1(str, sha1, orc, 1, NULL);\n+}\n extern int get_sha1_hex(const char *hex, unsigned char *sha1);\n extern char *sha1_to_hex(const unsigned char *sha1);\t/* static buffer result! */\n extern int read_ref(const char *filename, unsigned char *sha1);\ndiff --git a/sha1_name.c b/sha1_name.c\nindex bf92417..02358f9 100644\n--- a/sha1_name.c\n+++ b/sha1_name.c\n@@ -933,8 +933,8 @@ int interpret_branch_name(const char *name, struct strbuf *buf)\n  */\n int get_sha1(const char *name, unsigned char *sha1)\n {\n-\tunsigned unused;\n-\treturn get_sha1_with_mode(name, sha1, &unused);\n+\tstruct object_context unused;\n+\treturn get_sha1_with_context(name, sha1, &unused);\n }\n \n /* Must be called only when object_name:filename doesn't exist. */\n@@ -1032,11 +1032,22 @@ static void diagnose_invalid_index_path(int stage,\n \n int get_sha1_with_mode_1(const char *name, unsigned char *sha1, unsigned *mode, int gently, const char *prefix)\n {\n+\tstruct object_context orc;\n+\tint ret;\n+\tret = get_sha1_with_context_1(name, sha1, &orc, gently, prefix, NULL);\n+\t*mode = orc.mode;\n+\treturn ret;\n+}\n+\n+int get_sha1_with_context_1(const char *name, unsigned char *sha1,\n+\t\t\t    struct object_context *orc,\n+\t\t\t    int gently, const char *prefix)\n+{\n \tint ret, bracket_depth;\n \tint namelen = strlen(name);\n \tconst char *cp;\n \n-\t*mode = S_IFINVALID;\n+\torc->mode = S_IFINVALID;\n \tret = get_sha1_1(name, namelen, sha1);\n \tif (!ret)\n \t\treturn ret;\n@@ -1059,6 +1070,11 @@ int get_sha1_with_mode_1(const char *name, unsigned char *sha1, unsigned *mode,\n \t\t\tcp = name + 3;\n \t\t}\n \t\tnamelen = namelen - (cp - name);\n+\n+\t\tstrncpy(orc->path, cp,\n+\t\t\tsizeof(orc->path));\n+\t\torc->path[sizeof(orc->path)] = '\\0';\n+\n \t\tif (!active_cache)\n \t\t\tread_cache();\n \t\tpos = cache_name_pos(cp, namelen);\n@@ -1071,7 +1087,6 @@ int get_sha1_with_mode_1(const char *name, unsigned char *sha1, unsigned *mode,\n \t\t\t\tbreak;\n \t\t\tif (ce_stage(ce) == stage) {\n \t\t\t\thashcpy(sha1, ce->sha1);\n-\t\t\t\t*mode = ce->ce_mode;\n \t\t\t\treturn 0;\n \t\t\t}\n \t\t\tpos++;\n@@ -1098,12 +1113,17 @@ int get_sha1_with_mode_1(const char *name, unsigned char *sha1, unsigned *mode,\n \t\t}\n \t\tif (!get_sha1_1(name, cp-name, tree_sha1)) {\n \t\t\tconst char *filename = cp+1;\n-\t\t\tret = get_tree_entry(tree_sha1, filename, sha1, mode);\n+\t\t\tret = get_tree_entry(tree_sha1, filename, sha1, &orc->mode);\n \t\t\tif (!gently) {\n \t\t\t\tdiagnose_invalid_sha1_path(prefix, filename,\n \t\t\t\t\t\t\t   tree_sha1, object_name);\n \t\t\t\tfree(object_name);\n \t\t\t}\n+\t\t\thashcpy(orc->tree, tree_sha1);\n+\t\t\tstrncpy(orc->path, filename,\n+\t\t\t\tsizeof(orc->path));\n+\t\t\torc->path[sizeof(orc->path)] = '\\0';\n+\n \t\t\treturn ret;\n \t\t} else {\n \t\t\tif (!gently)\n-- \n1.7.1.202.g79415.dirty\n"},{"id":"143232","messageId":"1276004958-13540-3-git-send-email-clement.poulain@ensimag.imag.fr","threadId":"24043","inReplyTo":"1276004958-13540-2-git-send-email-clement.poulain@ensimag.imag.fr","subject":"[PATCH v2 2/4] textconv: support for cat_file","fromName":"Clément Poulain","fromEmail":"clement.poulain@ensimag.imag.fr","sentAt":"2010-06-08T13:49:16Z","receivedAt":"2010-06-08T13:49:16Z","isPatch":true,"sender":{"key":"clement.poulain@ensimag.imag.fr","avatar":null},"body":"Make the textconv_object function public, and add --textconv option to cat-file\nto perform conversion on blob objects. Using --textconv implies that we are\nworking on a blob.\nAs files drivers need to be initialized, a new config is required in addition\nto git_default_config. Therefore git_cat_file_config() is introduced\n\nSigned-off-by: Clément Poulain <clement.poulain@ensimag.imag.fr>\nSigned-off-by: Diane Gasselin <diane.gasselin@ensimag.imag.fr> \nSigned-off-by: Axel Bonnet <axel.bonnet@ensimag.imag.fr> \n---\n builtin/blame.c    |    8 ++++----\n builtin/cat-file.c |   32 +++++++++++++++++++++++++++++---\n 2 files changed, 33 insertions(+), 7 deletions(-)\n\ndiff --git a/builtin/blame.c b/builtin/blame.c\nindex f831e3a..64605f5 100644\n--- a/builtin/blame.c\n+++ b/builtin/blame.c\n@@ -91,10 +91,10 @@ struct origin {\n  * if the textconv driver exists.\n  * Return 1 if the conversion succeeds, 0 otherwise.\n  */\n-static int textconv_object(const char *path,\n-\t\t\t   const unsigned char *sha1,\n-\t\t\t   char **buf,\n-\t\t\t   size_t *buf_size)\n+int textconv_object(const char *path,\n+\t\t    const unsigned char *sha1,\n+\t\t    char **buf,\n+\t\t    size_t *buf_size)\n {\n \tstruct diff_filespec *df;\n \tstruct userdiff_driver *textconv;\ndiff --git a/builtin/cat-file.c b/builtin/cat-file.c\nindex a933eaa..1457340 100644\n--- a/builtin/cat-file.c\n+++ b/builtin/cat-file.c\n@@ -9,6 +9,7 @@\n #include \"tree.h\"\n #include \"builtin.h\"\n #include \"parse-options.h\"\n+#include \"diff.h\"\n \n #define BATCH 1\n #define BATCH_CHECK 2\n@@ -86,8 +87,9 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name)\n \tenum object_type type;\n \tvoid *buf;\n \tunsigned long size;\n+\tstruct object_context obj_context = OBJECT_CONTEXT_INIT;\n \n-\tif (get_sha1(obj_name, sha1))\n+\tif (get_sha1_with_context(obj_name, sha1, &obj_context))\n \t\tdie(\"Not a valid object name %s\", obj_name);\n \n \tbuf = NULL;\n@@ -132,6 +134,17 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name)\n \n \t\t/* otherwise just spit out the data */\n \t\tbreak;\n+\n+\tcase 'c':\n+\t\tif (!obj_context.path)\n+\t\t\tdie(\"git cat-file --textconv %s: <object> must be <sha1:path>\",\n+\t\t\t    obj_name);\n+\n+\t\tif(!textconv_object(obj_context.path, sha1, &buf, (size_t *) &size))\n+\t\t\tdie(\"git cat-file --textconv: unable to run textconv on %s\",\n+\t\t\t    obj_name);\n+\t\tbreak;\n+\n \tcase 0:\n \t\tbuf = read_object_with_reference(sha1, exp_type, &size, NULL);\n \t\tbreak;\n@@ -201,11 +214,22 @@ static int batch_objects(int print_contents)\n }\n \n static const char * const cat_file_usage[] = {\n-\t\"git cat-file (-t|-s|-e|-p|<type>) <object>\",\n+\t\"git cat-file (-t|-s|-e|-p|<type>|--textconv) <object>\",\n \t\"git cat-file (--batch|--batch-check) < <list_of_objects>\",\n \tNULL\n };\n \n+static int git_cat_file_config(const char *var, const char *value, void *cb)\n+{\n+\tswitch (userdiff_config(var, value)) {\n+\t\tcase 0: break;\n+\t\tcase -1: return -1;\n+\t\tdefault: return 0;\n+\t}\n+\n+\treturn git_default_config(var, value, cb);\n+}\n+\n int cmd_cat_file(int argc, const char **argv, const char *prefix)\n {\n \tint opt = 0, batch = 0;\n@@ -218,6 +242,8 @@ int cmd_cat_file(int argc, const char **argv, const char *prefix)\n \t\tOPT_SET_INT('e', NULL, &opt,\n \t\t\t    \"exit with zero when there's no error\", 'e'),\n \t\tOPT_SET_INT('p', NULL, &opt, \"pretty-print object's content\", 'p'),\n+\t\tOPT_SET_INT(0, \"textconv\", &opt,\n+\t\t\t\t\"for blob objects, run textconv on object's content\", 'c'),\n \t\tOPT_SET_INT(0, \"batch\", &batch,\n \t\t\t    \"show info and content of objects fed from the standard input\",\n \t\t\t    BATCH),\n@@ -227,7 +253,7 @@ int cmd_cat_file(int argc, const char **argv, const char *prefix)\n \t\tOPT_END()\n \t};\n \n-\tgit_config(git_default_config, NULL);\n+\tgit_config(git_cat_file_config, NULL);\n \n \tif (argc != 3 && argc != 2)\n \t\tusage_with_options(cat_file_usage, options);\n-- \n1.7.1.202.g79415.dirty\n"},{"id":"143235","messageId":"1276004958-13540-4-git-send-email-clement.poulain@ensimag.imag.fr","threadId":"24043","inReplyTo":"1276004958-13540-3-git-send-email-clement.poulain@ensimag.imag.fr","subject":"[PATCH v2 3/4] git gui: use textconv filter for diff and blame","fromName":"Clément Poulain","fromEmail":"clement.poulain@ensimag.imag.fr","sentAt":"2010-06-08T13:49:17Z","receivedAt":"2010-06-08T13:49:17Z","isPatch":true,"sender":{"key":"clement.poulain@ensimag.imag.fr","avatar":null},"body":"Create a checkbox \"Use Textconv For Diffs and Blame\" in git-gui options.\nIf checked and if the driver for the concerned file exists, git-gui calls diff\nand blame with --textconv option\n\nSigned-off-by: Clément Poulain <clement.poulain@ensimag.imag.fr>\nSigned-off-by: Diane Gasselin <diane.gasselin@ensimag.imag.fr> \nSigned-off-by: Axel Bonnet <axel.bonnet@ensimag.imag.fr> \n---\n git-gui/git-gui.sh     |   28 +++++++++++++++++++++++++++-\n git-gui/lib/blame.tcl  |   21 +++++++++++++++++++--\n git-gui/lib/diff.tcl   |    5 ++++-\n git-gui/lib/option.tcl |    1 +\n 4 files changed, 51 insertions(+), 4 deletions(-)\n\ndiff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh\nindex 7d54511..59edf39 100755\n--- a/git-gui/git-gui.sh\n+++ b/git-gui/git-gui.sh\n@@ -269,6 +269,17 @@ proc is_config_true {name} {\n \t}\n }\n \n+proc is_config_false {name} {\n+\tglobal repo_config\n+\tif {[catch {set v $repo_config($name)}]} {\n+\t\treturn 0\n+\t} elseif {$v eq {false} || $v eq {0} || $v eq {no}} {\n+\t\treturn 1\n+\t} else {\n+\t\treturn 0\n+\t}\n+}\n+\n proc get_config {name} {\n \tglobal repo_config\n \tif {[catch {set v $repo_config($name)}]} {\n@@ -782,6 +793,7 @@ set default_config(user.email) {}\n \n set default_config(gui.encoding) [encoding system]\n set default_config(gui.matchtrackingbranch) false\n+set default_config(gui.textconv) true\n set default_config(gui.pruneduringfetch) false\n set default_config(gui.trustmtime) false\n set default_config(gui.fastcopyblame) false\n@@ -3405,6 +3417,19 @@ lappend diff_actions [list $ctxmsm entryconf [$ctxmsm index last] -state]\n $ctxmsm add separator\n create_common_diff_popup $ctxmsm\n \n+proc has_textconv {path} {\n+\tif {[is_config_false gui.textconv]} {\n+\t\treturn 0\n+\t}\n+\tset filter [gitattr $path diff set]\n+\tset textconv [get_config [join [list diff $filter textconv] .]]\n+\tif {$filter ne {set} && $textconv ne {}} {\n+\t\treturn 1\n+\t} else {\n+\t\treturn 0\n+\t}\n+}\n+\n proc popup_diff_menu {ctxm ctxmmg ctxmsm x y X Y} {\n \tglobal current_diff_path file_states\n \tset ::cursorX $x\n@@ -3440,7 +3465,8 @@ proc popup_diff_menu {ctxm ctxmmg ctxmsm x y X Y} {\n \t\t\t|| {__} eq $state\n \t\t\t|| {_O} eq $state\n \t\t\t|| {_T} eq $state\n-\t\t\t|| {T_} eq $state} {\n+\t\t\t|| {T_} eq $state\n+\t\t\t|| [has_textconv $current_diff_path]} {\n \t\t\tset s disabled\n \t\t} else {\n \t\t\tset s normal\ndiff --git a/git-gui/lib/blame.tcl b/git-gui/lib/blame.tcl\nindex 786b50b..b0f2f23 100644\n--- a/git-gui/lib/blame.tcl\n+++ b/git-gui/lib/blame.tcl\n@@ -449,11 +449,28 @@ method _load {jump} {\n \n \t$status show [mc \"Reading %s...\" \"$commit:[escape_path $path]\"]\n \t$w_path conf -text [escape_path $path]\n+\n+\tset do_textconv 0\n+\tif {![is_config_false gui.textconv]} {\n+\t\tset filter [gitattr $path diff set]\n+\t\tset textconv [get_config [join [list diff $filter textconv] .]]\n+\t\tif {$filter ne {set} && $textconv ne {}} {\n+\t\t\tset do_textconv 1\n+\t\t}\n+\t}\n \tif {$commit eq {}} {\n-\t\tset fd [open $path r]\n+\t\tif {$do_textconv ne 0} {\n+\t\t\tset fd [open \"|$textconv $path\" r]\n+\t\t} else {\n+\t\t\tset fd [open $path r]\n+\t\t}\n \t\tfconfigure $fd -eofchar {}\n \t} else {\n-\t\tset fd [git_read cat-file blob \"$commit:$path\"]\n+\t\tif {$do_textconv ne 0} {\n+\t\t\tset fd [git_read cat-file --textconv \"$commit:$path\"]\n+\t\t} else {\n+\t\t\tset fd [git_read cat-file blob \"$commit:$path\"]\n+\t\t}\n \t}\n \tfconfigure $fd \\\n \t\t-blocking 0 \\\ndiff --git a/git-gui/lib/diff.tcl b/git-gui/lib/diff.tcl\nindex ec8c11e..c628750 100644\n--- a/git-gui/lib/diff.tcl\n+++ b/git-gui/lib/diff.tcl\n@@ -55,7 +55,7 @@ proc handle_empty_diff {} {\n \n \tset path $current_diff_path\n \tset s $file_states($path)\n-\tif {[lindex $s 0] ne {_M}} return\n+\tif {[lindex $s 0] ne {_M} || [has_textconv $path]} return\n \n \t# Prevent infinite rescan loops\n \tincr diff_empty_count\n@@ -280,6 +280,9 @@ proc start_show_diff {cont_info {add_opts {}}} {\n \t\t\tlappend cmd diff-files\n \t\t}\n \t}\n+\tif {![is_config_false gui.textconv] && [git-version >= 1.6.1]} {\n+\t\tlappend cmd --textconv\n+\t}\n \n \tif {[string match {160000 *} [lindex $s 2]]\n \t || [string match {160000 *} [lindex $s 3]]} {\ndiff --git a/git-gui/lib/option.tcl b/git-gui/lib/option.tcl\nindex d4c5e45..3807c8d 100644\n--- a/git-gui/lib/option.tcl\n+++ b/git-gui/lib/option.tcl\n@@ -148,6 +148,7 @@ proc do_options {} {\n \t\t{b gui.trustmtime  {mc \"Trust File Modification Timestamps\"}}\n \t\t{b gui.pruneduringfetch {mc \"Prune Tracking Branches During Fetch\"}}\n \t\t{b gui.matchtrackingbranch {mc \"Match Tracking Branches\"}}\n+\t\t{b gui.textconv {mc \"Use Textconv For Diffs and Blames\"}}\n \t\t{b gui.fastcopyblame {mc \"Blame Copy Only On Changed Files\"}}\n \t\t{i-20..200 gui.copyblamethreshold {mc \"Minimum Letters To Blame Copy On\"}}\n \t\t{i-0..300 gui.blamehistoryctx {mc \"Blame History Context Radius (days)\"}}\n-- \n1.7.1.202.g79415.dirty\n"},{"id":"143233","messageId":"1276004958-13540-5-git-send-email-clement.poulain@ensimag.imag.fr","threadId":"24043","inReplyTo":"1276004958-13540-4-git-send-email-clement.poulain@ensimag.imag.fr","subject":"[PATCH v2 4/4] t/t8007: test textconv support for cat-file","fromName":"Clément Poulain","fromEmail":"clement.poulain@ensimag.imag.fr","sentAt":"2010-06-08T13:49:18Z","receivedAt":"2010-06-08T13:49:18Z","isPatch":true,"sender":{"key":"clement.poulain@ensimag.imag.fr","avatar":null},"body":"Test the correct functionning of textconv with cat-file <sha1:blob>\nand cat-file HEAD^ <file>. Test the case when no driver is specified\n\nSigned-off-by: Clément Poulain <clement.poulain@ensimag.imag.fr>\nSigned-off-by: Diane Gasselin <diane.gasselin@ensimag.imag.fr> \nSigned-off-by: Axel Bonnet <axel.bonnet@ensimag.imag.fr> \n---\n t/t8007-cat-file-textconv.sh |   70 ++++++++++++++++++++++++++++++++++++++++++\n 1 files changed, 70 insertions(+), 0 deletions(-)\n create mode 100755 t/t8007-cat-file-textconv.sh\n\ndiff --git a/t/t8007-cat-file-textconv.sh b/t/t8007-cat-file-textconv.sh\nnew file mode 100755\nindex 0000000..38ac05e\n--- /dev/null\n+++ b/t/t8007-cat-file-textconv.sh\n@@ -0,0 +1,70 @@\n+#!/bin/sh\n+\n+test_description='git cat-file textconv support'\n+. ./test-lib.sh\n+\n+cat >helper <<'EOF'\n+#!/bin/sh\n+sed 's/^/converted: /' \"$@\"\n+EOF\n+chmod +x helper\n+\n+test_expect_success 'setup ' '\n+\techo test >one.bin &&\n+\tgit add . &&\n+\tGIT_AUTHOR_NAME=Number1 git commit -a -m First --date=\"2010-01-01 18:00:00\" &&\n+\techo test version 2 >one.bin &&\n+\tGIT_AUTHOR_NAME=Number2 git commit -a -m Second --date=\"2010-01-01 20:00:00\"\n+'\n+\n+cat >expected <<EOF\n+fatal: git cat-file --textconv: unable to run textconv on :one.bin\n+EOF\n+\n+test_expect_success 'no filter specified' '\n+\tgit cat-file --textconv :one.bin 2>result\n+\ttest_cmp expected result\n+'\n+\n+test_expect_success 'setup textconv filters' '\n+\techo \"*.bin diff=test\" >.gitattributes &&\n+\tgit config diff.test.textconv ./helper &&\n+\tgit config diff.test.cachetextconv false\n+'\n+\n+cat >expected <<EOF\n+test version 2\n+EOF\n+\n+test_expect_success 'cat-file without --textconv' '\n+\tgit cat-file blob :one.bin >result &&\n+\ttest_cmp expected result\n+'\n+\n+cat >expected <<EOF\n+test\n+EOF\n+\n+test_expect_success 'cat-file without --textconv on previous commit' '\n+\tgit cat-file -p HEAD^:one.bin >result &&\n+\ttest_cmp expected result\n+'\n+\n+cat >expected <<EOF\n+converted: test version 2\n+EOF\n+\n+test_expect_success 'cat-file --textconv on last commit' '\n+\tgit cat-file --textconv :one.bin >result &&\n+\ttest_cmp expected result\n+'\n+\n+cat >expected <<EOF\n+converted: test\n+EOF\n+\n+test_expect_success 'cat-file --textconv on previous commit' '\n+\tgit cat-file --textconv HEAD^:one.bin >result &&\n+\ttest_cmp expected result\n+'\n+test_done\n-- \n1.7.1.202.g79415.dirty\n"},{"id":"143259","messageId":"vpqiq5t5rvd.fsf@bauges.imag.fr","threadId":"24043","inReplyTo":"1276004958-13540-2-git-send-email-clement.poulain@ensimag.imag.fr","subject":"Re: [PATCH v2 1/4] sha1_name: add get_sha1_with_context()","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2010-06-08T17:57:58Z","receivedAt":"2010-06-08T17:57:58Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"This patch produces uncompilable code for me:\n\ncc1: warnings being treated as errors\nIn file included from builtin.h:6,\n                 from fast-import.c:147:\ncache.h: In function ‘get_sha1_with_context’:\ncache.h:748: error: implicit declaration of function ‘get_sha1_with_context_1’\n\nForgot to add get_sha1_with_context_1 to cache.h?\n\nClément Poulain <clement.poulain@ensimag.imag.fr> writes:\n\n> +struct object_context {\n> +\tunsigned char tree[20];\n> +\tchar path[PATH_MAX];\n> +\tunsigned mode;\n> +};\n> +#define OBJECT_CONTEXT_INIT  { 0, 0, 0 }\n> +\n\nI'm not an expert in struct initializers, but after doing experiments\nwith GCC, this raises a warning\n\nbuiltin/cat-file.c:90: error: missing braces around initializer\nbuiltin/cat-file.c:90: error: (near initialization for ‘obj_context.tree’)\n\nand the behavior is to flatten the arrays contained inside the\nstructure. So, your OBJECT_CONTEXT_INIT initializes the 3 first bytes\nof tree to 0, and leaves other fields uninitialized.\n\nYou probably want something like this instead if you want to\ninitialize the whole struct:\n\n{{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, \n  0, 0, 0, 0, 0, 0, 0, 0, 0, 0}, \"\", 0}\n\n> --- a/sha1_name.c\n> +++ b/sha1_name.c\n> @@ -933,8 +933,8 @@ int interpret_branch_name(const char *name, struct strbuf *buf)\n>   */\n>  int get_sha1(const char *name, unsigned char *sha1)\n>  {\n> -\tunsigned unused;\n> -\treturn get_sha1_with_mode(name, sha1, &unused);\n> +\tstruct object_context unused;\n> +\treturn get_sha1_with_context(name, sha1, &unused);\n>  }\n\nThis changes doesn't seem harmful, but it doesn't seem useful to me\neither: get_sha1_with_mode still exists, right?\n\n>  int get_sha1_with_mode_1(const char *name, unsigned char *sha1, unsigned *mode, int gently, const char *prefix)\n>  {\n> +\tstruct object_context orc;\n\nWhat does orc stand for? I understand \"oc\" for \"object context\", but\nI'm curious about the r ;-).\n\n> +\t\torc->path[sizeof(orc->path)] = '\\0';\n> +\n\nIsn't this an off-by-one? The last element of an array of size N is\narray[N-1] ...\n\n> +\t\t\torc->path[sizeof(orc->path)] = '\\0';\n\nSame here.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"143261","messageId":"vpq7hm95r7h.fsf@bauges.imag.fr","threadId":"24043","inReplyTo":"1276004958-13540-3-git-send-email-clement.poulain@ensimag.imag.fr","subject":"Re: [PATCH v2 2/4] textconv: support for cat_file","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2010-06-08T18:12:18Z","receivedAt":"2010-06-08T18:12:18Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Clément Poulain <clement.poulain@ensimag.imag.fr> writes:\n\n> --- a/builtin/cat-file.c\n> +++ b/builtin/cat-file.c\n> @@ -9,6 +9,7 @@\n> +\tstruct object_context obj_context = OBJECT_CONTEXT_INIT;\n>  \n> -\tif (get_sha1(obj_name, sha1))\n> +\tif (get_sha1_with_context(obj_name, sha1, &obj_context))\n\nDo you really need to initialize obj_context here? I'd say the\nsemantics of get_sha1_with_context should be \"give me a pointer to an\nobject_context, and I'll fill it in with the object context, whatever\nbe its initial value\", just like\n\nint i;\nscanf(\"%d\", &i);\n\ndoesn't require i to be initialized.\n\n> +\tcase 'c':\n> +\t\tif (!obj_context.path)\n> +\t\t\tdie(\"git cat-file --textconv %s: <object> must be <sha1:path>\",\n> +\t\t\t    obj_name);\n\nobj_context.path is an array contained in the struct. It is always\nnon-null. Just tried:\n\n$ ./git cat-file --textconv 99f036302a7e6d884369d1d3f4ce428e437cbccd | head\nfatal: git cat-file --textconv: unable to run textconv on 99f036302a7e6d884369d1d3f4ce428e437cbccd\n\nyou want to check that obj_context.path contains an empty string.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"143292","messageId":"AANLkTinI_ghLE5U3tQ0JFmvuU8DySLFtdl03sv0uW-Ab@mail.gmail.com","threadId":"24043","inReplyTo":"vpqiq5t5rvd.fsf@bauges.imag.fr","subject":"Re: [PATCH v2 1/4] sha1_name: add get_sha1_with_context()","fromName":"Clément Poulain","fromEmail":"clement.poulain@ensimag.imag.fr","sentAt":"2010-06-08T22:30:31Z","receivedAt":"2010-06-08T22:30:31Z","isPatch":true,"sender":{"key":"clement.poulain@ensimag.imag.fr","avatar":null},"body":"Le 8 juin 2010 19:57, Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> a écrit :\n> This patch produces uncompilable code for me:\n>\n> cc1: warnings being treated as errors\n> In file included from builtin.h:6,\n>                 from fast-import.c:147:\n> cache.h: In function ‘get_sha1_with_context’:\n> cache.h:748: error: implicit declaration of function ‘get_sha1_with_context_1’\n>\n> Forgot to add get_sha1_with_context_1 to cache.h?\n\nUh, we compiled it almost ten times on both our pc and ensibm (our\nschool server), whithout any problems. Seems that we need to check our\ncompilation configurations.\n\n> I'm not an expert in struct initializers, but after doing experiments\n> with GCC, this raises a warning\n>\n> builtin/cat-file.c:90: error: missing braces around initializer\n> builtin/cat-file.c:90: error: (near initialization for ‘obj_context.tree’)\n>\n> and the behavior is to flatten the arrays contained inside the\n> structure. So, your OBJECT_CONTEXT_INIT initializes the 3 first bytes\n> of tree to 0, and leaves other fields uninitialized.\n>\n> You probably want something like this instead if you want to\n> initialize the whole struct:\n>\n> {{0, 0, 0, 0, 0, 0, 0, 0, 0, 0,\n>  0, 0, 0, 0, 0, 0, 0, 0, 0, 0}, \"\", 0}\n\nAs you pointed out in your second answer, initialization is maybe no\nrequired, we have to check it tomorrow.\nOtherwise, an easy way to do it can be something like :\nvoid object_context_init(struct object_context *oc)\n{\n\tmemset(oc, 0, sizeof(*oc));\n}\n\n>> --- a/sha1_name.c\n>> +++ b/sha1_name.c\n>> @@ -933,8 +933,8 @@ int interpret_branch_name(const char *name, struct strbuf *buf)\n>>   */\n>>  int get_sha1(const char *name, unsigned char *sha1)\n>>  {\n>> -     unsigned unused;\n>> -     return get_sha1_with_mode(name, sha1, &unused);\n>> +     struct object_context unused;\n>> +     return get_sha1_with_context(name, sha1, &unused);\n>>  }\n>\n> This changes doesn't seem harmful, but it doesn't seem useful to me\n> either: get_sha1_with_mode still exists, right?\n\nRight. But the aim was to skip one function call (see the call-stack below)\n_with_mode => _with_mode_1 => _with_context_1\nwhereas:\n _with_context => _with_context_1\n\n> What does orc stand for? I understand \"oc\" for \"object context\", but\n> I'm curious about the r ;-).\n\n\"orc\" was for \"object resolve context\". This is an artifact of our\nprevious version. We'll change it, it won't bother you no more ;-)\n\n>> +             orc->path[sizeof(orc->path)] = '\\0';\n>> +\n>\n> Isn't this an off-by-one? The last element of an array of size N is\n> array[N-1] ...\n>\n>> +                     orc->path[sizeof(orc->path)] = '\\0';\n>\n> Same here.\n\nThat's true. Stupid error, we copied this line without checking it.\n"},{"id":"143305","messageId":"20100609061337.GA14007@coredump.intra.peff.net","threadId":"24043","inReplyTo":"AANLkTinI_ghLE5U3tQ0JFmvuU8DySLFtdl03sv0uW-Ab@mail.gmail.com","subject":"Re: [PATCH v2 1/4] sha1_name: add get_sha1_with_context()","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2010-06-09T06:13:37Z","receivedAt":"2010-06-09T06:13:37Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Jun 09, 2010 at 12:30:31AM +0200, Clément Poulain wrote:\n\n> Le 8 juin 2010 19:57, Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> a écrit :\n> > This patch produces uncompilable code for me:\n> >\n> > cc1: warnings being treated as errors\n> > In file included from builtin.h:6,\n> >                 from fast-import.c:147:\n> > cache.h: In function ‘get_sha1_with_context’:\n> > cache.h:748: error: implicit declaration of function ‘get_sha1_with_context_1’\n> >\n> > Forgot to add get_sha1_with_context_1 to cache.h?\n> \n> Uh, we compiled it almost ten times on both our pc and ensibm (our\n> school server), whithout any problems. Seems that we need to check our\n> compilation configurations.\n\nNote the \"warnings being treated as errors\". Matthieu is compiling with\n-Werror (and presumably -Wall). We strive to be warning-free in git, and\nI think many of the developers compile with \"-Wall -Werror\".\n\n> Right. But the aim was to skip one function call (see the call-stack below)\n> _with_mode => _with_mode_1 => _with_context_1\n> whereas:\n>  _with_context => _with_context_1\n\nPerhaps that was your goal, but my goal when I suggested it was to give\nus a cleaner codebase. We don't want a proliferation of get_sha1_with_*\nfunctions. Introducing _with_context instead of _with_tree or _with_path\nwas meant not to make things worse. But collapsing _with_mode into\n_with_context actively makes things better.\n\n> >> +                     orc->path[sizeof(orc->path)] = '\\0';\n> >\n> > Same here.\n> \n> That's true. Stupid error, we copied this line without checking it.\n\nOops, that's my fault for introducing the bug in the first place (I had\noriginally had an snprintf and changed it to strncpy at the last\nminute). :)\n\n-Peff\n"},{"id":"143312","messageId":"vpqhblcvf3y.fsf@bauges.imag.fr","threadId":"24043","inReplyTo":"20100609061337.GA14007@coredump.intra.peff.net","subject":"Re: [PATCH v2 1/4] sha1_name: add get_sha1_with_context()","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2010-06-09T07:29:05Z","receivedAt":"2010-06-09T07:29:05Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Jeff King <peff@peff.net> writes:\n\n> Note the \"warnings being treated as errors\". Matthieu is compiling with\n> -Werror (and presumably -Wall). We strive to be warning-free in git, and\n> I think many of the developers compile with \"-Wall -Werror\".\n\nRight. Try this:\n\necho 'CFLAGS = -g -Wall -Werror' > config.mak\nmake\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"}]}