{"thread":{"id":"36682","subject":"[PATCH v2 00/10] replace: add option to edit a Git object","startedAt":"2014-05-17T06:41:22Z","lastAt":"2014-05-17T07:24:51Z","messageCount":16,"participants":["Christian Couder","Jeff King"],"isPatch":true,"patchVersion":2,"patchTotal":10},"messages":[{"id":"242022","messageId":"20140517062418.18932.21200.chriscool@tuxfamily.org","threadId":"36682","inReplyTo":null,"subject":"[PATCH v2 00/10] replace: add option to edit a Git object","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2014-05-17T06:41:22Z","receivedAt":"2014-05-17T06:41:22Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"This patch series comes from what Peff sent in the following thread:\n\nhttp://thread.gmane.org/gmane.comp.version-control.git/243361/focus=243528\n\nThe first 4 patches (1/4, 2/4, 3/4 and 4/4) didn't change since v1.\nI added 6 more small patches to add tests, documentation and a few\nsmall improvements. \n\nChristian Couder (6):\n  replace: make sure --edit results in a different object\n  replace: refactor checking ref validity\n  replace: die early if replace ref already exists\n  replace: add tests for --edit\n  replace: add --edit to usage string\n  Documentation: replace: describe new --edit option\n\nJeff King (4):\n  replace: refactor command-mode determination\n  replace: use OPT_CMDMODE to handle modes\n  replace: factor object resolution out of replace_object\n  replace: add --edit option\n\n Documentation/git-replace.txt |  15 ++-\n builtin/replace.c             | 225 +++++++++++++++++++++++++++++++++---------\n t/t6050-replace.sh            |  29 ++++++\n 3 files changed, 223 insertions(+), 46 deletions(-)\n\n-- \n1.9.rc0.17.g651113e\n"},{"id":"242032","messageId":"20140517064133.18932.23762.chriscool@tuxfamily.org","threadId":"36682","inReplyTo":"20140517062418.18932.21200.chriscool@tuxfamily.org","subject":"[PATCH v2 01/10] replace: refactor command-mode determination","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2014-05-17T06:41:23Z","receivedAt":"2014-05-17T06:41:23Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"From: Jeff King <peff@peff.net>\n\nThe git-replace command has three modes: listing, deleting,\nand replacing. The first two are selected explicitly. If\nnone is selected, we fallback to listing when there are no\narguments, and replacing otherwise.\n\nLet's figure out up front which operation we are going to\ndo, before getting into the application logic. That lets us\nsimplify our option checks (e.g., we currently have to check\nwhether a useless \"--force\" is given both along with an\nexplicit list, as well as with an implicit one).\n\nThis saves some lines, makes the logic easier to follow, and\nwill facilitate further cleanups.\n\nSigned-off-by: Jeff King <peff@peff.net>\nSigned-off-by: Christian Couder <chriscool@tuxfamily.org>\n---\n builtin/replace.c | 14 ++++++--------\n 1 file changed, 6 insertions(+), 8 deletions(-)\n\ndiff --git a/builtin/replace.c b/builtin/replace.c\nindex b62420a..28db96f 100644\n--- a/builtin/replace.c\n+++ b/builtin/replace.c\n@@ -182,12 +182,16 @@ int cmd_replace(int argc, const char **argv, const char *prefix)\n \n \targc = parse_options(argc, argv, prefix, options, git_replace_usage, 0);\n \n+\tif (!list && !delete)\n+\t\tif (!argc)\n+\t\t\tlist = 1;\n+\n \tif (list && delete)\n \t\tusage_msg_opt(\"-l and -d cannot be used together\",\n \t\t\t      git_replace_usage, options);\n \n-\tif (format && delete)\n-\t\tusage_msg_opt(\"--format and -d cannot be used together\",\n+\tif (format && !list)\n+\t\tusage_msg_opt(\"--format cannot be used when not listing\",\n \t\t\t      git_replace_usage, options);\n \n \tif (force && (list || delete))\n@@ -207,9 +211,6 @@ int cmd_replace(int argc, const char **argv, const char *prefix)\n \t\tif (argc != 2)\n \t\t\tusage_msg_opt(\"bad number of arguments\",\n \t\t\t\t      git_replace_usage, options);\n-\t\tif (format)\n-\t\t\tusage_msg_opt(\"--format cannot be used when not listing\",\n-\t\t\t\t      git_replace_usage, options);\n \t\treturn replace_object(argv[0], argv[1], force);\n \t}\n \n@@ -217,9 +218,6 @@ int cmd_replace(int argc, const char **argv, const char *prefix)\n \tif (argc > 1)\n \t\tusage_msg_opt(\"only one pattern can be given with -l\",\n \t\t\t      git_replace_usage, options);\n-\tif (force)\n-\t\tusage_msg_opt(\"-f needs some arguments\",\n-\t\t\t      git_replace_usage, options);\n \n \treturn list_replace_refs(argv[0], format);\n }\n-- \n1.9.rc0.17.g651113e\n"},{"id":"242031","messageId":"20140517064133.18932.65969.chriscool@tuxfamily.org","threadId":"36682","inReplyTo":"20140517062418.18932.21200.chriscool@tuxfamily.org","subject":"[PATCH v2 02/10] replace: use OPT_CMDMODE to handle modes","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2014-05-17T06:41:24Z","receivedAt":"2014-05-17T06:41:24Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"From: Jeff King <peff@peff.net>\n\nBy using OPT_CMDMODE, the mutual exclusion between modes is\ntaken care of for us. It also makes it easy for us to\nmaintain a single variable with the mode, which makes its\nintent more clear. We can use a single switch() to make sure\nwe have covered all of the modes.\n\nThis ends up breaking even in code size, but the win will be\nmuch bigger when we start adding more modes.\n\nSigned-off-by: Jeff King <peff@peff.net>\nSigned-off-by: Christian Couder <chriscool@tuxfamily.org>\n---\n builtin/replace.c | 49 +++++++++++++++++++++++++------------------------\n 1 file changed, 25 insertions(+), 24 deletions(-)\n\ndiff --git a/builtin/replace.c b/builtin/replace.c\nindex 28db96f..29cf699 100644\n--- a/builtin/replace.c\n+++ b/builtin/replace.c\n@@ -168,11 +168,17 @@ static int replace_object(const char *object_ref, const char *replace_ref,\n \n int cmd_replace(int argc, const char **argv, const char *prefix)\n {\n-\tint list = 0, delete = 0, force = 0;\n+\tint force = 0;\n \tconst char *format = NULL;\n+\tenum {\n+\t\tMODE_UNSPECIFIED = 0,\n+\t\tMODE_LIST,\n+\t\tMODE_DELETE,\n+\t\tMODE_REPLACE\n+\t} cmdmode = MODE_UNSPECIFIED;\n \tstruct option options[] = {\n-\t\tOPT_BOOL('l', \"list\", &list, N_(\"list replace refs\")),\n-\t\tOPT_BOOL('d', \"delete\", &delete, N_(\"delete replace refs\")),\n+\t\tOPT_CMDMODE('l', \"list\", &cmdmode, N_(\"list replace refs\"), MODE_LIST),\n+\t\tOPT_CMDMODE('d', \"delete\", &cmdmode, N_(\"delete replace refs\"), MODE_DELETE),\n \t\tOPT_BOOL('f', \"force\", &force, N_(\"replace the ref if it exists\")),\n \t\tOPT_STRING(0, \"format\", &format, N_(\"format\"), N_(\"use this format\")),\n \t\tOPT_END()\n@@ -182,42 +188,37 @@ int cmd_replace(int argc, const char **argv, const char *prefix)\n \n \targc = parse_options(argc, argv, prefix, options, git_replace_usage, 0);\n \n-\tif (!list && !delete)\n-\t\tif (!argc)\n-\t\t\tlist = 1;\n+\tif (!cmdmode)\n+\t\tcmdmode = argc ? MODE_REPLACE : MODE_LIST;\n \n-\tif (list && delete)\n-\t\tusage_msg_opt(\"-l and -d cannot be used together\",\n-\t\t\t      git_replace_usage, options);\n-\n-\tif (format && !list)\n+\tif (format && cmdmode != MODE_LIST)\n \t\tusage_msg_opt(\"--format cannot be used when not listing\",\n \t\t\t      git_replace_usage, options);\n \n-\tif (force && (list || delete))\n-\t\tusage_msg_opt(\"-f cannot be used with -d or -l\",\n+\tif (force && cmdmode != MODE_REPLACE)\n+\t\tusage_msg_opt(\"-f only makes sense when writing a replacement\",\n \t\t\t      git_replace_usage, options);\n \n-\t/* Delete refs */\n-\tif (delete) {\n+\tswitch (cmdmode) {\n+\tcase MODE_DELETE:\n \t\tif (argc < 1)\n \t\t\tusage_msg_opt(\"-d needs at least one argument\",\n \t\t\t\t      git_replace_usage, options);\n \t\treturn for_each_replace_name(argv, delete_replace_ref);\n-\t}\n \n-\t/* Replace object */\n-\tif (!list && argc) {\n+\tcase MODE_REPLACE:\n \t\tif (argc != 2)\n \t\t\tusage_msg_opt(\"bad number of arguments\",\n \t\t\t\t      git_replace_usage, options);\n \t\treturn replace_object(argv[0], argv[1], force);\n-\t}\n \n-\t/* List refs, even if \"list\" is not set */\n-\tif (argc > 1)\n-\t\tusage_msg_opt(\"only one pattern can be given with -l\",\n-\t\t\t      git_replace_usage, options);\n+\tcase MODE_LIST:\n+\t\tif (argc > 1)\n+\t\t\tusage_msg_opt(\"only one pattern can be given with -l\",\n+\t\t\t\t      git_replace_usage, options);\n+\t\treturn list_replace_refs(argv[0], format);\n \n-\treturn list_replace_refs(argv[0], format);\n+\tdefault:\n+\t\tdie(\"BUG: invalid cmdmode %d\", (int)cmdmode);\n+\t}\n }\n-- \n1.9.rc0.17.g651113e\n"},{"id":"242023","messageId":"20140517064133.18932.3267.chriscool@tuxfamily.org","threadId":"36682","inReplyTo":"20140517062418.18932.21200.chriscool@tuxfamily.org","subject":"[PATCH v2 03/10] replace: factor object resolution out of replace_object","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2014-05-17T06:41:25Z","receivedAt":"2014-05-17T06:41:25Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"From: Jeff King <peff@peff.net>\n\nAs we add new options that operate on objects before\nreplacing them, we'll want to be able to feed raw sha1s\nstraight into replace_object. Split replace_object into the\nobject-resolution part and the actual replacement.\n\nSigned-off-by: Jeff King <peff@peff.net>\nSigned-off-by: Christian Couder <chriscool@tuxfamily.org>\n---\n builtin/replace.c | 26 ++++++++++++++++++--------\n 1 file changed, 18 insertions(+), 8 deletions(-)\n\ndiff --git a/builtin/replace.c b/builtin/replace.c\nindex 29cf699..af40129 100644\n--- a/builtin/replace.c\n+++ b/builtin/replace.c\n@@ -123,19 +123,17 @@ static int delete_replace_ref(const char *name, const char *ref,\n \treturn 0;\n }\n \n-static int replace_object(const char *object_ref, const char *replace_ref,\n-\t\t\t  int force)\n+static int replace_object_sha1(const char *object_ref,\n+\t\t\t       unsigned char object[20],\n+\t\t\t       const char *replace_ref,\n+\t\t\t       unsigned char repl[20],\n+\t\t\t       int force)\n {\n-\tunsigned char object[20], prev[20], repl[20];\n+\tunsigned char prev[20];\n \tenum object_type obj_type, repl_type;\n \tchar ref[PATH_MAX];\n \tstruct ref_lock *lock;\n \n-\tif (get_sha1(object_ref, object))\n-\t\tdie(\"Failed to resolve '%s' as a valid ref.\", object_ref);\n-\tif (get_sha1(replace_ref, repl))\n-\t\tdie(\"Failed to resolve '%s' as a valid ref.\", replace_ref);\n-\n \tif (snprintf(ref, sizeof(ref),\n \t\t     \"refs/replace/%s\",\n \t\t     sha1_to_hex(object)) > sizeof(ref) - 1)\n@@ -166,6 +164,18 @@ static int replace_object(const char *object_ref, const char *replace_ref,\n \treturn 0;\n }\n \n+static int replace_object(const char *object_ref, const char *replace_ref, int force)\n+{\n+\tunsigned char object[20], repl[20];\n+\n+\tif (get_sha1(object_ref, object))\n+\t\tdie(\"Failed to resolve '%s' as a valid ref.\", object_ref);\n+\tif (get_sha1(replace_ref, repl))\n+\t\tdie(\"Failed to resolve '%s' as a valid ref.\", replace_ref);\n+\n+\treturn replace_object_sha1(object_ref, object, replace_ref, repl, force);\n+}\n+\n int cmd_replace(int argc, const char **argv, const char *prefix)\n {\n \tint force = 0;\n-- \n1.9.rc0.17.g651113e\n"},{"id":"242024","messageId":"20140517064133.18932.44214.chriscool@tuxfamily.org","threadId":"36682","inReplyTo":"20140517062418.18932.21200.chriscool@tuxfamily.org","subject":"[PATCH v2 04/10] replace: add --edit option","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2014-05-17T06:41:26Z","receivedAt":"2014-05-17T06:41:26Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"From: Jeff King <peff@peff.net>\n\nThis allows you to run:\n\n    git replace --edit SHA1\n\nto get dumped in an editor with the contents of the object\nfor SHA1. The result is then read back in and used as a\n\"replace\" object for SHA1. The writing/reading is\ntype-aware, so you get to edit \"ls-tree\" output rather than\nthe binary tree format.\n\nMissing documentation and tests.\n\nSigned-off-by: Jeff King <peff@peff.net>\nSigned-off-by: Christian Couder <chriscool@tuxfamily.org>\n---\n builtin/replace.c | 112 +++++++++++++++++++++++++++++++++++++++++++++++++++++-\n 1 file changed, 111 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/replace.c b/builtin/replace.c\nindex af40129..3da1bae 100644\n--- a/builtin/replace.c\n+++ b/builtin/replace.c\n@@ -12,6 +12,7 @@\n #include \"builtin.h\"\n #include \"refs.h\"\n #include \"parse-options.h\"\n+#include \"run-command.h\"\n \n static const char * const git_replace_usage[] = {\n \tN_(\"git replace [-f] <object> <replacement>\"),\n@@ -176,6 +177,107 @@ static int replace_object(const char *object_ref, const char *replace_ref, int f\n \treturn replace_object_sha1(object_ref, object, replace_ref, repl, force);\n }\n \n+/*\n+ * Write the contents of the object named by \"sha1\" to the file \"filename\",\n+ * pretty-printed for human editing based on its type.\n+ */\n+static void export_object(const unsigned char *sha1, const char *filename)\n+{\n+\tconst char *argv[] = { \"--no-replace-objects\", \"cat-file\", \"-p\", NULL, NULL };\n+\tstruct child_process cmd = { argv };\n+\tint fd;\n+\n+\tfd = open(filename, O_WRONLY | O_CREAT | O_TRUNC, 0666);\n+\tif (fd < 0)\n+\t\tdie_errno(\"unable to open %s for writing\", filename);\n+\n+\targv[3] = sha1_to_hex(sha1);\n+\tcmd.git_cmd = 1;\n+\tcmd.out = fd;\n+\n+\tif (run_command(&cmd))\n+\t\tdie(\"cat-file reported failure\");\n+\n+\tclose(fd);\n+}\n+\n+/*\n+ * Read a previously-exported (and possibly edited) object back from \"filename\",\n+ * interpreting it as \"type\", and writing the result to the object database.\n+ * The sha1 of the written object is returned via sha1.\n+ */\n+static void import_object(unsigned char *sha1, enum object_type type,\n+\t\t\t  const char *filename)\n+{\n+\tint fd;\n+\n+\tfd = open(filename, O_RDONLY);\n+\tif (fd < 0)\n+\t\tdie_errno(\"unable to open %s for reading\", filename);\n+\n+\tif (type == OBJ_TREE) {\n+\t\tconst char *argv[] = { \"mktree\", NULL };\n+\t\tstruct child_process cmd = { argv };\n+\t\tstruct strbuf result = STRBUF_INIT;\n+\n+\t\tcmd.argv = argv;\n+\t\tcmd.git_cmd = 1;\n+\t\tcmd.in = fd;\n+\t\tcmd.out = -1;\n+\n+\t\tif (start_command(&cmd))\n+\t\t\tdie(\"unable to spawn mktree\");\n+\n+\t\tif (strbuf_read(&result, cmd.out, 41) < 0)\n+\t\t\tdie_errno(\"unable to read from mktree\");\n+\t\tclose(cmd.out);\n+\n+\t\tif (finish_command(&cmd))\n+\t\t\tdie(\"mktree reported failure\");\n+\t\tif (get_sha1_hex(result.buf, sha1) < 0)\n+\t\t\tdie(\"mktree did not return an object name\");\n+\n+\t\tstrbuf_release(&result);\n+\t} else {\n+\t\tstruct stat st;\n+\t\tint flags = HASH_FORMAT_CHECK | HASH_WRITE_OBJECT;\n+\n+\t\tif (fstat(fd, &st) < 0)\n+\t\t\tdie_errno(\"unable to fstat %s\", filename);\n+\t\tif (index_fd(sha1, fd, &st, type, NULL, flags) < 0)\n+\t\t\tdie(\"unable to write object to database\");\n+\t\t/* index_fd close()s fd for us */\n+\t}\n+\n+\t/*\n+\t * No need to close(fd) here; both run-command and index-fd\n+\t * will have done it for us.\n+\t */\n+}\n+\n+static int edit_and_replace(const char *object_ref, int force)\n+{\n+\tchar *tmpfile = git_pathdup(\"REPLACE_EDITOBJ\");\n+\tenum object_type type;\n+\tunsigned char old[20], new[20];\n+\n+\tif (get_sha1(object_ref, old) < 0)\n+\t\tdie(\"Not a valid object name: '%s'\", object_ref);\n+\n+\ttype = sha1_object_info(old, NULL);\n+\tif (type < 0)\n+\t\tdie(\"unable to get object type for %s\", sha1_to_hex(old));\n+\n+\texport_object(old, tmpfile);\n+\tif (launch_editor(tmpfile, NULL, NULL) < 0)\n+\t\tdie(\"editing object file failed\");\n+\timport_object(new, type, tmpfile);\n+\n+\tfree(tmpfile);\n+\n+\treturn replace_object_sha1(object_ref, old, \"replacement\", new, force);\n+}\n+\n int cmd_replace(int argc, const char **argv, const char *prefix)\n {\n \tint force = 0;\n@@ -184,11 +286,13 @@ int cmd_replace(int argc, const char **argv, const char *prefix)\n \t\tMODE_UNSPECIFIED = 0,\n \t\tMODE_LIST,\n \t\tMODE_DELETE,\n+\t\tMODE_EDIT,\n \t\tMODE_REPLACE\n \t} cmdmode = MODE_UNSPECIFIED;\n \tstruct option options[] = {\n \t\tOPT_CMDMODE('l', \"list\", &cmdmode, N_(\"list replace refs\"), MODE_LIST),\n \t\tOPT_CMDMODE('d', \"delete\", &cmdmode, N_(\"delete replace refs\"), MODE_DELETE),\n+\t\tOPT_CMDMODE('e', \"edit\", &cmdmode, N_(\"edit existing object\"), MODE_EDIT),\n \t\tOPT_BOOL('f', \"force\", &force, N_(\"replace the ref if it exists\")),\n \t\tOPT_STRING(0, \"format\", &format, N_(\"format\"), N_(\"use this format\")),\n \t\tOPT_END()\n@@ -205,7 +309,7 @@ int cmd_replace(int argc, const char **argv, const char *prefix)\n \t\tusage_msg_opt(\"--format cannot be used when not listing\",\n \t\t\t      git_replace_usage, options);\n \n-\tif (force && cmdmode != MODE_REPLACE)\n+\tif (force && cmdmode != MODE_REPLACE && cmdmode != MODE_EDIT)\n \t\tusage_msg_opt(\"-f only makes sense when writing a replacement\",\n \t\t\t      git_replace_usage, options);\n \n@@ -222,6 +326,12 @@ int cmd_replace(int argc, const char **argv, const char *prefix)\n \t\t\t\t      git_replace_usage, options);\n \t\treturn replace_object(argv[0], argv[1], force);\n \n+\tcase MODE_EDIT:\n+\t\tif (argc != 1)\n+\t\t\tusage_msg_opt(\"-e needs exactly one argument\",\n+\t\t\t\t      git_replace_usage, options);\n+\t\treturn edit_and_replace(argv[0], force);\n+\n \tcase MODE_LIST:\n \t\tif (argc > 1)\n \t\t\tusage_msg_opt(\"only one pattern can be given with -l\",\n-- \n1.9.rc0.17.g651113e\n"},{"id":"242026","messageId":"20140517064133.18932.27058.chriscool@tuxfamily.org","threadId":"36682","inReplyTo":"20140517062418.18932.21200.chriscool@tuxfamily.org","subject":"[PATCH v2 05/10] replace: make sure --edit results in a different object","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2014-05-17T06:41:27Z","receivedAt":"2014-05-17T06:41:27Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"It's a bad idea to create a replace ref for an object\nthat points to the original object itself.\n\nThat's why we have to check if the result from editing\nthe original object is a different object and error out\nif it isn't.\n\nSigned-off-by: Christian Couder <chriscool@tuxfamily.org>\n---\n builtin/replace.c | 3 +++\n 1 file changed, 3 insertions(+)\n\ndiff --git a/builtin/replace.c b/builtin/replace.c\nindex 3da1bae..0751804 100644\n--- a/builtin/replace.c\n+++ b/builtin/replace.c\n@@ -275,6 +275,9 @@ static int edit_and_replace(const char *object_ref, int force)\n \n \tfree(tmpfile);\n \n+\tif (!hashcmp(old, new))\n+\t\treturn error(\"new object is the same as the old one: '%s'\", sha1_to_hex(old));\n+\n \treturn replace_object_sha1(object_ref, old, \"replacement\", new, force);\n }\n \n-- \n1.9.rc0.17.g651113e\n"},{"id":"242029","messageId":"20140517064133.18932.16751.chriscool@tuxfamily.org","threadId":"36682","inReplyTo":"20140517062418.18932.21200.chriscool@tuxfamily.org","subject":"[PATCH v2 06/10] replace: refactor checking ref validity","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2014-05-17T06:41:28Z","receivedAt":"2014-05-17T06:41:28Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"This will be useful in a following commit when we will\nwant to check if the ref already exists before we let the\nuser edit an object.\n\nSigned-off-by: Christian Couder <chriscool@tuxfamily.org>\n---\n builtin/replace.c | 31 ++++++++++++++++++++-----------\n 1 file changed, 20 insertions(+), 11 deletions(-)\n\ndiff --git a/builtin/replace.c b/builtin/replace.c\nindex 0751804..3d6edaf 100644\n--- a/builtin/replace.c\n+++ b/builtin/replace.c\n@@ -124,6 +124,25 @@ static int delete_replace_ref(const char *name, const char *ref,\n \treturn 0;\n }\n \n+static void check_ref_valid(unsigned char object[20],\n+\t\t\t    unsigned char prev[20],\n+\t\t\t    char *ref,\n+\t\t\t    int ref_size,\n+\t\t\t    int force)\n+{\n+\tif (snprintf(ref, ref_size,\n+\t\t     \"refs/replace/%s\",\n+\t\t     sha1_to_hex(object)) > ref_size - 1)\n+\t\tdie(\"replace ref name too long: %.*s...\", 50, ref);\n+\tif (check_refname_format(ref, 0))\n+\t\tdie(\"'%s' is not a valid ref name.\", ref);\n+\n+\tif (read_ref(ref, prev))\n+\t\thashclr(prev);\n+\telse if (!force)\n+\t\tdie(\"replace ref '%s' already exists\", ref);\n+}\n+\n static int replace_object_sha1(const char *object_ref,\n \t\t\t       unsigned char object[20],\n \t\t\t       const char *replace_ref,\n@@ -135,13 +154,6 @@ static int replace_object_sha1(const char *object_ref,\n \tchar ref[PATH_MAX];\n \tstruct ref_lock *lock;\n \n-\tif (snprintf(ref, sizeof(ref),\n-\t\t     \"refs/replace/%s\",\n-\t\t     sha1_to_hex(object)) > sizeof(ref) - 1)\n-\t\tdie(\"replace ref name too long: %.*s...\", 50, ref);\n-\tif (check_refname_format(ref, 0))\n-\t\tdie(\"'%s' is not a valid ref name.\", ref);\n-\n \tobj_type = sha1_object_info(object, NULL);\n \trepl_type = sha1_object_info(repl, NULL);\n \tif (!force && obj_type != repl_type)\n@@ -151,10 +163,7 @@ static int replace_object_sha1(const char *object_ref,\n \t\t    object_ref, typename(obj_type),\n \t\t    replace_ref, typename(repl_type));\n \n-\tif (read_ref(ref, prev))\n-\t\thashclr(prev);\n-\telse if (!force)\n-\t\tdie(\"replace ref '%s' already exists\", ref);\n+\tcheck_ref_valid(object, prev, ref, sizeof(ref), force);\n \n \tlock = lock_any_ref_for_update(ref, prev, 0, NULL);\n \tif (!lock)\n-- \n1.9.rc0.17.g651113e\n"},{"id":"242025","messageId":"20140517064133.18932.92343.chriscool@tuxfamily.org","threadId":"36682","inReplyTo":"20140517062418.18932.21200.chriscool@tuxfamily.org","subject":"[PATCH v2 07/10] replace: die early if replace ref already exists","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2014-05-17T06:41:29Z","receivedAt":"2014-05-17T06:41:29Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"If a replace ref already exists for an object, it is\nmuch better for the user if we error out before we\nlet the user edit the object, rather than after.\n\nSigned-off-by: Christian Couder <chriscool@tuxfamily.org>\n---\n builtin/replace.c | 5 ++++-\n 1 file changed, 4 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/replace.c b/builtin/replace.c\nindex 3d6edaf..4ee3d92 100644\n--- a/builtin/replace.c\n+++ b/builtin/replace.c\n@@ -268,7 +268,8 @@ static int edit_and_replace(const char *object_ref, int force)\n {\n \tchar *tmpfile = git_pathdup(\"REPLACE_EDITOBJ\");\n \tenum object_type type;\n-\tunsigned char old[20], new[20];\n+\tunsigned char old[20], new[20], prev[20];\n+\tchar ref[PATH_MAX];\n \n \tif (get_sha1(object_ref, old) < 0)\n \t\tdie(\"Not a valid object name: '%s'\", object_ref);\n@@ -277,6 +278,8 @@ static int edit_and_replace(const char *object_ref, int force)\n \tif (type < 0)\n \t\tdie(\"unable to get object type for %s\", sha1_to_hex(old));\n \n+\tcheck_ref_valid(old, prev, ref, sizeof(ref), force);\n+\n \texport_object(old, tmpfile);\n \tif (launch_editor(tmpfile, NULL, NULL) < 0)\n \t\tdie(\"editing object file failed\");\n-- \n1.9.rc0.17.g651113e\n"},{"id":"242027","messageId":"20140517064133.18932.74476.chriscool@tuxfamily.org","threadId":"36682","inReplyTo":"20140517062418.18932.21200.chriscool@tuxfamily.org","subject":"[PATCH v2 08/10] replace: add tests for --edit","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2014-05-17T06:41:30Z","receivedAt":"2014-05-17T06:41:30Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"Signed-off-by: Christian Couder <chriscool@tuxfamily.org>\n---\n t/t6050-replace.sh | 29 +++++++++++++++++++++++++++++\n 1 file changed, 29 insertions(+)\n\ndiff --git a/t/t6050-replace.sh b/t/t6050-replace.sh\nindex 719a116..7609174 100755\n--- a/t/t6050-replace.sh\n+++ b/t/t6050-replace.sh\n@@ -318,6 +318,35 @@ test_expect_success 'test --format long' '\n \ttest_cmp expected actual\n '\n \n+test_expect_success 'setup a fake editor' '\n+\tcat >fakeeditor <<-\\EOF &&\n+\t\t#!/bin/sh\n+\t\tsed -e \"s/A U Thor/A fake Thor/\" \"$1\" >\"$1.new\"\n+\t\tmv \"$1.new\" \"$1\"\n+\tEOF\n+\tchmod +x fakeeditor\n+'\n+\n+test_expect_success '--edit with and without already replaced object' '\n+\tGIT_EDITOR=./fakeeditor test_must_fail git replace --edit \"$PARA3\" &&\n+\tGIT_EDITOR=./fakeeditor git replace --force --edit \"$PARA3\" &&\n+\tgit replace -l | grep \"$PARA3\" &&\n+\tgit cat-file commit \"$PARA3\" | grep \"A fake Thor\" &&\n+\tgit replace -d \"$PARA3\" &&\n+\tGIT_EDITOR=./fakeeditor git replace --edit \"$PARA3\" &&\n+\tgit replace -l | grep \"$PARA3\" &&\n+\tgit cat-file commit \"$PARA3\" | grep \"A fake Thor\"\n+'\n+\n+test_expect_success '--edit and change nothing or command failed' '\n+\tgit replace -d \"$PARA3\" &&\n+\tGIT_EDITOR=true test_must_fail git replace --edit \"$PARA3\" &&\n+\tGIT_EDITOR=\"./fakeeditor;false\" test_must_fail git replace --edit \"$PARA3\" &&\n+\tGIT_EDITOR=./fakeeditor git replace --edit \"$PARA3\" &&\n+\tgit replace -l | grep \"$PARA3\" &&\n+\tgit cat-file commit \"$PARA3\" | grep \"A fake Thor\"\n+'\n+\n test_expect_success 'replace ref cleanup' '\n \ttest -n \"$(git replace)\" &&\n \tgit replace -d $(git replace) &&\n-- \n1.9.rc0.17.g651113e\n"},{"id":"242030","messageId":"20140517064133.18932.42280.chriscool@tuxfamily.org","threadId":"36682","inReplyTo":"20140517062418.18932.21200.chriscool@tuxfamily.org","subject":"[PATCH v2 09/10] replace: add --edit to usage string","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2014-05-17T06:41:31Z","receivedAt":"2014-05-17T06:41:31Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"Signed-off-by: Christian Couder <chriscool@tuxfamily.org>\n---\n builtin/replace.c | 1 +\n 1 file changed, 1 insertion(+)\n\ndiff --git a/builtin/replace.c b/builtin/replace.c\nindex 4ee3d92..1bb491d 100644\n--- a/builtin/replace.c\n+++ b/builtin/replace.c\n@@ -16,6 +16,7 @@\n \n static const char * const git_replace_usage[] = {\n \tN_(\"git replace [-f] <object> <replacement>\"),\n+\tN_(\"git replace [-f] --edit <object>\"),\n \tN_(\"git replace -d <object>...\"),\n \tN_(\"git replace [--format=<format>] [-l [<pattern>]]\"),\n \tNULL\n-- \n1.9.rc0.17.g651113e\n"},{"id":"242028","messageId":"20140517064133.18932.96622.chriscool@tuxfamily.org","threadId":"36682","inReplyTo":"20140517062418.18932.21200.chriscool@tuxfamily.org","subject":"[PATCH v2 10/10] Documentation: replace: describe new --edit option","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2014-05-17T06:41:32Z","receivedAt":"2014-05-17T06:41:32Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"Signed-off-by: Christian Couder <chriscool@tuxfamily.org>\n---\n Documentation/git-replace.txt | 15 ++++++++++++++-\n 1 file changed, 14 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/git-replace.txt b/Documentation/git-replace.txt\nindex 0a02f70..37d872d 100644\n--- a/Documentation/git-replace.txt\n+++ b/Documentation/git-replace.txt\n@@ -9,6 +9,7 @@ SYNOPSIS\n --------\n [verse]\n 'git replace' [-f] <object> <replacement>\n+'git replace' [-f] --edit <object>\n 'git replace' -d <object>...\n 'git replace' [--format=<format>] [-l [<pattern>]]\n \n@@ -63,6 +64,14 @@ OPTIONS\n --delete::\n \tDelete existing replace refs for the given objects.\n \n+--edit <object>::\n+\tLaunch an editor to let the user change the content of\n+\t<object>, then create a new object of the same type with the\n+\tchanged content, and create a replace ref to replace <object>\n+\twith the new object. See linkgit:git-commit[1] and\n+\tlinkgit:git-var[1] for details about how the editor will be\n+\tchosen.\n+\n -l <pattern>::\n --list <pattern>::\n \tList replace refs for objects that match the given pattern (or\n@@ -92,7 +101,9 @@ CREATING REPLACEMENT OBJECTS\n \n linkgit:git-filter-branch[1], linkgit:git-hash-object[1] and\n linkgit:git-rebase[1], among other git commands, can be used to create\n-replacement objects from existing objects.\n+replacement objects from existing objects. The `--edit` option can\n+also be used with 'git replace' to create a replacement object by\n+editing an existing object.\n \n If you want to replace many blobs, trees or commits that are part of a\n string of commits, you may just want to create a replacement string of\n@@ -117,6 +128,8 @@ linkgit:git-filter-branch[1]\n linkgit:git-rebase[1]\n linkgit:git-tag[1]\n linkgit:git-branch[1]\n+linkgit:git-commit[1]\n+linkgit:git-var[1]\n linkgit:git[1]\n \n GIT\n-- \n1.9.rc0.17.g651113e\n"},{"id":"242034","messageId":"20140517070339.GC13003@sigill.intra.peff.net","threadId":"36682","inReplyTo":"20140517064133.18932.27058.chriscool@tuxfamily.org","subject":"Re: [PATCH v2 05/10] replace: make sure --edit results in a different object","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-05-17T07:03:40Z","receivedAt":"2014-05-17T07:03:40Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, May 17, 2014 at 08:41:27AM +0200, Christian Couder wrote:\n\n> It's a bad idea to create a replace ref for an object\n> that points to the original object itself.\n> \n> That's why we have to check if the result from editing\n> the original object is a different object and error out\n> if it isn't.\n\nI think that's reasonable.\n\nAnother option, which I think I mentioned earlier, would be to delete\nany existing replacement ref, and return success. So you could use that\nto \"revert\" a replaced object back to its original non-replaced form.\n\nOn a similar note, we might want to consider what happens when you\n\"--edit\" an object which already has a replacement. Right now you end up\nediting the _original_ object. I wonder if it would make sense to start\nthe editor with the contents of the replacement object (in which case\nyou might even \"revert\" without realizing it).\n\nBut those can easily come later if somebody feels like working on them.\nErroring out is not that bad an outcome, since the user can then \"git\nreplace -d\" themselves if they want to.\n\n-Peff\n"},{"id":"242035","messageId":"20140517070518.GD13003@sigill.intra.peff.net","threadId":"36682","inReplyTo":"20140517064133.18932.92343.chriscool@tuxfamily.org","subject":"Re: [PATCH v2 07/10] replace: die early if replace ref already exists","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-05-17T07:05:18Z","receivedAt":"2014-05-17T07:05:18Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, May 17, 2014 at 08:41:29AM +0200, Christian Couder wrote:\n\n> If a replace ref already exists for an object, it is\n> much better for the user if we error out before we\n> let the user edit the object, rather than after.\n\nDefinitely. Code looks fine to me.\n\n-Peff\n"},{"id":"242036","messageId":"20140517071410.GE13003@sigill.intra.peff.net","threadId":"36682","inReplyTo":"20140517064133.18932.74476.chriscool@tuxfamily.org","subject":"Re: [PATCH v2 08/10] replace: add tests for --edit","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-05-17T07:14:10Z","receivedAt":"2014-05-17T07:14:10Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, May 17, 2014 at 08:41:30AM +0200, Christian Couder wrote:\n\n> Signed-off-by: Christian Couder <chriscool@tuxfamily.org>\n> ---\n>  t/t6050-replace.sh | 29 +++++++++++++++++++++++++++++\n>  1 file changed, 29 insertions(+)\n\nYay, tests.\n\n> +test_expect_success 'setup a fake editor' '\n> +\tcat >fakeeditor <<-\\EOF &&\n> +\t\t#!/bin/sh\n> +\t\tsed -e \"s/A U Thor/A fake Thor/\" \"$1\" >\"$1.new\"\n> +\t\tmv \"$1.new\" \"$1\"\n> +\tEOF\n> +\tchmod +x fakeeditor\n> +'\n\nPlease use write_script, like:\n\n  test_expect_success 'setup a fake editor' '\n\twrite_script fakeeditor <<-\\EOF\n\t\tsed -e \"s/A U Thor/A fake Thor/\" \"$1\" >\"$1.new\"\n\t\tmv \"$1.new\" \"$1\"\n\tEOF\n  '\n\nwhich will use the $(SHELL_PATH) shebang. It doesn't matter for such a\nsimple script here (which even a broken #!/bin/sh could manage), but in\ngeneral, I think we're trying to set it consistently.\n\nYou could also &&-chain the commands in your fakeeditor script, though I\nfind it unlikely that sed will fail.\n\n> +test_expect_success '--edit with and without already replaced object' '\n> +\tGIT_EDITOR=./fakeeditor test_must_fail git replace --edit \"$PARA3\" &&\n\nUnfortunately it's not portable to do a one-shot environment variable on\na shell function like this; some shells leave the variable set after the\nfunction returns. We've been using:\n\n  test_must_fail env GIT_EDITOR=./fakeeditor git ...\n\nThe regular\n\n  GIT_EDITOR=./fakeeditor git ...\n\nones are OK.\n\n-Peff\n"},{"id":"242037","messageId":"20140517072347.GF13003@sigill.intra.peff.net","threadId":"36682","inReplyTo":"20140517064133.18932.96622.chriscool@tuxfamily.org","subject":"Re: [PATCH v2 10/10] Documentation: replace: describe new --edit option","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-05-17T07:23:47Z","receivedAt":"2014-05-17T07:23:47Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, May 17, 2014 at 08:41:32AM +0200, Christian Couder wrote:\n\n> @@ -63,6 +64,14 @@ OPTIONS\n>  --delete::\n>  \tDelete existing replace refs for the given objects.\n>  \n> +--edit <object>::\n> +\tLaunch an editor to let the user change the content of\n> +\t<object>, then create a new object of the same type with the\n> +\tchanged content, and create a replace ref to replace <object>\n> +\twith the new object. See linkgit:git-commit[1] and\n> +\tlinkgit:git-var[1] for details about how the editor will be\n> +\tchosen.\n\nI found the first sentence a little hard to parse, and there are a few\nmore details that might be worth mentioning:\n\n  --edit <object>::\n    Edit an object's content interactively. The existing content for\n    <object> is pretty-printed into a temporary file, an editor is\n    launched on the file, and the result is parsed to create a new\n    object of the same type as <object>. A replacement ref is then\n    created to replace <object> with the newly created object.\n\nI do not know if it is worth mentioning git-commit and git-var here. But\nif we want to, I think git-var is sufficient (git-commit seems to mostly\njust points at git-var these days).\n\n-Peff\n"},{"id":"242038","messageId":"20140517072451.GG13003@sigill.intra.peff.net","threadId":"36682","inReplyTo":"20140517062418.18932.21200.chriscool@tuxfamily.org","subject":"Re: [PATCH v2 00/10] replace: add option to edit a Git object","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-05-17T07:24:51Z","receivedAt":"2014-05-17T07:24:51Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, May 17, 2014 at 08:41:22AM +0200, Christian Couder wrote:\n\n> This patch series comes from what Peff sent in the following thread:\n> \n> http://thread.gmane.org/gmane.comp.version-control.git/243361/focus=243528\n> \n> The first 4 patches (1/4, 2/4, 3/4 and 4/4) didn't change since v1.\n> I added 6 more small patches to add tests, documentation and a few\n> small improvements. \n\nThanks, I think these look pretty good. I made a few small comments in\nreply to the individual patches.\n\n-Peff\n"}]}