Re: [PATCH] transport.c: call dash-less form of receive-pack and upload-pack on remote

Subsystems: the rest

11 messages, 2 authors, 2016-06-15 · open the first message on its own page

Re: [PATCH] transport.c: call dash-less form of receive-pack and upload-pack on remote

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:43:55

Johannes Schindelin [off-list ref] writes:

quoted

I only eyeball-tested it and looks Okay, but that does not assure us
much ;-).  Is this change easy to test using local transport?

Seems like it breaks down with git-shell.  But then, I think that we just 
have to fix execv_git_cmd() to call builtins via "git" instead.

So in execv_git_cmd(), instead of doing the strbuf_addf(), we do
something like this (and drop your patch)?

---
 exec_cmd.c |   34 +++++++++++++---------------------
 1 files changed, 13 insertions(+), 21 deletions(-)

diff --git a/exec_cmd.c b/exec_cmd.c
index 2d0a758..2920335 100644
--- a/exec_cmd.c
+++ b/exec_cmd.c

@@ -65,32 +65,24 @@ void setup_path(const char *cmd_path)
 
 int execv_git_cmd(const char **argv)
 {
-	struct strbuf cmd;
-	const char *tmp;
-
-	strbuf_init(&cmd, 0);
-	strbuf_addf(&cmd, "git-%s", argv[0]);
-
-	/*
-	 * argv[0] must be the git command, but the argv array
-	 * belongs to the caller, and may be reused in
-	 * subsequent loop iterations. Save argv[0] and
-	 * restore it on error.
-	 */
-	tmp = argv[0];
-	argv[0] = cmd.buf;
-
-	trace_argv_printf(argv, -1, "trace: exec:");
+	int i;
+	const char **args;
+
+	for (i = 0; argv[i]; i++)
+		;
+	args = xcalloc(i + 1, sizeof(*args));
+	for (i = 0; argv[i]; i++)
+		args[i+1] = argv[i];
+	args[0] = "git";
+	args[i+1] = NULL;
+	trace_argv_printf(args, -1, "trace: exec:");
 
 	/* execvp() can only ever return if it fails */
-	execvp(cmd.buf, (char **)argv);
+	execvp(args[0], (char **)args);
 
 	trace_printf("trace: exec failed: %s\n", strerror(errno));
 
-	argv[0] = tmp;
-
-	strbuf_release(&cmd);
-
+	free(args);
 	return -1;
 }

Re: [PATCH] transport.c: call dash-less form of receive-pack and upload-pack on remote

From: Johannes Schindelin <hidden>
Date: 2016-06-15 22:43:55

Hi,

On Sat, 1 Dec 2007, Junio C Hamano wrote:

Johannes Schindelin [off-list ref] writes:

quoted

I only eyeball-tested it and looks Okay, but that does not assure us
much ;-).  Is this change easy to test using local transport?

Seems like it breaks down with git-shell.  But then, I think that we just 
have to fix execv_git_cmd() to call builtins via "git" instead.

So in execv_git_cmd(), instead of doing the strbuf_addf(), we do
something like this (and drop your patch)?

You know what is really funny?  I have this in my stash:

-- snip --
 exec_cmd.c      |   30 ++++++++++++------------------
 t/t0020-crlf.sh |    2 +-
 2 files changed, 13 insertions(+), 19 deletions(-)

diff --git a/exec_cmd.c b/exec_cmd.c
index 2d0a758..7d022a2 100644
--- a/exec_cmd.c
+++ b/exec_cmd.c

@@ -65,31 +65,25 @@ void setup_path(const char *cmd_path)
 
 int execv_git_cmd(const char **argv)
 {
-	struct strbuf cmd;
-	const char *tmp;
+	int i;
+	char **new_argv;
 
-	strbuf_init(&cmd, 0);
-	strbuf_addf(&cmd, "git-%s", argv[0]);
+	for (i = 0; argv[i]; i++)
+		; /* do nothing */
+	new_argv = xmalloc((i + 2) * sizeof(*new_argv));
+	new_argv[0] = "git";
+	for (i = 0; argv[i]; i++)
+		new_argv[i + 1] = (char *)argv[i];
+	new_argv[i] = NULL;
 
-	/*
-	 * argv[0] must be the git command, but the argv array
-	 * belongs to the caller, and may be reused in
-	 * subsequent loop iterations. Save argv[0] and
-	 * restore it on error.
-	 */
-	tmp = argv[0];
-	argv[0] = cmd.buf;
-
-	trace_argv_printf(argv, -1, "trace: exec:");
+	trace_argv_printf((const char **)new_argv, -1, "trace: exec:");
 
 	/* execvp() can only ever return if it fails */
-	execvp(cmd.buf, (char **)argv);
+	execvp(new_argv[0], new_argv);
 
 	trace_printf("trace: exec failed: %s\n", strerror(errno));
 
-	argv[0] = tmp;
-
-	strbuf_release(&cmd);
+	free(new_argv);
 
 	return -1;
 }

diff --git a/t/t0020-crlf.sh b/t/t0020-crlf.sh
index 62bc4bb..275379d 100755
--- a/t/t0020-crlf.sh
+++ b/t/t0020-crlf.sh

@@ -36,7 +36,7 @@ test_expect_success setup '
 
 	for w in Some extra lines here; do echo $w; done >>one &&
 	git diff >patch.file &&
-	patched=`git hash-object --stdin <one` &&
+	patched=`GIT_TRACE=2 git hash-object --stdin <one` &&
 	git read-tree --reset -u HEAD &&
 
 	echo happy.
-- snap --

Which looks awfully like your patch (except that I called it new_argv, I 
think).

Now you might ask why there is such a funny patch to t0020?  Well, the 
patch does not work as-is.

Will investigate right now,
Dscho

Re: [PATCH] transport.c: call dash-less form of receive-pack and upload-pack on remote

From: Johannes Schindelin <hidden>
Date: 2016-06-15 22:43:55

Hi,

On Sat, 1 Dec 2007, Johannes Schindelin wrote:

Will investigate right now,

The problem is that "git <command>" will call execv_git_cmd() for 
non-builtins, which in turn will execute "git <command>", ... ad 
infinitum.

Ciao,
Dscho

[PATCH 0/3] Call builtin functions directly, was Re: [PATCH] transport.c: call dash-less form of receive-pack and upload-pack on remote

From: Johannes Schindelin <hidden>
Date: 2016-06-15 22:43:55

Hi,

On Sat, 1 Dec 2007, Johannes Schindelin wrote:

On Sat, 1 Dec 2007, Johannes Schindelin wrote:

quoted

Will investigate right now,

The problem is that "git <command>" will call execv_git_cmd() for 
non-builtins, which in turn will execute "git <command>", ... ad 
infinitum.

Okay, I bit the apple and tried to move the builtins into the library, and 
rename handle_internal_command into execv_git_builtin(), moving it into 
exec-cmd.c.

Big mistake.

Why?  Because there is at least one caller, git-bundle, which relies on 
execv_git_cmd() _not_ reusing all those "nice" one-shot static variables, 
like for example the object hashmap and the objects themselves.

Now, it seems that we can get away for the moment with just introducing an 
object release mechanism and calling that in execv_git_builtin() before 
calling a builtin function, because the existing callers do not rely on 
more than a cleanup of the objects.

But it is hairy, since it is such an essential part of git.  And since I 
was utterly tired while preparing this patch series.  So I suggest maybe 
putting this into pu, but no further for the moment.  I will use the 
patched git in the next days, though, to catch breakages (hopefully).

Ciao,
Dscho

[PATCH 1/3] Introduce release_all_objects()

From: Johannes Schindelin <hidden>
Date: 2016-06-15 22:43:55

The new function release_all_objects() can be used to flush the object
cache.  This will be needed for the upcoming change in execv_git_cmd(),
which should call the builtin functions directly instead of calling
execvp().

Signed-off-by: Johannes Schindelin <redacted>
---

	Guess how surprised I was when "free(commit);" would work the
	first time, but not the second...

	ATM I use an ugly way to cope with the static "nr" variables
	in alloc.c: I just do not free() the last block.

	It might be a better idea to refactor the (quite ugly) code in
	alloc.c to have global structures a la

		#define BLOCKING 1024

		struct object_block {
			size_t struct_size;
			int nr;
			/* first (uint32_t *) is pointer to previous block */
			void *block;
		};

		static void *alloc_node(struct object_block *block)
		{
			if (!block || block->nr >= BLOCKING) {
				void *next = xmalloc(sizeof(void *)
					+ BLOCKING * block->struct_size);
				*(void **)next = block->block;
				block->block = next;
				block->nr = 0;
			}
			return block->block + sizeof(void *)
				+ block->struct_size * block->nr++;
		}

		static void release_nodes(struct object_block *block)
		{
			while (block->block) {
				void *previous = *(void **)block->block;
				free(block->block);
				block->block = previous;
			}
			block->nr = 0; /* not strictly necessary */
		}

		#define DEFINE_ALLOCATOR(type)				\
		static object_block type##s = { sizeof(struct type) };	\
		struct type *alloc_##type##_node(void)			\
		{							\
			return alloc_node(&type##s);			\
		}

		DEFINE_ALLOCATOR(object)
		DEFINE_ALLOCATOR(blob)
		DEFINE_ALLOCATOR(tree)
		DEFINE_ALLOCATOR(commit)
		DEFINE_ALLOCATOR(tag)

	but I am waaay too tired to brush this up, test it and submit it
	(hint, hint).

 alloc.c  |   31 ++++++++++++++++++++++++++++++-
 blob.c   |    3 +++
 blob.h   |    1 +
 cache.h  |    6 ++++++
 commit.c |    8 ++++++++
 commit.h |    2 ++
 object.c |   24 ++++++++++++++++++++++++
 object.h |    2 ++
 tag.c    |    8 ++++++++
 tag.h    |    2 ++
 tree.c   |    6 ++++++
 tree.h   |    1 +
 12 files changed, 93 insertions(+), 1 deletions(-)

diff --git a/alloc.c b/alloc.c
index 216c23a..8c5e5e0 100644
--- a/alloc.c
+++ b/alloc.c

@@ -20,6 +20,7 @@
 
 #define DEFINE_ALLOCATOR(name, type)				\
 static unsigned int name##_allocs;				\
+static void *last_alloced_##name;				\
 void *alloc_##name##_node(void)					\
 {								\
 	static int nr;						\

@@ -28,13 +29,32 @@ void *alloc_##name##_node(void)					\
 								\
 	if (!nr) {						\
 		nr = BLOCKING;					\
-		block = xmalloc(BLOCKING * sizeof(type));	\
+		struct {					\
+			void *previous;				\
+			type block[BLOCKING];			\
+		} *buf = xmalloc(sizeof(*buf));			\
+		buf->previous = last_alloced_##name;		\
+		last_alloced_##name = buf;			\
+		block = buf->block;				\
 	}							\
 	nr--;							\
 	name##_allocs++;					\
 	ret = block++;						\
 	memset(ret, 0, sizeof(type));				\
 	return ret;						\
+}								\
+								\
+void release_all_##name##_nodes(void)				\
+{								\
+	void *buf = last_alloced_##name;			\
+	if (!buf)						\
+		return;						\
+	buf = *(void **)buf;					\
+	while (buf) {						\
+		void *next = *(void **)buf;			\
+		free(buf);					\
+		buf = next;					\
+	}							\
 }
 
 union any_object {

@@ -74,3 +94,12 @@ void alloc_report(void)
 	REPORT(commit);
 	REPORT(tag);
 }
+
+void release_all_nodes(void)
+{
+	release_all_blob_nodes();
+	release_all_tree_nodes();
+	release_all_commit_nodes();
+	release_all_tag_nodes();
+	release_all_object_nodes();
+}

diff --git a/blob.c b/blob.c
index bd7d078..63756e6 100644
--- a/blob.c
+++ b/blob.c

@@ -18,6 +18,9 @@ struct blob *lookup_blob(const unsigned char *sha1)
 	return (struct blob *) obj;
 }
 
+void release_blob(struct blob *blob) {
+}
+
 int parse_blob_buffer(struct blob *item, void *buffer, unsigned long size)
 {
 	item->object.parsed = 1;

diff --git a/blob.h b/blob.h
index ea5d9e9..7560671 100644
--- a/blob.h
+++ b/blob.h

@@ -10,6 +10,7 @@ struct blob {
 };
 
 struct blob *lookup_blob(const unsigned char *sha1);
+void release_blob(struct blob *blob);
 
 int parse_blob_buffer(struct blob *item, void *buffer, unsigned long size);

diff --git a/cache.h b/cache.h
index 4e59646..cc50f1c 100644
--- a/cache.h
+++ b/cache.h

@@ -613,6 +613,12 @@ extern void *alloc_tree_node(void);
 extern void *alloc_commit_node(void);
 extern void *alloc_tag_node(void);
 extern void *alloc_object_node(void);
+extern void release_all_blob_nodes(void);
+extern void release_all_tree_nodes(void);
+extern void release_all_commit_nodes(void);
+extern void release_all_tag_nodes(void);
+extern void release_all_object_nodes(void);
+extern void release_all_nodes(void);
 extern void alloc_report(void);
 
 /* trace.c */

diff --git a/commit.c b/commit.c
index f074811..59c2236 100644
--- a/commit.c
+++ b/commit.c

@@ -48,6 +48,14 @@ struct commit *lookup_commit(const unsigned char *sha1)
 	return check_commit(obj, sha1, 0);
 }
 
+void release_commit(struct commit *commit)
+{
+	if (commit->parents)
+		free_commit_list(commit->parents);
+	if (commit->buffer)
+		free(commit->buffer);
+}
+
 static unsigned long parse_commit_date(const char *buf)
 {
 	unsigned long date;

diff --git a/commit.h b/commit.h
index 10e2b5d..363b9fb 100644
--- a/commit.h
+++ b/commit.h

@@ -21,6 +21,7 @@ struct commit {
 	char *buffer;
 };
 
+
 extern int save_commit_buffer;
 extern const char *commit_type;

@@ -35,6 +36,7 @@ struct commit *lookup_commit(const unsigned char *sha1);
 struct commit *lookup_commit_reference(const unsigned char *sha1);
 struct commit *lookup_commit_reference_gently(const unsigned char *sha1,
 					      int quiet);
+void release_commit(struct commit *commit);
 
 int parse_commit_buffer(struct commit *item, void *buffer, unsigned long size);

diff --git a/object.c b/object.c
index 16793d9..f217122 100644
--- a/object.c
+++ b/object.c

@@ -192,6 +192,30 @@ struct object *parse_object(const unsigned char *sha1)
 	return NULL;
 }
 
+void release_all_objects(void)
+{
+	int i;
+	for (i = 0; i < obj_hash_size; i++)
+		if (obj_hash[i]) {
+			switch (obj_hash[i]->type) {
+			case OBJ_BLOB:
+				release_blob((struct blob *)obj_hash[i]);
+				break;
+			case OBJ_COMMIT:
+				release_commit((struct commit *)obj_hash[i]);
+				break;
+			/*case OBJ_TREE:
+				release_tree((struct tree *)obj_hash[i]);
+				break;
+			case OBJ_TAG:
+				release_tag((struct tag *)obj_hash[i]);
+				break;*/
+			}
+			obj_hash[i] = NULL;
+		}
+	release_all_nodes();
+}
+
 struct object_list *object_list_insert(struct object *item,
 				       struct object_list **list_p)
 {

diff --git a/object.h b/object.h
index 397bbfa..ad6184c 100644
--- a/object.h
+++ b/object.h

@@ -44,6 +44,8 @@ extern unsigned int get_max_object_index(void);
 extern struct object *get_indexed_object(unsigned int);
 extern struct object_refs *lookup_object_refs(struct object *);
 
+extern void release_all_objects(void);
+
 /** Internal only **/
 struct object *lookup_object(const unsigned char *sha1);

diff --git a/tag.c b/tag.c
index f62bcdd..8bc6840 100644
--- a/tag.c
+++ b/tag.c

@@ -33,6 +33,14 @@ struct tag *lookup_tag(const unsigned char *sha1)
         return (struct tag *) obj;
 }
 
+void release_tag(struct tag *tag)
+{
+	if (tag->tag)
+		free(tag->tag);
+	if (tag->signature)
+		free(tag->signature);
+}
+
 int parse_tag_buffer(struct tag *item, void *data, unsigned long size)
 {
 	int typelen, taglen;

diff --git a/tag.h b/tag.h
index 7a0cb00..fbc6048 100644
--- a/tag.h
+++ b/tag.h

@@ -12,6 +12,8 @@ struct tag {
 	char *signature; /* not actually implemented */
 };
 
+void release_tag(struct tag *tag);
+
 extern struct tag *lookup_tag(const unsigned char *sha1);
 extern int parse_tag_buffer(struct tag *item, void *data, unsigned long size);
 extern int parse_tag(struct tag *item);

diff --git a/tree.c b/tree.c
index 8c0819f..ee99bc6 100644
--- a/tree.c
+++ b/tree.c

@@ -202,6 +202,12 @@ struct tree *lookup_tree(const unsigned char *sha1)
 	return (struct tree *) obj;
 }
 
+void release_tree(struct tree *tree)
+{
+	if (tree->buffer)
+		free(tree->buffer);
+}
+
 /*
  * NOTE! Tree refs to external git repositories
  * (ie gitlinks) do not count as real references.

diff --git a/tree.h b/tree.h
index dd25c53..f8372a2 100644
--- a/tree.h
+++ b/tree.h

@@ -12,6 +12,7 @@ struct tree {
 };
 
 struct tree *lookup_tree(const unsigned char *sha1);
+void release_tree(struct tree *tree);
 
 int parse_tree_buffer(struct tree *item, void *buffer, unsigned long size);

-- 
1.5.3.6.2112.ge2263

[PATCH 2/3] Include the objects needed for the builtin functions into libgit.a

From: Johannes Schindelin <hidden>
Date: 2016-06-15 22:43:55

For the upcoming change in execv_git_cmd() to call builtin functions
directly, it is necessary to be able to access the builtins, so
move the corresponding objects into libgit.a.

Signed-off-by: Johannes Schindelin <redacted>
---
 Makefile |   14 +++++++++-----
 1 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/Makefile b/Makefile
index f000a5e..f9a62eb 100644
--- a/Makefile
+++ b/Makefile

@@ -314,7 +314,8 @@ LIB_OBJS = \
 	alloc.o merge-file.o path-list.o help.o unpack-trees.o $(DIFF_OBJS) \
 	color.o wt-status.o archive-zip.o archive-tar.o shallow.o utf8.o \
 	convert.o attr.o decorate.o progress.o mailmap.o symlinks.o remote.o \
-	transport.o bundle.o walker.o parse-options.o
+	transport.o bundle.o walker.o parse-options.o \
+	$(BUILTIN_OBJS)
 
 BUILTIN_OBJS = \
 	builtin-add.o \

@@ -785,12 +786,12 @@ strip: $(PROGRAMS) git$X
 	$(STRIP) $(STRIP_OPTS) $(PROGRAMS) git$X
 
 git.o: git.c common-cmds.h GIT-CFLAGS
-	$(QUIET_CC)$(CC) -DGIT_VERSION='"$(GIT_VERSION)"' \
+	$(QUIET_CC)$(CC) \
 		$(ALL_CFLAGS) -c $(filter %.c,$^)
 
 git$X: git.o $(BUILTIN_OBJS) $(GITLIBS)
 	$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ git.o \
-		$(BUILTIN_OBJS) $(ALL_LDFLAGS) $(LIBS)
+		$(ALL_LDFLAGS) $(LIBS)
 
 help.o: common-cmds.h

@@ -894,7 +895,10 @@ git.o git.spec \
 	$(QUIET_CC)$(CC) -o $*.o -c $(ALL_CFLAGS) $<
 
 exec_cmd.o: exec_cmd.c GIT-CFLAGS
-	$(QUIET_CC)$(CC) -o $*.o -c $(ALL_CFLAGS) '-DGIT_EXEC_PATH="$(gitexecdir_SQ)"' $<
+	$(QUIET_CC)$(CC) -o $*.o -c $(ALL_CFLAGS) \
+		-DGIT_EXEC_PATH='"$(gitexecdir_SQ)"' \
+		-DGIT_VERSION='"$(GIT_VERSION)"' \
+		$<
 builtin-init-db.o: builtin-init-db.c GIT-CFLAGS
 	$(QUIET_CC)$(CC) -o $*.o -c $(ALL_CFLAGS) -DDEFAULT_GIT_TEMPLATE_DIR='"$(template_dir_SQ)"' $<

@@ -920,7 +924,7 @@ git-http-push$X: revision.o http.o http-push.o $(GITLIBS)
 	$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \
 		$(LIBS) $(CURL_LIBCURL) $(EXPAT_LIBEXPAT)
 
-$(LIB_OBJS) $(BUILTIN_OBJS): $(LIB_H)
+$(LIB_OBJS): $(LIB_H)
 $(patsubst git-%$X,%.o,$(PROGRAMS)): $(LIB_H) $(wildcard */*.h)
 builtin-revert.o builtin-runstatus.o wt-status.o: wt-status.h

-- 
1.5.3.6.2112.ge2263

[PATCH 3/3] Introduce execv_git_builtin() and use it

From: Johannes Schindelin <hidden>
Date: 2016-06-15 22:43:55

You can call execv_git_builtin() to execute a builtin command.  This
function is a reborn handle_internal_command() from git.c, and has the
semantics of execv_git_cmd(), i.e. it exits with the exit code of the
builtin if there is a matching builtin, but it avoids the real
execvp() call.

This function calls release_all_objects() and discard_cache() to start
from a clean slate (this, along with the calculation of argc, is the
only difference from a straight code move).

The test suite passes, which at least does not contradict the
hypothesis that this is good enough.  However, it might be
necessary to de-initialize more global variables.

The function execv_git_cmd() and git.c's main() function were changed
to take advantage of execv_git_builtin().

Signed-off-by: Johannes Schindelin <redacted>
---
 exec_cmd.c |  178 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 exec_cmd.h |    1 +
 git.c      |  172 +---------------------------------------------------------
 3 files changed, 180 insertions(+), 171 deletions(-)

diff --git a/exec_cmd.c b/exec_cmd.c
index 2d0a758..ac21181 100644
--- a/exec_cmd.c
+++ b/exec_cmd.c

@@ -1,6 +1,8 @@
 #include "cache.h"
 #include "exec_cmd.h"
 #include "quote.h"
+#include "builtin.h"
+#include "object.h"
 #define MAX_ARGS	32
 
 extern char **environ;

@@ -63,11 +65,187 @@ void setup_path(const char *cmd_path)
 	strbuf_release(&new_path);
 }
 
+const char git_usage_string[] =
+	"git [--version] [--exec-path[=GIT_EXEC_PATH]] [-p|--paginate|--no-pager] [--bare] [--git-dir=GIT_DIR] [--work-tree=GIT_WORK_TREE] [--help] COMMAND [ARGS]";
+
+const char git_version_string[] = GIT_VERSION;
+
+#define RUN_SETUP	(1<<0)
+#define USE_PAGER	(1<<1)
+/*
+ * require working tree to be present -- anything uses this needs
+ * RUN_SETUP for reading from the configuration file.
+ */
+#define NEED_WORK_TREE	(1<<2)
+
+struct cmd_struct {
+	const char *cmd;
+	int (*fn)(int, const char **, const char *);
+	int option;
+};
+
+static int run_command(struct cmd_struct *p, int argc, const char **argv)
+{
+	int status;
+	struct stat st;
+	const char *prefix;
+
+	prefix = NULL;
+	if (p->option & RUN_SETUP)
+		prefix = setup_git_directory();
+	if (p->option & USE_PAGER)
+		setup_pager();
+	if (p->option & NEED_WORK_TREE)
+		setup_work_tree();
+
+	trace_argv_printf(argv, argc, "trace: built-in: git");
+
+	status = p->fn(argc, argv, prefix);
+	if (status)
+		return status & 0xff;
+
+	/* Somebody closed stdout? */
+	if (fstat(fileno(stdout), &st))
+		return 0;
+	/* Ignore write errors for pipes and sockets.. */
+	if (S_ISFIFO(st.st_mode) || S_ISSOCK(st.st_mode))
+		return 0;
+
+	/* Check for ENOSPC and EIO errors.. */
+	if (fflush(stdout))
+		die("write failure on standard output: %s", strerror(errno));
+	if (ferror(stdout))
+		die("unknown write failure on standard output");
+	if (fclose(stdout))
+		die("close failed on standard output: %s", strerror(errno));
+	return 0;
+}
+
+int execv_git_builtin(const char **argv)
+{
+	const char *cmd = argv[0];
+	static struct cmd_struct commands[] = {
+		{ "add", cmd_add, RUN_SETUP | NEED_WORK_TREE },
+		{ "annotate", cmd_annotate, RUN_SETUP },
+		{ "apply", cmd_apply },
+		{ "archive", cmd_archive },
+		{ "blame", cmd_blame, RUN_SETUP },
+		{ "branch", cmd_branch, RUN_SETUP },
+		{ "bundle", cmd_bundle },
+		{ "cat-file", cmd_cat_file, RUN_SETUP },
+		{ "checkout-index", cmd_checkout_index,
+			RUN_SETUP | NEED_WORK_TREE},
+		{ "check-ref-format", cmd_check_ref_format },
+		{ "check-attr", cmd_check_attr, RUN_SETUP | NEED_WORK_TREE },
+		{ "cherry", cmd_cherry, RUN_SETUP },
+		{ "cherry-pick", cmd_cherry_pick, RUN_SETUP | NEED_WORK_TREE },
+		{ "clean", cmd_clean, RUN_SETUP | NEED_WORK_TREE },
+		{ "commit", cmd_commit, RUN_SETUP | NEED_WORK_TREE },
+		{ "commit-tree", cmd_commit_tree, RUN_SETUP },
+		{ "config", cmd_config },
+		{ "count-objects", cmd_count_objects, RUN_SETUP },
+		{ "describe", cmd_describe, RUN_SETUP },
+		{ "diff", cmd_diff },
+		{ "diff-files", cmd_diff_files },
+		{ "diff-index", cmd_diff_index, RUN_SETUP },
+		{ "diff-tree", cmd_diff_tree, RUN_SETUP },
+		{ "fast-export", cmd_fast_export, RUN_SETUP },
+		{ "fetch", cmd_fetch, RUN_SETUP },
+		{ "fetch-pack", cmd_fetch_pack, RUN_SETUP },
+		{ "fetch--tool", cmd_fetch__tool, RUN_SETUP },
+		{ "fmt", cmd_fmt },
+		{ "fmt-merge-msg", cmd_fmt_merge_msg, RUN_SETUP },
+		{ "for-each-ref", cmd_for_each_ref, RUN_SETUP },
+		{ "format-patch", cmd_format_patch, RUN_SETUP },
+		{ "fsck", cmd_fsck, RUN_SETUP },
+		{ "fsck-objects", cmd_fsck, RUN_SETUP },
+		{ "gc", cmd_gc, RUN_SETUP },
+		{ "get-tar-commit-id", cmd_get_tar_commit_id },
+		{ "grep", cmd_grep, RUN_SETUP | USE_PAGER },
+		{ "help", cmd_help },
+#ifndef NO_CURL
+		{ "http-fetch", cmd_http_fetch, RUN_SETUP },
+#endif
+		{ "init", cmd_init_db },
+		{ "init-db", cmd_init_db },
+		{ "log", cmd_log, RUN_SETUP | USE_PAGER },
+		{ "ls-files", cmd_ls_files, RUN_SETUP },
+		{ "ls-tree", cmd_ls_tree, RUN_SETUP },
+		{ "ls-remote", cmd_ls_remote },
+		{ "mailinfo", cmd_mailinfo },
+		{ "mailsplit", cmd_mailsplit },
+		{ "merge-base", cmd_merge_base, RUN_SETUP },
+		{ "merge-file", cmd_merge_file },
+		{ "merge-ours", cmd_merge_ours, RUN_SETUP },
+		{ "mv", cmd_mv, RUN_SETUP | NEED_WORK_TREE },
+		{ "name-rev", cmd_name_rev, RUN_SETUP },
+		{ "pack-objects", cmd_pack_objects, RUN_SETUP },
+		{ "peek-remote", cmd_ls_remote },
+		{ "pickaxe", cmd_blame, RUN_SETUP },
+		{ "prune", cmd_prune, RUN_SETUP },
+		{ "prune-packed", cmd_prune_packed, RUN_SETUP },
+		{ "push", cmd_push, RUN_SETUP },
+		{ "read-tree", cmd_read_tree, RUN_SETUP },
+		{ "reflog", cmd_reflog, RUN_SETUP },
+		{ "repo-config", cmd_config },
+		{ "rerere", cmd_rerere, RUN_SETUP },
+		{ "reset", cmd_reset, RUN_SETUP },
+		{ "rev-list", cmd_rev_list, RUN_SETUP },
+		{ "rev-parse", cmd_rev_parse },
+		{ "revert", cmd_revert, RUN_SETUP | NEED_WORK_TREE },
+		{ "rm", cmd_rm, RUN_SETUP },
+		{ "send-pack", cmd_send_pack, RUN_SETUP },
+		{ "shortlog", cmd_shortlog, RUN_SETUP | USE_PAGER },
+		{ "show-branch", cmd_show_branch, RUN_SETUP },
+		{ "show", cmd_show, RUN_SETUP | USE_PAGER },
+		{ "status", cmd_status, RUN_SETUP | NEED_WORK_TREE },
+		{ "stripspace", cmd_stripspace },
+		{ "symbolic-ref", cmd_symbolic_ref, RUN_SETUP },
+		{ "tag", cmd_tag, RUN_SETUP },
+		{ "tar-tree", cmd_tar_tree },
+		{ "unpack-objects", cmd_unpack_objects, RUN_SETUP },
+		{ "update-index", cmd_update_index, RUN_SETUP },
+		{ "update-ref", cmd_update_ref, RUN_SETUP },
+		{ "upload-archive", cmd_upload_archive },
+		{ "verify-tag", cmd_verify_tag, RUN_SETUP },
+		{ "version", cmd_version },
+		{ "whatchanged", cmd_whatchanged, RUN_SETUP | USE_PAGER },
+		{ "write-tree", cmd_write_tree, RUN_SETUP },
+		{ "verify-pack", cmd_verify_pack },
+		{ "show-ref", cmd_show_ref, RUN_SETUP },
+		{ "pack-refs", cmd_pack_refs, RUN_SETUP },
+	};
+	int i;
+
+	/* Turn "git cmd --help" into "git help cmd" */
+	if (argv[0] && argv[1] && !strcmp(argv[1], "--help")) {
+		argv[1] = argv[0];
+		argv[0] = cmd = "help";
+	}
+
+	for (i = 0; i < ARRAY_SIZE(commands); i++) {
+		int argc;
+		struct cmd_struct *p = commands+i;
+		if (strcmp(p->cmd, cmd))
+			continue;
+		release_all_objects();
+		discard_cache();
+		for (argc = 0; argv[argc]; argc++)
+			; /* do nothing */
+		exit(run_command(p, argc, argv));
+	}
+	return -1;
+}
+
 int execv_git_cmd(const char **argv)
 {
 	struct strbuf cmd;
 	const char *tmp;
 
+	/* Try builtin first... */
+	execv_git_builtin(argv);
+
+	/* ... and then external commands */
 	strbuf_init(&cmd, 0);
 	strbuf_addf(&cmd, "git-%s", argv[0]);

diff --git a/exec_cmd.h b/exec_cmd.h
index a892355..bb15425 100644
--- a/exec_cmd.h
+++ b/exec_cmd.h

@@ -4,6 +4,7 @@
 extern void git_set_argv_exec_path(const char *exec_path);
 extern const char* git_exec_path(void);
 extern void setup_path(const char *);
+extern int execv_git_builtin(const char **argv); /* NULL terminated */
 extern int execv_git_cmd(const char **argv); /* NULL terminated */
 extern int execl_git_cmd(const char *cmd, ...);

diff --git a/git.c b/git.c
index 38b01ca..1523c4a 100644
--- a/git.c
+++ b/git.c

@@ -3,9 +3,6 @@
 #include "cache.h"
 #include "quote.h"
 
-const char git_usage_string[] =
-	"git [--version] [--exec-path[=GIT_EXEC_PATH]] [-p|--paginate|--no-pager] [--bare] [--git-dir=GIT_DIR] [--work-tree=GIT_WORK_TREE] [--help] COMMAND [ARGS]";
-
 static int handle_options(const char*** argv, int* argc, int* envchanged)
 {
 	int handled = 0;

@@ -231,169 +228,6 @@ static int handle_alias(int *argcp, const char ***argv)
 	return ret;
 }
 
-const char git_version_string[] = GIT_VERSION;
-
-#define RUN_SETUP	(1<<0)
-#define USE_PAGER	(1<<1)
-/*
- * require working tree to be present -- anything uses this needs
- * RUN_SETUP for reading from the configuration file.
- */
-#define NEED_WORK_TREE	(1<<2)
-
-struct cmd_struct {
-	const char *cmd;
-	int (*fn)(int, const char **, const char *);
-	int option;
-};
-
-static int run_command(struct cmd_struct *p, int argc, const char **argv)
-{
-	int status;
-	struct stat st;
-	const char *prefix;
-
-	prefix = NULL;
-	if (p->option & RUN_SETUP)
-		prefix = setup_git_directory();
-	if (p->option & USE_PAGER)
-		setup_pager();
-	if (p->option & NEED_WORK_TREE)
-		setup_work_tree();
-
-	trace_argv_printf(argv, argc, "trace: built-in: git");
-
-	status = p->fn(argc, argv, prefix);
-	if (status)
-		return status & 0xff;
-
-	/* Somebody closed stdout? */
-	if (fstat(fileno(stdout), &st))
-		return 0;
-	/* Ignore write errors for pipes and sockets.. */
-	if (S_ISFIFO(st.st_mode) || S_ISSOCK(st.st_mode))
-		return 0;
-
-	/* Check for ENOSPC and EIO errors.. */
-	if (fflush(stdout))
-		die("write failure on standard output: %s", strerror(errno));
-	if (ferror(stdout))
-		die("unknown write failure on standard output");
-	if (fclose(stdout))
-		die("close failed on standard output: %s", strerror(errno));
-	return 0;
-}
-
-static void handle_internal_command(int argc, const char **argv)
-{
-	const char *cmd = argv[0];
-	static struct cmd_struct commands[] = {
-		{ "add", cmd_add, RUN_SETUP | NEED_WORK_TREE },
-		{ "annotate", cmd_annotate, RUN_SETUP },
-		{ "apply", cmd_apply },
-		{ "archive", cmd_archive },
-		{ "blame", cmd_blame, RUN_SETUP },
-		{ "branch", cmd_branch, RUN_SETUP },
-		{ "bundle", cmd_bundle },
-		{ "cat-file", cmd_cat_file, RUN_SETUP },
-		{ "checkout-index", cmd_checkout_index,
-			RUN_SETUP | NEED_WORK_TREE},
-		{ "check-ref-format", cmd_check_ref_format },
-		{ "check-attr", cmd_check_attr, RUN_SETUP | NEED_WORK_TREE },
-		{ "cherry", cmd_cherry, RUN_SETUP },
-		{ "cherry-pick", cmd_cherry_pick, RUN_SETUP | NEED_WORK_TREE },
-		{ "clean", cmd_clean, RUN_SETUP | NEED_WORK_TREE },
-		{ "commit", cmd_commit, RUN_SETUP | NEED_WORK_TREE },
-		{ "commit-tree", cmd_commit_tree, RUN_SETUP },
-		{ "config", cmd_config },
-		{ "count-objects", cmd_count_objects, RUN_SETUP },
-		{ "describe", cmd_describe, RUN_SETUP },
-		{ "diff", cmd_diff },
-		{ "diff-files", cmd_diff_files },
-		{ "diff-index", cmd_diff_index, RUN_SETUP },
-		{ "diff-tree", cmd_diff_tree, RUN_SETUP },
-		{ "fast-export", cmd_fast_export, RUN_SETUP },
-		{ "fetch", cmd_fetch, RUN_SETUP },
-		{ "fetch-pack", cmd_fetch_pack, RUN_SETUP },
-		{ "fetch--tool", cmd_fetch__tool, RUN_SETUP },
-		{ "fmt", cmd_fmt },
-		{ "fmt-merge-msg", cmd_fmt_merge_msg, RUN_SETUP },
-		{ "for-each-ref", cmd_for_each_ref, RUN_SETUP },
-		{ "format-patch", cmd_format_patch, RUN_SETUP },
-		{ "fsck", cmd_fsck, RUN_SETUP },
-		{ "fsck-objects", cmd_fsck, RUN_SETUP },
-		{ "gc", cmd_gc, RUN_SETUP },
-		{ "get-tar-commit-id", cmd_get_tar_commit_id },
-		{ "grep", cmd_grep, RUN_SETUP | USE_PAGER },
-		{ "help", cmd_help },
-#ifndef NO_CURL
-		{ "http-fetch", cmd_http_fetch, RUN_SETUP },
-#endif
-		{ "init", cmd_init_db },
-		{ "init-db", cmd_init_db },
-		{ "log", cmd_log, RUN_SETUP | USE_PAGER },
-		{ "ls-files", cmd_ls_files, RUN_SETUP },
-		{ "ls-tree", cmd_ls_tree, RUN_SETUP },
-		{ "ls-remote", cmd_ls_remote },
-		{ "mailinfo", cmd_mailinfo },
-		{ "mailsplit", cmd_mailsplit },
-		{ "merge-base", cmd_merge_base, RUN_SETUP },
-		{ "merge-file", cmd_merge_file },
-		{ "merge-ours", cmd_merge_ours, RUN_SETUP },
-		{ "mv", cmd_mv, RUN_SETUP | NEED_WORK_TREE },
-		{ "name-rev", cmd_name_rev, RUN_SETUP },
-		{ "pack-objects", cmd_pack_objects, RUN_SETUP },
-		{ "peek-remote", cmd_ls_remote },
-		{ "pickaxe", cmd_blame, RUN_SETUP },
-		{ "prune", cmd_prune, RUN_SETUP },
-		{ "prune-packed", cmd_prune_packed, RUN_SETUP },
-		{ "push", cmd_push, RUN_SETUP },
-		{ "read-tree", cmd_read_tree, RUN_SETUP },
-		{ "reflog", cmd_reflog, RUN_SETUP },
-		{ "repo-config", cmd_config },
-		{ "rerere", cmd_rerere, RUN_SETUP },
-		{ "reset", cmd_reset, RUN_SETUP },
-		{ "rev-list", cmd_rev_list, RUN_SETUP },
-		{ "rev-parse", cmd_rev_parse },
-		{ "revert", cmd_revert, RUN_SETUP | NEED_WORK_TREE },
-		{ "rm", cmd_rm, RUN_SETUP },
-		{ "send-pack", cmd_send_pack, RUN_SETUP },
-		{ "shortlog", cmd_shortlog, RUN_SETUP | USE_PAGER },
-		{ "show-branch", cmd_show_branch, RUN_SETUP },
-		{ "show", cmd_show, RUN_SETUP | USE_PAGER },
-		{ "status", cmd_status, RUN_SETUP | NEED_WORK_TREE },
-		{ "stripspace", cmd_stripspace },
-		{ "symbolic-ref", cmd_symbolic_ref, RUN_SETUP },
-		{ "tag", cmd_tag, RUN_SETUP },
-		{ "tar-tree", cmd_tar_tree },
-		{ "unpack-objects", cmd_unpack_objects, RUN_SETUP },
-		{ "update-index", cmd_update_index, RUN_SETUP },
-		{ "update-ref", cmd_update_ref, RUN_SETUP },
-		{ "upload-archive", cmd_upload_archive },
-		{ "verify-tag", cmd_verify_tag, RUN_SETUP },
-		{ "version", cmd_version },
-		{ "whatchanged", cmd_whatchanged, RUN_SETUP | USE_PAGER },
-		{ "write-tree", cmd_write_tree, RUN_SETUP },
-		{ "verify-pack", cmd_verify_pack },
-		{ "show-ref", cmd_show_ref, RUN_SETUP },
-		{ "pack-refs", cmd_pack_refs, RUN_SETUP },
-	};
-	int i;
-
-	/* Turn "git cmd --help" into "git help cmd" */
-	if (argc > 1 && !strcmp(argv[1], "--help")) {
-		argv[1] = argv[0];
-		argv[0] = cmd = "help";
-	}
-
-	for (i = 0; i < ARRAY_SIZE(commands); i++) {
-		struct cmd_struct *p = commands+i;
-		if (strcmp(p->cmd, cmd))
-			continue;
-		exit(run_command(p, argc, argv));
-	}
-}
-
 int main(int argc, const char **argv)
 {
 	const char *cmd = argv[0] ? argv[0] : "git-help";

@@ -425,7 +259,7 @@ int main(int argc, const char **argv)
 	if (!prefixcmp(cmd, "git-")) {
 		cmd += 4;
 		argv[0] = cmd;
-		handle_internal_command(argc, argv);
+		execv_git_builtin(argv);
 		die("cannot handle %s internally", cmd);
 	}

@@ -453,10 +287,6 @@ int main(int argc, const char **argv)
 	setup_path(cmd_path);
 
 	while (1) {
-		/* See if it's an internal command */
-		handle_internal_command(argc, argv);
-
-		/* .. then try the external ones */
 		execv_git_cmd(argv);
 
 		/* It could be an alias -- this works around the insanity

-- 
1.5.3.6.2112.ge2263

Re: [PATCH 3/3] Introduce execv_git_builtin() and use it

From: Johannes Schindelin <hidden>
Date: 2016-06-15 22:43:55

Hi,

On Sun, 2 Dec 2007, Johannes Schindelin wrote:

+		{ "fmt", cmd_fmt },

Ah, well.  This slipped in by mistake.  Will resend in a few minutes.

Ciao,
Dscho

[REPLACEMENT PATCH 3/3] Introduce execv_git_builtin() and use it

From: Johannes Schindelin <hidden>
Date: 2016-06-15 22:43:55

You can call execv_git_builtin() to execute a builtin command.  This 
function is a reborn handle_internal_command() from git.c, and has the 
semantics of execv_git_cmd(), i.e. it exits with the exit code of the 
builtin if there is a matching builtin, but it avoids the real execvp() 
call.

This function calls release_all_objects() and discard_cache() to start 
from a clean slate (this, along with the calculation of argc, is the only 
difference from a straight code move).

The test suite passes, which at least does not contradict the hypothesis 
that this is good enough.  However, it might be necessary to de-initialize 
more global variables.

The function execv_git_cmd() and git.c's main() function were changed to 
take advantage of execv_git_builtin().

Signed-off-by: Johannes Schindelin <redacted>
---

	On Sun, 2 Dec 2007, Johannes Schindelin wrote:

	> Hi,
	> 
	> On Sun, 2 Dec 2007, Johannes Schindelin wrote:
	> 
	> > +		{ "fmt", cmd_fmt },
	> 
	> Ah, well.  This slipped in by mistake.  Will resend in a few 
	> minutes.

	Here we go.

 exec_cmd.c |  176 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 exec_cmd.h |    1 +
 git.c      |  170 +---------------------------------------------------------
 3 files changed, 178 insertions(+), 169 deletions(-)

diff --git a/exec_cmd.c b/exec_cmd.c
index 2d0a758..745b951 100644
--- a/exec_cmd.c
+++ b/exec_cmd.c

@@ -1,6 +1,8 @@
 #include "cache.h"
 #include "exec_cmd.h"
 #include "quote.h"
+#include "builtin.h"
+#include "object.h"
 #define MAX_ARGS	32
 
 extern char **environ;

@@ -63,11 +65,185 @@ void setup_path(const char *cmd_path)
 	strbuf_release(&new_path);
 }
 
+const char git_usage_string[] =
+	"git [--version] [--exec-path[=GIT_EXEC_PATH]] [-p|--paginate|--no-pager] [--bare] [--git-dir=GIT_DIR] [--work-tree=GIT_WORK_TREE] [--help] COMMAND [ARGS]";
+
+const char git_version_string[] = GIT_VERSION;
+
+#define RUN_SETUP	(1<<0)
+#define USE_PAGER	(1<<1)
+/*
+ * require working tree to be present -- anything uses this needs
+ * RUN_SETUP for reading from the configuration file.
+ */
+#define NEED_WORK_TREE	(1<<2)
+
+struct cmd_struct {
+	const char *cmd;
+	int (*fn)(int, const char **, const char *);
+	int option;
+};
+
+static int run_command(struct cmd_struct *p, int argc, const char **argv)
+{
+	int status;
+	struct stat st;
+	const char *prefix;
+
+	prefix = NULL;
+	if (p->option & RUN_SETUP)
+		prefix = setup_git_directory();
+	if (p->option & USE_PAGER)
+		setup_pager();
+	if (p->option & NEED_WORK_TREE)
+		setup_work_tree();
+
+	trace_argv_printf(argv, argc, "trace: built-in: git");
+
+	status = p->fn(argc, argv, prefix);
+	if (status)
+		return status & 0xff;
+
+	/* Somebody closed stdout? */
+	if (fstat(fileno(stdout), &st))
+		return 0;
+	/* Ignore write errors for pipes and sockets.. */
+	if (S_ISFIFO(st.st_mode) || S_ISSOCK(st.st_mode))
+		return 0;
+
+	/* Check for ENOSPC and EIO errors.. */
+	if (fflush(stdout))
+		die("write failure on standard output: %s", strerror(errno));
+	if (ferror(stdout))
+		die("unknown write failure on standard output");
+	if (fclose(stdout))
+		die("close failed on standard output: %s", strerror(errno));
+	return 0;
+}
+
+int execv_git_builtin(const char **argv)
+{
+	const char *cmd = argv[0];
+	static struct cmd_struct commands[] = {
+		{ "add", cmd_add, RUN_SETUP | NEED_WORK_TREE },
+		{ "annotate", cmd_annotate, RUN_SETUP },
+		{ "apply", cmd_apply },
+		{ "archive", cmd_archive },
+		{ "blame", cmd_blame, RUN_SETUP },
+		{ "branch", cmd_branch, RUN_SETUP },
+		{ "bundle", cmd_bundle },
+		{ "cat-file", cmd_cat_file, RUN_SETUP },
+		{ "checkout-index", cmd_checkout_index,
+			RUN_SETUP | NEED_WORK_TREE},
+		{ "check-ref-format", cmd_check_ref_format },
+		{ "check-attr", cmd_check_attr, RUN_SETUP | NEED_WORK_TREE },
+		{ "cherry", cmd_cherry, RUN_SETUP },
+		{ "cherry-pick", cmd_cherry_pick, RUN_SETUP | NEED_WORK_TREE },
+		{ "clean", cmd_clean, RUN_SETUP | NEED_WORK_TREE },
+		{ "commit", cmd_commit, RUN_SETUP | NEED_WORK_TREE },
+		{ "commit-tree", cmd_commit_tree, RUN_SETUP },
+		{ "config", cmd_config },
+		{ "count-objects", cmd_count_objects, RUN_SETUP },
+		{ "describe", cmd_describe, RUN_SETUP },
+		{ "diff", cmd_diff },
+		{ "diff-files", cmd_diff_files },
+		{ "diff-index", cmd_diff_index, RUN_SETUP },
+		{ "diff-tree", cmd_diff_tree, RUN_SETUP },
+		{ "fetch", cmd_fetch, RUN_SETUP },
+		{ "fetch-pack", cmd_fetch_pack, RUN_SETUP },
+		{ "fetch--tool", cmd_fetch__tool, RUN_SETUP },
+		{ "fmt-merge-msg", cmd_fmt_merge_msg, RUN_SETUP },
+		{ "for-each-ref", cmd_for_each_ref, RUN_SETUP },
+		{ "format-patch", cmd_format_patch, RUN_SETUP },
+		{ "fsck", cmd_fsck, RUN_SETUP },
+		{ "fsck-objects", cmd_fsck, RUN_SETUP },
+		{ "gc", cmd_gc, RUN_SETUP },
+		{ "get-tar-commit-id", cmd_get_tar_commit_id },
+		{ "grep", cmd_grep, RUN_SETUP | USE_PAGER },
+		{ "help", cmd_help },
+#ifndef NO_CURL
+		{ "http-fetch", cmd_http_fetch, RUN_SETUP },
+#endif
+		{ "init", cmd_init_db },
+		{ "init-db", cmd_init_db },
+		{ "log", cmd_log, RUN_SETUP | USE_PAGER },
+		{ "ls-files", cmd_ls_files, RUN_SETUP },
+		{ "ls-tree", cmd_ls_tree, RUN_SETUP },
+		{ "ls-remote", cmd_ls_remote },
+		{ "mailinfo", cmd_mailinfo },
+		{ "mailsplit", cmd_mailsplit },
+		{ "merge-base", cmd_merge_base, RUN_SETUP },
+		{ "merge-file", cmd_merge_file },
+		{ "merge-ours", cmd_merge_ours, RUN_SETUP },
+		{ "mv", cmd_mv, RUN_SETUP | NEED_WORK_TREE },
+		{ "name-rev", cmd_name_rev, RUN_SETUP },
+		{ "pack-objects", cmd_pack_objects, RUN_SETUP },
+		{ "peek-remote", cmd_ls_remote },
+		{ "pickaxe", cmd_blame, RUN_SETUP },
+		{ "prune", cmd_prune, RUN_SETUP },
+		{ "prune-packed", cmd_prune_packed, RUN_SETUP },
+		{ "push", cmd_push, RUN_SETUP },
+		{ "read-tree", cmd_read_tree, RUN_SETUP },
+		{ "reflog", cmd_reflog, RUN_SETUP },
+		{ "repo-config", cmd_config },
+		{ "rerere", cmd_rerere, RUN_SETUP },
+		{ "reset", cmd_reset, RUN_SETUP },
+		{ "rev-list", cmd_rev_list, RUN_SETUP },
+		{ "rev-parse", cmd_rev_parse },
+		{ "revert", cmd_revert, RUN_SETUP | NEED_WORK_TREE },
+		{ "rm", cmd_rm, RUN_SETUP },
+		{ "send-pack", cmd_send_pack, RUN_SETUP },
+		{ "shortlog", cmd_shortlog, RUN_SETUP | USE_PAGER },
+		{ "show-branch", cmd_show_branch, RUN_SETUP },
+		{ "show", cmd_show, RUN_SETUP | USE_PAGER },
+		{ "status", cmd_status, RUN_SETUP | NEED_WORK_TREE },
+		{ "stripspace", cmd_stripspace },
+		{ "symbolic-ref", cmd_symbolic_ref, RUN_SETUP },
+		{ "tag", cmd_tag, RUN_SETUP },
+		{ "tar-tree", cmd_tar_tree },
+		{ "unpack-objects", cmd_unpack_objects, RUN_SETUP },
+		{ "update-index", cmd_update_index, RUN_SETUP },
+		{ "update-ref", cmd_update_ref, RUN_SETUP },
+		{ "upload-archive", cmd_upload_archive },
+		{ "verify-tag", cmd_verify_tag, RUN_SETUP },
+		{ "version", cmd_version },
+		{ "whatchanged", cmd_whatchanged, RUN_SETUP | USE_PAGER },
+		{ "write-tree", cmd_write_tree, RUN_SETUP },
+		{ "verify-pack", cmd_verify_pack },
+		{ "show-ref", cmd_show_ref, RUN_SETUP },
+		{ "pack-refs", cmd_pack_refs, RUN_SETUP },
+	};
+	int i;
+
+	/* Turn "git cmd --help" into "git help cmd" */
+	if (argv[0] && argv[1] && !strcmp(argv[1], "--help")) {
+		argv[1] = argv[0];
+		argv[0] = cmd = "help";
+	}
+
+	for (i = 0; i < ARRAY_SIZE(commands); i++) {
+		int argc;
+		struct cmd_struct *p = commands+i;
+		if (strcmp(p->cmd, cmd))
+			continue;
+		release_all_objects();
+		discard_cache();
+		for (argc = 0; argv[argc]; argc++)
+			; /* do nothing */
+		exit(run_command(p, argc, argv));
+	}
+	return -1;
+}
+
 int execv_git_cmd(const char **argv)
 {
 	struct strbuf cmd;
 	const char *tmp;
 
+	/* Try builtin first... */
+	execv_git_builtin(argv);
+
+	/* ... and then external commands */
 	strbuf_init(&cmd, 0);
 	strbuf_addf(&cmd, "git-%s", argv[0]);

diff --git a/exec_cmd.h b/exec_cmd.h
index a892355..bb15425 100644
--- a/exec_cmd.h
+++ b/exec_cmd.h

@@ -4,6 +4,7 @@
 extern void git_set_argv_exec_path(const char *exec_path);
 extern const char* git_exec_path(void);
 extern void setup_path(const char *);
+extern int execv_git_builtin(const char **argv); /* NULL terminated */
 extern int execv_git_cmd(const char **argv); /* NULL terminated */
 extern int execl_git_cmd(const char *cmd, ...);

diff --git a/git.c b/git.c
index 95296aa..1523c4a 100644
--- a/git.c
+++ b/git.c

@@ -3,9 +3,6 @@
 #include "cache.h"
 #include "quote.h"
 
-const char git_usage_string[] =
-	"git [--version] [--exec-path[=GIT_EXEC_PATH]] [-p|--paginate|--no-pager] [--bare] [--git-dir=GIT_DIR] [--work-tree=GIT_WORK_TREE] [--help] COMMAND [ARGS]";
-
 static int handle_options(const char*** argv, int* argc, int* envchanged)
 {
 	int handled = 0;

@@ -231,167 +228,6 @@ static int handle_alias(int *argcp, const char ***argv)
 	return ret;
 }
 
-const char git_version_string[] = GIT_VERSION;
-
-#define RUN_SETUP	(1<<0)
-#define USE_PAGER	(1<<1)
-/*
- * require working tree to be present -- anything uses this needs
- * RUN_SETUP for reading from the configuration file.
- */
-#define NEED_WORK_TREE	(1<<2)
-
-struct cmd_struct {
-	const char *cmd;
-	int (*fn)(int, const char **, const char *);
-	int option;
-};
-
-static int run_command(struct cmd_struct *p, int argc, const char **argv)
-{
-	int status;
-	struct stat st;
-	const char *prefix;
-
-	prefix = NULL;
-	if (p->option & RUN_SETUP)
-		prefix = setup_git_directory();
-	if (p->option & USE_PAGER)
-		setup_pager();
-	if (p->option & NEED_WORK_TREE)
-		setup_work_tree();
-
-	trace_argv_printf(argv, argc, "trace: built-in: git");
-
-	status = p->fn(argc, argv, prefix);
-	if (status)
-		return status & 0xff;
-
-	/* Somebody closed stdout? */
-	if (fstat(fileno(stdout), &st))
-		return 0;
-	/* Ignore write errors for pipes and sockets.. */
-	if (S_ISFIFO(st.st_mode) || S_ISSOCK(st.st_mode))
-		return 0;
-
-	/* Check for ENOSPC and EIO errors.. */
-	if (fflush(stdout))
-		die("write failure on standard output: %s", strerror(errno));
-	if (ferror(stdout))
-		die("unknown write failure on standard output");
-	if (fclose(stdout))
-		die("close failed on standard output: %s", strerror(errno));
-	return 0;
-}
-
-static void handle_internal_command(int argc, const char **argv)
-{
-	const char *cmd = argv[0];
-	static struct cmd_struct commands[] = {
-		{ "add", cmd_add, RUN_SETUP | NEED_WORK_TREE },
-		{ "annotate", cmd_annotate, RUN_SETUP },
-		{ "apply", cmd_apply },
-		{ "archive", cmd_archive },
-		{ "blame", cmd_blame, RUN_SETUP },
-		{ "branch", cmd_branch, RUN_SETUP },
-		{ "bundle", cmd_bundle },
-		{ "cat-file", cmd_cat_file, RUN_SETUP },
-		{ "checkout-index", cmd_checkout_index,
-			RUN_SETUP | NEED_WORK_TREE},
-		{ "check-ref-format", cmd_check_ref_format },
-		{ "check-attr", cmd_check_attr, RUN_SETUP | NEED_WORK_TREE },
-		{ "cherry", cmd_cherry, RUN_SETUP },
-		{ "cherry-pick", cmd_cherry_pick, RUN_SETUP | NEED_WORK_TREE },
-		{ "clean", cmd_clean, RUN_SETUP | NEED_WORK_TREE },
-		{ "commit", cmd_commit, RUN_SETUP | NEED_WORK_TREE },
-		{ "commit-tree", cmd_commit_tree, RUN_SETUP },
-		{ "config", cmd_config },
-		{ "count-objects", cmd_count_objects, RUN_SETUP },
-		{ "describe", cmd_describe, RUN_SETUP },
-		{ "diff", cmd_diff },
-		{ "diff-files", cmd_diff_files },
-		{ "diff-index", cmd_diff_index, RUN_SETUP },
-		{ "diff-tree", cmd_diff_tree, RUN_SETUP },
-		{ "fetch", cmd_fetch, RUN_SETUP },
-		{ "fetch-pack", cmd_fetch_pack, RUN_SETUP },
-		{ "fetch--tool", cmd_fetch__tool, RUN_SETUP },
-		{ "fmt-merge-msg", cmd_fmt_merge_msg, RUN_SETUP },
-		{ "for-each-ref", cmd_for_each_ref, RUN_SETUP },
-		{ "format-patch", cmd_format_patch, RUN_SETUP },
-		{ "fsck", cmd_fsck, RUN_SETUP },
-		{ "fsck-objects", cmd_fsck, RUN_SETUP },
-		{ "gc", cmd_gc, RUN_SETUP },
-		{ "get-tar-commit-id", cmd_get_tar_commit_id },
-		{ "grep", cmd_grep, RUN_SETUP | USE_PAGER },
-		{ "help", cmd_help },
-#ifndef NO_CURL
-		{ "http-fetch", cmd_http_fetch, RUN_SETUP },
-#endif
-		{ "init", cmd_init_db },
-		{ "init-db", cmd_init_db },
-		{ "log", cmd_log, RUN_SETUP | USE_PAGER },
-		{ "ls-files", cmd_ls_files, RUN_SETUP },
-		{ "ls-tree", cmd_ls_tree, RUN_SETUP },
-		{ "ls-remote", cmd_ls_remote },
-		{ "mailinfo", cmd_mailinfo },
-		{ "mailsplit", cmd_mailsplit },
-		{ "merge-base", cmd_merge_base, RUN_SETUP },
-		{ "merge-file", cmd_merge_file },
-		{ "merge-ours", cmd_merge_ours, RUN_SETUP },
-		{ "mv", cmd_mv, RUN_SETUP | NEED_WORK_TREE },
-		{ "name-rev", cmd_name_rev, RUN_SETUP },
-		{ "pack-objects", cmd_pack_objects, RUN_SETUP },
-		{ "peek-remote", cmd_ls_remote },
-		{ "pickaxe", cmd_blame, RUN_SETUP },
-		{ "prune", cmd_prune, RUN_SETUP },
-		{ "prune-packed", cmd_prune_packed, RUN_SETUP },
-		{ "push", cmd_push, RUN_SETUP },
-		{ "read-tree", cmd_read_tree, RUN_SETUP },
-		{ "reflog", cmd_reflog, RUN_SETUP },
-		{ "repo-config", cmd_config },
-		{ "rerere", cmd_rerere, RUN_SETUP },
-		{ "reset", cmd_reset, RUN_SETUP },
-		{ "rev-list", cmd_rev_list, RUN_SETUP },
-		{ "rev-parse", cmd_rev_parse },
-		{ "revert", cmd_revert, RUN_SETUP | NEED_WORK_TREE },
-		{ "rm", cmd_rm, RUN_SETUP },
-		{ "send-pack", cmd_send_pack, RUN_SETUP },
-		{ "shortlog", cmd_shortlog, RUN_SETUP | USE_PAGER },
-		{ "show-branch", cmd_show_branch, RUN_SETUP },
-		{ "show", cmd_show, RUN_SETUP | USE_PAGER },
-		{ "status", cmd_status, RUN_SETUP | NEED_WORK_TREE },
-		{ "stripspace", cmd_stripspace },
-		{ "symbolic-ref", cmd_symbolic_ref, RUN_SETUP },
-		{ "tag", cmd_tag, RUN_SETUP },
-		{ "tar-tree", cmd_tar_tree },
-		{ "unpack-objects", cmd_unpack_objects, RUN_SETUP },
-		{ "update-index", cmd_update_index, RUN_SETUP },
-		{ "update-ref", cmd_update_ref, RUN_SETUP },
-		{ "upload-archive", cmd_upload_archive },
-		{ "verify-tag", cmd_verify_tag, RUN_SETUP },
-		{ "version", cmd_version },
-		{ "whatchanged", cmd_whatchanged, RUN_SETUP | USE_PAGER },
-		{ "write-tree", cmd_write_tree, RUN_SETUP },
-		{ "verify-pack", cmd_verify_pack },
-		{ "show-ref", cmd_show_ref, RUN_SETUP },
-		{ "pack-refs", cmd_pack_refs, RUN_SETUP },
-	};
-	int i;
-
-	/* Turn "git cmd --help" into "git help cmd" */
-	if (argc > 1 && !strcmp(argv[1], "--help")) {
-		argv[1] = argv[0];
-		argv[0] = cmd = "help";
-	}
-
-	for (i = 0; i < ARRAY_SIZE(commands); i++) {
-		struct cmd_struct *p = commands+i;
-		if (strcmp(p->cmd, cmd))
-			continue;
-		exit(run_command(p, argc, argv));
-	}
-}
-
 int main(int argc, const char **argv)
 {
 	const char *cmd = argv[0] ? argv[0] : "git-help";

@@ -423,7 +259,7 @@ int main(int argc, const char **argv)
 	if (!prefixcmp(cmd, "git-")) {
 		cmd += 4;
 		argv[0] = cmd;
-		handle_internal_command(argc, argv);
+		execv_git_builtin(argv);
 		die("cannot handle %s internally", cmd);
 	}

@@ -451,10 +287,6 @@ int main(int argc, const char **argv)
 	setup_path(cmd_path);
 
 	while (1) {
-		/* See if it's an internal command */
-		handle_internal_command(argc, argv);
-
-		/* .. then try the external ones */
 		execv_git_cmd(argv);
 
 		/* It could be an alias -- this works around the insanity

-- 
1.5.3.6.2112.ge2263

Re: [PATCH 0/3] Call builtin functions directly, was Re: [PATCH] transport.c: call dash-less form of receive-pack and upload-pack on remote

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:43:55

Johannes Schindelin [off-list ref] writes:

Okay, I bit the apple and tried to move the builtins into the library, and 
rename handle_internal_command into execv_git_builtin(), moving it into 
exec-cmd.c.

Big mistake.

I really feel this should not go in.  Anything called exec _should_
assure the callers that the new command will start from a clean slate,
and the way to give that assurance is by actually doing exec(), not
introducing "clean-up" functions for random things we can think of (like
cached objects) and risking of forgetting some others.  I do not think
the complexity is worth it.

The first step we have decided to take is to move git-foo form out of
users' PATH.  This would reduce the cluttered PATH problem, and it means
not all of external commands have to become built-ins on a single flag
day.  I also think it has always been a nice touch that we allowed users
to drop their own custom git-foo script to their path and call "git foo"
as if it is part of the official git suite, so spawning commands in
git-foo form needs to be supported via GIT_EXEC_PATH even if everything
eventually becomes built-in.

So I would prefer doing something like this instead for v1.5.5 (see
the top of updated release notes for 1.5.4 for deprecation notice).

 * execv_git_cmd() function will exec "git" with the given subcommand
   and its arguments;

 * The command dispatcher of git potty itself will first try the
   built-ins, and then try externals in dash form (which cannot be done
   with execv_git_cmd() anymore), and then aliases.

 * Just to be nice, we allow git-shell to treat "git foo arg" as if
   "git-foo arg" was given, but it continues to use execv_git_cmd(), and
   starts from a clean slate.

---
 exec_cmd.c |   31 ++++++++++++-------------------
 git.c      |   32 +++++++++++++++++++++++++++++++-
 shell.c    |   26 +++++++++++++++-----------
 3 files changed, 58 insertions(+), 31 deletions(-)

diff --git a/exec_cmd.c b/exec_cmd.c
index 2d0a758..10b2908 100644
--- a/exec_cmd.c
+++ b/exec_cmd.c

@@ -65,32 +65,25 @@ void setup_path(const char *cmd_path)
 
 int execv_git_cmd(const char **argv)
 {
-	struct strbuf cmd;
-	const char *tmp;
-
-	strbuf_init(&cmd, 0);
-	strbuf_addf(&cmd, "git-%s", argv[0]);
+	int argc;
+	const char **nargv;
 
-	/*
-	 * argv[0] must be the git command, but the argv array
-	 * belongs to the caller, and may be reused in
-	 * subsequent loop iterations. Save argv[0] and
-	 * restore it on error.
-	 */
-	tmp = argv[0];
-	argv[0] = cmd.buf;
+	for (argc = 0; argv[argc]; argc++)
+		; /* just counting */
+	nargv = xmalloc(sizeof(*nargv) * (argc + 2));
 
-	trace_argv_printf(argv, -1, "trace: exec:");
+	nargv[0] = "git";
+	for (argc = 0; argv[argc]; argc++)
+		nargv[argc + 1] = argv[argc];
+	nargv[argc + 1] = NULL;
+	trace_argv_printf(nargv, -1, "trace: exec:");
 
 	/* execvp() can only ever return if it fails */
-	execvp(cmd.buf, (char **)argv);
+	execvp("git", (char **)nargv);
 
 	trace_printf("trace: exec failed: %s\n", strerror(errno));
 
-	argv[0] = tmp;
-
-	strbuf_release(&cmd);
-
+	free(nargv);
 	return -1;
 }

diff --git a/git.c b/git.c
index 01bbbc7..d690426 100644
--- a/git.c
+++ b/git.c

@@ -382,6 +382,36 @@ static void handle_internal_command(int argc, const char **argv)
 	}
 }
 
+static void execv_dashed_external(const char **argv)
+{
+	struct strbuf cmd;
+	const char *tmp;
+
+	strbuf_init(&cmd, 0);
+	strbuf_addf(&cmd, "git-%s", argv[0]);
+
+	/*
+	 * argv[0] must be the git command, but the argv array
+	 * belongs to the caller, and may be reused in
+	 * subsequent loop iterations. Save argv[0] and
+	 * restore it on error.
+	 */
+	tmp = argv[0];
+	argv[0] = cmd.buf;
+
+	trace_argv_printf(argv, -1, "trace: exec:");
+
+	/* execvp() can only ever return if it fails */
+	execvp(cmd.buf, (char **)argv);
+
+	trace_printf("trace: exec failed: %s\n", strerror(errno));
+
+	argv[0] = tmp;
+
+	strbuf_release(&cmd);
+}
+
+
 int main(int argc, const char **argv)
 {
 	const char *cmd = argv[0] ? argv[0] : "git-help";

@@ -445,7 +475,7 @@ int main(int argc, const char **argv)
 		handle_internal_command(argc, argv);
 
 		/* .. then try the external ones */
-		execv_git_cmd(argv);
+		execv_dashed_external(argv);
 
 		/* It could be an alias -- this works around the insanity
 		 * of overriding "git log" with "git show" by having

diff --git a/shell.c b/shell.c
index 9826109..729797c 100644
--- a/shell.c
+++ b/shell.c

@@ -19,17 +19,13 @@ static int do_generic_cmd(const char *me, char *arg)
 	return execv_git_cmd(my_argv);
 }
 
-static int do_cvs_cmd(const char *me, char *arg)
+static int do_cvs_cmd(void)
 {
 	const char *cvsserver_argv[3] = {
 		"cvsserver", "server", NULL
 	};
 
-	if (!arg || strcmp(arg, "server"))
-		die("git-cvsserver only handles server: %s", arg);
-
 	setup_path(NULL);
-
 	return execv_git_cmd(cvsserver_argv);
 }

@@ -40,7 +36,6 @@ static struct commands {
 } cmd_list[] = {
 	{ "git-receive-pack", do_generic_cmd },
 	{ "git-upload-pack", do_generic_cmd },
-	{ "cvs", do_cvs_cmd },
 	{ NULL },
 };

@@ -49,15 +44,24 @@ int main(int argc, char **argv)
 	char *prog;
 	struct commands *cmd;
 
+	/*
+	 * Special hack to pretend to be a CVS server
+	 */
 	if (argc == 2 && !strcmp(argv[1], "cvs server"))
-		argv--;
-	/* We want to see "-c cmd args", and nothing else */
-	else if (argc != 3 || strcmp(argv[1], "-c"))
+		exit(do_cvs_cmd());
+
+	/*
+	 * We do not accept anything but "-c" followed by "cmd arg",
+	 * where "cmd" is a very limited subset of git commands.
+	 */
+	if (argc != 3 || strcmp(argv[1], "-c"))
 		die("What do you think I am? A shell?");
 
 	prog = argv[2];
-	argv += 2;
-	argc -= 2;
+	if (!strncmp(prog, "git", 3) && isspace(prog[3]))
+		/* Accept "git foo" as if the caller said "git-foo". */
+		prog[3] = '-';
+
 	for (cmd = cmd_list ; cmd->name ; cmd++) {
 		int len = strlen(cmd->name);
 		char *arg;

Re: [PATCH 0/3] Call builtin functions directly, was Re: [PATCH] transport.c: call dash-less form of receive-pack and upload-pack on remote

From: Johannes Schindelin <hidden>
Date: 2016-06-15 22:43:55

Hi,

On Sat, 1 Dec 2007, Junio C Hamano wrote:

Johannes Schindelin [off-list ref] writes:

quoted

Okay, I bit the apple and tried to move the builtins into the library, 
and rename handle_internal_command into execv_git_builtin(), moving it 
into exec-cmd.c.

Big mistake.

I really feel this should not go in.

Hmm.

My rationale was not only avoiding an exec() (which I would find worth it 
on its own), but because this would be a non-verbal step towards 
libification.

Ciao,
Dscho

`h`	back out one level
`j`	next message in thread
`k`	previous message in thread
`l`	drill in
`Esc`	close help / fold thread tree
`?`	toggle this help