{"thread":{"id":"33773","subject":"[PATCH] Support running an arbitrary git action through checkout","startedAt":"2013-05-10T15:06:07Z","lastAt":"2013-05-10T17:00:17Z","messageCount":4,"participants":["Thomas Rast","Junio C Hamano","Ramkumar Ramachandra","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"216894","messageId":"2d5cfb3be9487f607051cad3d5230434660307ba.1368198269.git.trast@inf.ethz.ch","threadId":"33773","inReplyTo":null,"subject":"[PATCH] Support running an arbitrary git action through checkout","fromName":"Thomas Rast","fromEmail":"trast@inf.ethz.ch","sentAt":"2013-05-10T15:06:07Z","receivedAt":"2013-05-10T15:06:07Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"[1] correctly observed that we are already wrapping three different\noperations under the git-checkout command. To lead that design -- and\nthe Koan -- to the obvious conclusion, some additional work is\nrequired.\n\nWith this patch, you can say\n\n git checkout --reset foo # reset HEAD to foo\n git checkout --bisect start # begin a bisection\n git checkout --rebase master # rebase the current branch on master\n\nand so on for any git command.\n\nNote that this actually shadows the long form of the existing --merge\noption. But since all reasonable Git users are extremely lazy typers,\nthey will just use the short form (-m) and this change is not expected\nto cause them any problems.\n\n[1] http://stevelosh.com/blog/2013/04/git-koans/\n\nCc: Steve Losh \nCc: Jeff King \nSigned-off-by: Thomas Rast \n---\n builtin/checkout.c | 21 +++++++++++++++++++++\n 1 file changed, 21 insertions(+)\n\ndiff --git a/builtin/checkout.c b/builtin/checkout.c\nindex f5b50e5..17419a2 100644\n--- a/builtin/checkout.c\n+++ b/builtin/checkout.c\n@@ -20,6 +20,8 @@\n #include \"resolve-undo.h\"\n #include \"submodule.h\"\n #include \"argv-array.h\"\n+#include \"help.h\"\n+#include \"exec_cmd.h\"\n \n static const char * const checkout_usage[] = {\n \tN_(\"git checkout [options] \"),\n@@ -1071,6 +1073,25 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)\n \t\tOPT_END(),\n \t};\n \n+\tif (argc > 1 && !prefixcmp(argv[1], \"--\")) {\n+\t\tconst char *subcommand = argv[1] + 2;\n+\t\tstruct cmdnames main_cmds, other_cmds;\n+\n+\t\tmemset(&main_cmds, 0, sizeof(main_cmds));\n+\t\tmemset(&other_cmds, 0, sizeof(other_cmds));\n+\n+\t\tload_command_list(\"git-\", &main_cmds, &other_cmds);\n+\n+\t\tif (is_in_cmdlist(&main_cmds, subcommand) ||\n+\t\t is_in_cmdlist(&other_cmds, subcommand)) {\n+\t\t\tconst char **args = xmalloc((argc) * sizeof(char*));\n+\t\t\targs[0] = subcommand;\n+\t\t\tmemcpy(args+1, argv+2, argc*sizeof(char*));\n+\t\t\targs[argc] = NULL;\n+\t\t\texecv_git_cmd(args);\n+\t\t}\n+\t}\n+\n \tmemset(&opts, 0, sizeof(opts));\n \tmemset(&new, 0, sizeof(new));\n \topts.overwrite_ignore = 1;\n-- \n1.8.3.rc1.425.g49e5819\n"},{"id":"216909","messageId":"7vk3n63k8f.fsf@alter.siamese.dyndns.org","threadId":"33773","inReplyTo":"2d5cfb3be9487f607051cad3d5230434660307ba.1368198269.git.trast@inf.ethz.ch","subject":"Re: [PATCH] Support running an arbitrary git action through checkout","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-05-10T15:52:48Z","receivedAt":"2013-05-10T15:52:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thomas Rast writes:\n\n> [1] correctly observed that we are already wrapping three different\n> operations under the git-checkout command.\n\nIt is not April 1st, but perhaps it is in Berlin ;-)\n\nBy the way I do not necessarily think that different operations\n\"checking out specific paths\" and \"checking out another branch\"\nshould be invoked by separate commands. It of course would be\neasier to _implement_ because the command does not have to see which\noperation the user wants, but if humans use the same verb to express\nthese actions, the command should help them do so.\n"},{"id":"216911","messageId":"CALkWK0kES+b3A64-nm+4aerBiEBn-cHKkHXb-Vhtp4JdMP9COQ@mail.gmail.com","threadId":"33773","inReplyTo":"2d5cfb3be9487f607051cad3d5230434660307ba.1368198269.git.trast@inf.ethz.ch","subject":"Re: [PATCH] Support running an arbitrary git action through checkout","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-05-10T16:35:48Z","receivedAt":"2013-05-10T16:35:48Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Thomas Rast wrote:\n> [1] correctly observed that we are already wrapping three different\n> operations under the git-checkout command.\n\nIncorrect. We merge_trees() and update refs. I see no reason to\ncreate artificial abstractions on top of this.\n\n> [...]\n\nLet's not talk about how gross your implementation is.\n"},{"id":"216914","messageId":"20130510170016.GA29848@sigill.intra.peff.net","threadId":"33773","inReplyTo":"2d5cfb3be9487f607051cad3d5230434660307ba.1368198269.git.trast@inf.ethz.ch","subject":"Re: [PATCH] Support running an arbitrary git action through checkout","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-05-10T17:00:17Z","receivedAt":"2013-05-10T17:00:17Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, May 10, 2013 at 05:06:07PM +0200, Thomas Rast wrote:\n\n> +\tif (argc > 1 && !prefixcmp(argv[1], \"--\")) {\n> +\t\tconst char *subcommand = argv[1] + 2;\n> +\t\tstruct cmdnames main_cmds, other_cmds;\n> +\n> +\t\tmemset(&main_cmds, 0, sizeof(main_cmds));\n> +\t\tmemset(&other_cmds, 0, sizeof(other_cmds));\n> +\n> +\t\tload_command_list(\"git-\", &main_cmds, &other_cmds);\n> +\n> +\t\tif (is_in_cmdlist(&main_cmds, subcommand) ||\n> +\t\t is_in_cmdlist(&other_cmds, subcommand)) {\n> +\t\t\tconst char **args = xmalloc((argc) * sizeof(char*));\n> +\t\t\targs[0] = subcommand;\n> +\t\t\tmemcpy(args+1, argv+2, argc*sizeof(char*));\n> +\t\t\targs[argc] = NULL;\n\nDoesn't this memcpy overflow args, since it is only argc slots long? I\nthink you want to copy only (argc-1) slots, since you are omitting both\nargv[0] as well as the \"--subcommand\" argument in argv[1]. You can also\ndrop the setting of NULL, as you will copy the original NULL as the last\nitem in your memcpy.\n\nOther than that, the patch looks brilliant. :)\n\n-Peff\n"}]}