Re: [PATCH 3/8] Clean up work-tree handling

Hi,

On Fri, 27 Jul 2007, Junio C Hamano wrote:

Johannes Schindelin [off-list ref] writes:

quoted

	Not only because of ohloh am I proud that in spite of removing
	more lines than I added, there were more comments added than
	removed...

quoted

diff --git a/builtin-rev-parse.c b/builtin-rev-parse.c
index 497903a..3f787a8 100644
--- a/builtin-rev-parse.c
+++ b/builtin-rev-parse.c

@@ -320,15 +320,9 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)
 				continue;
 			}
 			if (!strcmp(arg, "--show-cdup")) {
-				const char *pfx = prefix;
-				while (pfx) {
-					pfx = strchr(pfx, '/');
-					if (pfx) {
-						pfx++;
-						printf("../");
-					}
-				}
-				putchar('\n');
+				const char *work_tree = get_git_work_tree();
+				if (work_tree)
+					printf("%s\n", work_tree);
 				continue;

This changes semantics, I think.

It used to be relative "up" path when no funny work-tree stuff
is used, but get_git_work_tree() now seems to return absolute,
hence this option as well.  If it introduces regression to
existing callers is up to what the caller does to the resulting
path, though.  If it only is used to prefix other things
(i.e. path="$(git rev-parse --show-cdup)$1"), the caller would
be safe, but if the caller counted number of ../ in the return
value to see how deep it is, or if the caller expected to see
empty string in order to see if the process is at the toplevel,
this change would become a regression.

I am somewhat negative on keeping _that_ much backwards compatibility.  
Scripts which depend on show-cdup being a relative path _will_ be broken 
by work-tree.  Is it worth it to detect those errors late?

quoted

@@ -62,15 +66,8 @@ static void setup_git_env(void)
 
 int is_bare_repository(void)
 {
+	/* if core.bare is not 'false', let's see if there is a work tree */
+	return is_bare_repository_cfg && !get_git_work_tree();
 }

I thought about making core.bare a tertiary, true/false/depends,
but I think this makes more sense.

Actually, you made me think again, and I am more along those lines now:

	return is_bare_repository_cfg >= 0 ?
		is_bare_repository_cfg : !get_git_work_tree();

and according patch to get_git_work_tree to return NULL if 
is_bare_repository_cfg == 1.

Ciao,
Dscho