threads / patch / 33773

patchSupport running an arbitrary git action through checkout

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

## tl;dr

4 messages between May 10, 2013 and May 10, 2013. Diffs are folded; open one to read it.

replies: 3people: 4as markdown or json

Thomas Rast· May 10, 2013, 15:06 UTC · lore

[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(+)
Show changes to builtin/checkout.c +21 −0
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
Junio C Hamano· May 10, 2013, 15:52 UTC · re: Thomas Rast · lore

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

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.

Ramkumar Ramachandra· May 10, 2013, 16:35 UTC · re: Thomas Rast · lore

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

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.
Jeff King· May 10, 2013, 17:00 UTC · re: Thomas Rast · lore

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

On Fri, May 10, 2013 at 05:06:07PM +0200, Thomas Rast wrote:
Show 15 quoted lines
> +	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

← back to recent threads