[PATCH 1/3] Refactor working tree setup

[PATCH 1/3] Refactor working tree setup

[Mike Hommey]

Create a setup_work_tree() that can be used from any command requiring
a working tree conditionally.

Signed-off-by: Mike Hommey <mh@glandium.org>
---

This kind of implements what Dscho suggested in
http://article.gmane.org/gmane.comp.version-control.git/62487

 cache.h |    1 +
 git.c   |   11 +++--------
 setup.c |    9 +++++++++
 3 files changed, 13 insertions(+), 8 deletions(-)

diff --git a/cache.h b/cache.h
index bfffa05..497b9f9 100644
--- a/cache.h
+++ b/cache.h
@@ -222,6 +222,7 @@ extern const char *get_git_work_tree(void);
 #define ALTERNATE_DB_ENVIRONMENT "GIT_ALTERNATE_OBJECT_DIRECTORIES"
 
 extern const char **get_pathspec(const char *prefix, const char **pathspec);
+extern void setup_work_tree(void);
 extern const char *setup_git_directory_gently(int *);
 extern const char *setup_git_directory(void);
 extern const char *prefix_path(const char *prefix, int len, const char *path);
diff --git a/git.c b/git.c
index 4e10581..eb31c93 100644
--- a/git.c
+++ b/git.c
@@ -249,14 +249,9 @@ static int run_command(struct cmd_struct *p, int argc, const char **argv)
 		prefix = setup_git_directory();
 	if (p->option & USE_PAGER)
 		setup_pager();
-	if (p->option & NEED_WORK_TREE) {
-		const char *work_tree = get_git_work_tree();
-		const char *git_dir = get_git_dir();
-		if (!is_absolute_path(git_dir))
-			set_git_dir(make_absolute_path(git_dir));
-		if (!work_tree || chdir(work_tree))
-			die("%s must be run in a work tree", p->cmd);
-	}
+	if (p->option & NEED_WORK_TREE)
+		setup_work_tree();
+
 	trace_argv_printf(argv, argc, "trace: built-in: git");
 
 	status = p->fn(argc, argv, prefix);
diff --git a/setup.c b/setup.c
index 145eca5..df006d9 100644
--- a/setup.c
+++ b/setup.c
@@ -206,6 +206,15 @@ static const char *set_work_tree(const char *dir)
 	return NULL;
 }
 
+void setup_work_tree(void) {
+	const char *work_tree = get_git_work_tree();
+	const char *git_dir = get_git_dir();
+	if (!is_absolute_path(git_dir))
+		set_git_dir(make_absolute_path(git_dir));
+	if (!work_tree || chdir(work_tree))
+		die("This operation must be run in a work tree");
+}
+
 /*
  * We cannot decide in this function whether we are in the work tree or
  * not, since the config can only be read _after_ this function was called.
-- 
1.5.3.5

[PATCH 2/3] Use setup_work_tree() in builtin-ls-files.c

[Mike Hommey]

Signed-off-by: Mike Hommey <mh@glandium.org>
---
 builtin-ls-files.c |    7 ++-----
 1 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/builtin-ls-files.c b/builtin-ls-files.c
index b70da18..e0b856f 100644
--- a/builtin-ls-files.c
+++ b/builtin-ls-files.c
@@ -525,11 +525,8 @@ int cmd_ls_files(int argc, const char **argv, const char *prefix)
 		break;
 	}
 
-	if (require_work_tree && !is_inside_work_tree()) {
-		const char *work_tree = get_git_work_tree();
-		if (!work_tree || chdir(work_tree))
-			die("This operation must be run in a work tree");
-	}
+	if (require_work_tree && !is_inside_work_tree())
+		setup_work_tree();
 
 	pathspec = get_pathspec(prefix, argv + i);
 
-- 
1.5.3.5

[PATCH 3/3] Don't always require working tree for git-rm

[Mike Hommey]

This allows to do git rm --cached -r directory, instead of
git ls-files -z directory | git update-index --remove -z --stdin.
This can be particularly useful for git-filter-branch users.

Signed-off-by: Mike Hommey <mh@glandium.org>
---

This replaces the patch from
http://article.gmane.org/gmane.comp.version-control.git/63227

 builtin-rm.c |    3 +++
 git.c        |    2 +-
 2 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/builtin-rm.c b/builtin-rm.c
index bca2bd9..a3d25e6 100644
--- a/builtin-rm.c
+++ b/builtin-rm.c
@@ -155,6 +155,9 @@ int cmd_rm(int argc, const char **argv, const char *prefix)
 	if (!argc)
 		usage_with_options(builtin_rm_usage, builtin_rm_options);
 
+	if (!index_only)
+		setup_work_tree();
+
 	pathspec = get_pathspec(prefix, argv);
 	seen = NULL;
 	for (i = 0; pathspec[i] ; i++)
diff --git a/git.c b/git.c
index eb31c93..4a250f7 100644
--- a/git.c
+++ b/git.c
@@ -340,7 +340,7 @@ static void handle_internal_command(int argc, const char **argv)
 		{ "rev-list", cmd_rev_list, RUN_SETUP },
 		{ "rev-parse", cmd_rev_parse, RUN_SETUP },
 		{ "revert", cmd_revert, RUN_SETUP | NEED_WORK_TREE },
-		{ "rm", cmd_rm, RUN_SETUP | NEED_WORK_TREE },
+		{ "rm", cmd_rm, RUN_SETUP },
 		{ "runstatus", cmd_runstatus, RUN_SETUP | NEED_WORK_TREE },
 		{ "shortlog", cmd_shortlog, RUN_SETUP | USE_PAGER },
 		{ "show-branch", cmd_show_branch, RUN_SETUP },
-- 
1.5.3.5

Re: [PATCH 1/3] Refactor working tree setup

[Johannes Schindelin]

Hi,

On Sat, 3 Nov 2007, Mike Hommey wrote:

> Create a setup_work_tree() that can be used from any command requiring a 
> working tree conditionally.

All three patches look fine to me.

Ciao,
Dscho

Re: [PATCH 1/3] Refactor working tree setup

[Miles Bader]

Mike Hommey <mh@glandium.org> writes:
> Create a setup_work_tree() that can be used from any command requiring
> a working tree conditionally.
...
> +void setup_work_tree(void) {
> +	const char *work_tree = get_git_work_tree();

Hi, could you please not use this "function begin brace at EOL" style?

It's inconsistent with the rest of the source, makes the code harder to
read, and confuses Emacs in some cases[1].  Also it's an abomination
unto God, but I imagine it's the first of these reasons that you'll care
about the most... :-)

Thanks,

-Miles


[1] Until quite recently, Emacs c-mode couldn't find the beginning of
such functions (recent versions of c-mode seem OK though).

-- 
The secret to creativity is knowing how to hide your sources.
  --Albert Einstein

Re: [PATCH 1/3] Refactor working tree setup

[Junio C Hamano]

Miles Bader <miles@gnu.org> writes:

> Mike Hommey <mh@glandium.org> writes:
>> Create a setup_work_tree() that can be used from any command requiring
>> a working tree conditionally.
> ...
>> +void setup_work_tree(void) {
>> +	const char *work_tree = get_git_work_tree();
>
> Hi, could you please not use this "function begin brace at EOL" style?

Hmm.  I should have been more careful.

-- >8 --
[PATCH] Style: place opening brace of a function definition at column 1

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---

 bundle.c    |    3 ++-
 daemon.c    |    6 ++++--
 dir.c       |    3 ++-
 help.c      |    3 ++-
 quote.c     |    3 ++-
 setup.c     |    3 ++-
 transport.c |    6 ++++--
 utf8.c      |    3 ++-
 8 files changed, 20 insertions(+), 10 deletions(-)

diff --git a/bundle.c b/bundle.c
index 0869fcf..e4d60cd 100644
--- a/bundle.c
+++ b/bundle.c
@@ -23,7 +23,8 @@ static void add_to_ref_list(const unsigned char *sha1, const char *name,
 }
 
 /* returns an fd */
-int read_bundle_header(const char *path, struct bundle_header *header) {
+int read_bundle_header(const char *path, struct bundle_header *header)
+{
 	char buffer[1024];
 	int fd;
 	long fpos;
diff --git a/daemon.c b/daemon.c
index b8df980..41a60af 100644
--- a/daemon.c
+++ b/daemon.c
@@ -406,7 +406,8 @@ static struct daemon_service daemon_service[] = {
 	{ "receive-pack", "receivepack", receive_pack, 0, 1 },
 };
 
-static void enable_service(const char *name, int ena) {
+static void enable_service(const char *name, int ena)
+{
 	int i;
 	for (i = 0; i < ARRAY_SIZE(daemon_service); i++) {
 		if (!strcmp(daemon_service[i].name, name)) {
@@ -417,7 +418,8 @@ static void enable_service(const char *name, int ena) {
 	die("No such service %s", name);
 }
 
-static void make_service_overridable(const char *name, int ena) {
+static void make_service_overridable(const char *name, int ena)
+{
 	int i;
 	for (i = 0; i < ARRAY_SIZE(daemon_service); i++) {
 		if (!strcmp(daemon_service[i].name, name)) {
diff --git a/dir.c b/dir.c
index 5bcc764..01790ab 100644
--- a/dir.c
+++ b/dir.c
@@ -298,7 +298,8 @@ int excluded(struct dir_struct *dir, const char *pathname)
 	return 0;
 }
 
-static struct dir_entry *dir_entry_new(const char *pathname, int len) {
+static struct dir_entry *dir_entry_new(const char *pathname, int len)
+{
 	struct dir_entry *ent;
 
 	ent = xmalloc(sizeof(*ent) + len + 1);
diff --git a/help.c b/help.c
index 855aeef..8217d97 100644
--- a/help.c
+++ b/help.c
@@ -79,7 +79,8 @@ static void uniq(struct cmdnames *cmds)
 	cmds->cnt = j;
 }
 
-static void exclude_cmds(struct cmdnames *cmds, struct cmdnames *excludes) {
+static void exclude_cmds(struct cmdnames *cmds, struct cmdnames *excludes)
+{
 	int ci, cj, ei;
 	int cmp;
 
diff --git a/quote.c b/quote.c
index 919d092..0455783 100644
--- a/quote.c
+++ b/quote.c
@@ -131,7 +131,8 @@ static signed char const sq_lookup[256] = {
 	/* 0x80 */ /* set to 0 */
 };
 
-static inline int sq_must_quote(char c) {
+static inline int sq_must_quote(char c)
+{
 	return sq_lookup[(unsigned char)c] + quote_path_fully > 0;
 }
 
diff --git a/setup.c b/setup.c
index df006d9..1e2c55d 100644
--- a/setup.c
+++ b/setup.c
@@ -206,7 +206,8 @@ static const char *set_work_tree(const char *dir)
 	return NULL;
 }
 
-void setup_work_tree(void) {
+void setup_work_tree(void)
+{
 	const char *work_tree = get_git_work_tree();
 	const char *git_dir = get_git_dir();
 	if (!is_absolute_path(git_dir))
diff --git a/transport.c b/transport.c
index d44fe7c..fa5cfbb 100644
--- a/transport.c
+++ b/transport.c
@@ -380,7 +380,8 @@ static int disconnect_walker(struct transport *transport)
 }
 
 #ifndef NO_CURL
-static int curl_transport_push(struct transport *transport, int refspec_nr, const char **refspec, int flags) {
+static int curl_transport_push(struct transport *transport, int refspec_nr, const char **refspec, int flags)
+{
 	const char **argv;
 	int argc;
 	int err;
@@ -646,7 +647,8 @@ static int fetch_refs_via_pack(struct transport *transport,
 	return 0;
 }
 
-static int git_transport_push(struct transport *transport, int refspec_nr, const char **refspec, int flags) {
+static int git_transport_push(struct transport *transport, int refspec_nr, const char **refspec, int flags)
+{
 	struct git_transport_data *data = transport->data;
 	const char **argv;
 	char *rem;
diff --git a/utf8.c b/utf8.c
index 4efef6f..8095a71 100644
--- a/utf8.c
+++ b/utf8.c
@@ -11,7 +11,8 @@ struct interval {
 };
 
 /* auxiliary function for binary search in interval table */
-static int bisearch(ucs_char_t ucs, const struct interval *table, int max) {
+static int bisearch(ucs_char_t ucs, const struct interval *table, int max)
+{
 	int min = 0;
 	int mid;