[PATCH] Support running an arbitrary git action through checkout

[PATCH] Support running an arbitrary git action through checkout

[Thomas Rast]

[1] correctly observed that we are already wrapping three different
operations under the git-checkout command.  To lead that design -- and
the Koan -- to the obvious conclusion, some additional work is
required.

With this patch, you can say

  git checkout --reset foo      # reset HEAD to foo
  git checkout --bisect start   # begin a bisection
  git checkout --rebase master  # rebase the current branch on master

and so on for any git command.

Note that this actually shadows the long form of the existing --merge
option.  But since all reasonable Git users are extremely lazy typers,
they will just use the short form (-m) and this change is not expected
to cause them any problems.

[1] http://stevelosh.com/blog/2013/04/git-koans/

Cc: Steve Losh <steve@stevelosh.com>
Cc: Jeff King <peff@peff.net>
Signed-off-by: Thomas Rast <trast@inf.ethz.ch>
---
 builtin/checkout.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/builtin/checkout.c b/builtin/checkout.c
index f5b50e5..17419a2 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -20,6 +20,8 @@
 #include "resolve-undo.h"
 #include "submodule.h"
 #include "argv-array.h"
+#include "help.h"
+#include "exec_cmd.h"
 
 static const char * const checkout_usage[] = {
 	N_("git checkout [options] <branch>"),
@@ -1071,6 +1073,25 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
 		OPT_END(),
 	};
 
+	if (argc > 1 && !prefixcmp(argv[1], "--")) {
+		const char *subcommand = argv[1] + 2;
+		struct cmdnames main_cmds, other_cmds;
+
+		memset(&main_cmds, 0, sizeof(main_cmds));
+		memset(&other_cmds, 0, sizeof(other_cmds));
+
+		load_command_list("git-", &main_cmds, &other_cmds);
+
+		if (is_in_cmdlist(&main_cmds, subcommand) ||
+		    is_in_cmdlist(&other_cmds, subcommand)) {
+			const char **args = xmalloc((argc) * sizeof(char*));
+			args[0] = subcommand;
+			memcpy(args+1, argv+2, argc*sizeof(char*));
+			args[argc] = NULL;
+			execv_git_cmd(args);
+		}
+	}
+
 	memset(&opts, 0, sizeof(opts));
 	memset(&new, 0, sizeof(new));
 	opts.overwrite_ignore = 1;
-- 
1.8.3.rc1.425.g49e5819

Re: [PATCH] Support running an arbitrary git action through checkout

[Junio C Hamano]

Thomas Rast <trast@inf.ethz.ch> writes:

> [1] correctly observed that we are already wrapping three different
> operations under the git-checkout command.

It is not April 1st, but perhaps it is in Berlin ;-)

By the way I do not necessarily think that different operations
"checking out specific paths" and "checking out another branch"
should be invoked by separate commands.  It of course would be
easier to _implement_ because the command does not have to see which
operation the user wants, but if humans use the same verb to express
these actions, the command should help them do so.

Re: [PATCH] Support running an arbitrary git action through checkout

[Ramkumar Ramachandra]

Thomas Rast wrote:
> [1] correctly observed that we are already wrapping three different
> operations under the git-checkout command.

Incorrect.  We merge_trees() and update refs.  I see no reason to
create artificial abstractions on top of this.

> [...]

Let's not talk about how gross your implementation is.

Re: [PATCH] Support running an arbitrary git action through checkout

[Jeff King]

On Fri, May 10, 2013 at 05:06:07PM +0200, Thomas Rast wrote:

> +	if (argc > 1 && !prefixcmp(argv[1], "--")) {
> +		const char *subcommand = argv[1] + 2;
> +		struct cmdnames main_cmds, other_cmds;
> +
> +		memset(&main_cmds, 0, sizeof(main_cmds));
> +		memset(&other_cmds, 0, sizeof(other_cmds));
> +
> +		load_command_list("git-", &main_cmds, &other_cmds);
> +
> +		if (is_in_cmdlist(&main_cmds, subcommand) ||
> +		    is_in_cmdlist(&other_cmds, subcommand)) {
> +			const char **args = xmalloc((argc) * sizeof(char*));
> +			args[0] = subcommand;
> +			memcpy(args+1, argv+2, argc*sizeof(char*));
> +			args[argc] = NULL;

Doesn't this memcpy overflow args, since it is only argc slots long? I
think you want to copy only (argc-1) slots, since you are omitting both
argv[0] as well as the "--subcommand" argument in argv[1]. You can also
drop the setting of NULL, as you will copy the original NULL as the last
item in your memcpy.

Other than that, the patch looks brilliant. :)

-Peff