Hi,
On Fri, 27 Jul 2007, Junio C Hamano wrote:
> Johannes Schindelin <Johannes.Schindelin@xxxxxx> writes:
>
> > 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...
>
> > 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?
> > @@ -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
-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
