[PATCH v3] Support ent:relative_path

[PATCH v3] Support ent:relative_path

[Dana How]

Most commands accept relative paths,  but this is
not true of arguments in ent:path format.  This
patch makes all of the following git-show commands
work in the git source tree (not just the first):
 % cd xdiff
 % git-show v1.5.2-rc0:xdiff/xemit.h
 % git-show v1.5.2-rc0:./xemit.h
 % git-show v1.5.2-rc0:../sha1_name.c

It also adds ent:?string as a synonym for ent:/string .
This makes the following changes possible later:
ent:/path is an absolute path and ent:path is relative.

Signed-off-by: Dana L. How <danahow@gmail.com>
---
 cache.h     |    1 +
 setup.c     |    5 +++-
 sha1_name.c |   78 +++++++++++++++++++++++++++++++++++++++++++++++++++++++---
 3 files changed, 79 insertions(+), 5 deletions(-)

diff --git a/cache.h b/cache.h
index 8e76152..53507d9 100644
--- a/cache.h
+++ b/cache.h
@@ -215,6 +215,7 @@ extern char *get_graft_file(void);
 
 #define ALTERNATE_DB_ENVIRONMENT "GIT_ALTERNATE_OBJECT_DIRECTORIES"
 
+extern const char *prefix_to_cwd;
 extern const char **get_pathspec(const char *prefix, const char **pathspec);
 extern const char *setup_git_directory_gently(int *);
 extern const char *setup_git_directory(void);
diff --git a/setup.c b/setup.c
index a45ea83..46ae6e3 100644
--- a/setup.c
+++ b/setup.c
@@ -1,5 +1,7 @@
 #include "cache.h"
 
+const char *prefix_to_cwd;
+
 const char *prefix_path(const char *prefix, int len, const char *path)
 {
 	const char *orig = path;
@@ -252,7 +254,8 @@ const char *setup_git_directory_gently(int *nongit_ok)
 	cwd[len++] = '/';
 	cwd[len] = 0;
 	inside_git_dir = !prefixcmp(cwd + offset, ".git/");
-	return cwd + offset;
+	prefix_to_cwd = cwd + offset;
+	return prefix_to_cwd;
 }
 
 int git_config_perm(const char *var, const char *value)
diff --git a/sha1_name.c b/sha1_name.c
index 55f25a2..dd415c1 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -593,6 +593,67 @@ static int handle_one_ref(const char *path,
 }
 
 /*
+ * Future: nonzero to assume relative paths
+ */
+#define	ENT_COLON_ASSUME_RELATIVE	0
+
+/*
+ * Modify path into a full absolute path with no leading "/"
+ * and no "." or "..".  If we can't,
+ * leave input unaltered for client's error message.
+ */
+static void prepend_prefix(const char **cp, int *namelen)
+{
+	char *t, *t2;
+	const char *cp2 = *cp;
+	int type, namelen2 = *namelen;
+	static char fullpath[PATH_MAX];
+	int fixup = strstr(cp2, "/./") || strstr(cp2, "/../");
+	for (type = 4; --type > 0; )
+		if (namelen2 > type && !memcmp(cp2, "../" + 3 - type, type))
+			break;
+
+	/* handle simple cases else create absolute path */
+	fullpath[0] = 0;
+	if ((type == 0 && !ENT_COLON_ASSUME_RELATIVE) || type == 1) {
+		if (!fixup) goto done;
+	} else
+	if (prefix_to_cwd) {
+		namelen2 += strlen(prefix_to_cwd);
+		if (namelen2 >= PATH_MAX)
+			die("path too long");
+		strcpy(fullpath, prefix_to_cwd);
+		type = 0;
+	} else
+	if (type == 2) {
+		if (!fixup) goto done;
+	} else
+	if (type == 3)
+		return;	/* client will complain */
+	cp2 = strcat(fullpath, cp2);
+
+	while ((t = strstr(cp2, "/./")) != NULL)
+		memmove(t, t + 2, (namelen2 -= 2) - (t - cp2) + 1);
+
+	while ((t = strstr(cp2, "/../")) != NULL) {
+		if (t == cp2 || (t == cp2 + 1 && t[-1] == '.'))
+			return;	/* client will complain */
+		for (t2 = t; --t2 >= cp2 && *t2 != '/'; )
+			/* nothing */;
+		if (t2 < cp2) {
+			namelen2 -= t + 4 - cp2;
+			cp2 = t + 4;
+		} else {
+			memmove(t2, t + 3, namelen2 - (t + 3 - cp2) + 1);
+			namelen2 -= t + 3 - t2;
+		}
+	}
+
+done:	*cp = cp2 + type;
+	*namelen = namelen2 - type;
+}
+
+/*
  * This interprets names like ':/Initial revision of "git"' by searching
  * through history and returning the first commit whose message starts
  * with the given string.
@@ -647,6 +708,11 @@ int get_sha1(const char *name, unsigned char *sha1)
 	return get_sha1_with_mode(name, sha1, &unused);
 }
 
+/*
+ * Future: change to '?'
+ */
+#define	ENT_COLON_OLD_SEARCH	'/'
+
 int get_sha1_with_mode(const char *name, unsigned char *sha1, unsigned *mode)
 {
 	int ret, bracket_depth;
@@ -666,7 +732,8 @@ int get_sha1_with_mode(const char *name, unsigned char *sha1, unsigned *mode)
 		int stage = 0;
 		struct cache_entry *ce;
 		int pos;
-		if (namelen > 2 && name[1] == '/')
+		if (namelen > 2 &&
+		    (name[1] == ENT_COLON_OLD_SEARCH || name[1] == '?'))
 			return get_sha1_oneline(name + 2, sha1);
 		if (namelen < 3 ||
 		    name[2] != ':' ||
@@ -681,6 +748,7 @@ int get_sha1_with_mode(const char *name, unsigned char *sha1, unsigned *mode)
 			read_cache();
 		if (active_nr < 0)
 			return -1;
+		prepend_prefix(&cp, &namelen);
 		pos = cache_name_pos(cp, namelen);
 		if (pos < 0)
 			pos = -pos - 1;
@@ -708,9 +776,11 @@ int get_sha1_with_mode(const char *name, unsigned char *sha1, unsigned *mode)
 	}
 	if (*cp == ':') {
 		unsigned char tree_sha1[20];
-		if (!get_sha1_1(name, cp-name, tree_sha1))
-			return get_tree_entry(tree_sha1, cp+1, sha1,
-					      mode);
+		if (!get_sha1_1(name, cp - name, tree_sha1)) {
+			namelen -= ++cp - name;
+			prepend_prefix(&cp, &namelen);
+			return get_tree_entry(tree_sha1, cp, sha1, mode);
+		}
 	}
 	return ret;
 }
-- 
1.5.2.rc0.787.g0014

Re: [PATCH v3] Support ent:relative_path

[Jakub Narebski]

Dana How wrote:

> Most commands accept relative paths,  but this is
> not true of arguments in ent:path format.  This
> patch makes all of the following git-show commands
> work in the git source tree (not just the first):
>
>  % cd xdiff
>  % git-show v1.5.2-rc0:xdiff/xemit.h
>  % git-show v1.5.2-rc0:./xemit.h
>  % git-show v1.5.2-rc0:../sha1_name.c
> 
> It also adds ent:?string as a synonym for ent:/string .
> This makes the following changes possible later:
> ent:/path is an absolute path and ent:path is relative.

First, we now usually use 'tree-ish' or 'treeish', not 'ent'.
Second, it is ':/<text>', not '<tree-ish>:/<text>'

git-rev-parse(1):

 * A colon, followed by a slash, followed by a text: this names
   a commit whose commit message starts with the specified text.
   This name returns the youngest matching commit which is
   reachable from any ref.  If the commit message starts with a
   '!', you have to repeat that;  the special sequence ':/!',
   followed by something else than '!' is reserved for now.

-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git

Re: [PATCH v3] Support ent:relative_path

[Johannes Schindelin]

Hi,

On Fri, 4 May 2007, Dana How wrote:

> Most commands accept relative paths,  but this is
> not true of arguments in ent:path format.  This
> patch makes all of the following git-show commands
> work in the git source tree (not just the first):
>  % cd xdiff
>  % git-show v1.5.2-rc0:xdiff/xemit.h
>  % git-show v1.5.2-rc0:./xemit.h
>  % git-show v1.5.2-rc0:../sha1_name.c

ACK.

> It also adds ent:?string as a synonym for ent:/string .
> This makes the following changes possible later:
> ent:/path is an absolute path and ent:path is relative.

NACK. The "?" thing is highly unintuitive, and I still do not think that 
it is worth the confusion to make Git behave funny on 
<tree>:<file-name>.

Think of the "todo" branch, for example. And this is not even far fetched. 
In many repositories I have to have separate related, but non-branch 
HEADs. For example, I track some projects with custom scripts. These 
scripts do _not_ belong into that branch. However, they are related, so I 
store them in the same repository.

Plus, my example of the bare-repository still has not been answered by 
_anyone_ in favour of changing the current behaviour.

Ciao,
Dscho

Re: [PATCH v3] Support ent:relative_path

[Shawn O. Pearce]

Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> On Fri, 4 May 2007, Dana How wrote:
> 
> > Most commands accept relative paths,  but this is
> > not true of arguments in ent:path format.  This
> > patch makes all of the following git-show commands
> > work in the git source tree (not just the first):
> >  % cd xdiff
> >  % git-show v1.5.2-rc0:xdiff/xemit.h
> >  % git-show v1.5.2-rc0:./xemit.h
> >  % git-show v1.5.2-rc0:../sha1_name.c
> 
> ACK.

Double ACK.  I'm in complete agreement with every point Dscho has
made in this thread, even if I haven't quoted it.  ;-)

I *really* do not want to see "<tree-ish>:<path>" to mean include
the current prefix, *especially* when a bare repository is involved.

I often either do ad-hoc git-show lines against bare repositories,
or have scripts that depend on this existing absolute path behavior.
I'd like to see those not break.  ;-)

Since "." and ".." should not be valid names in any tree of a Git
repository I think we're OK to say that ":./" and ":../" are to
imply relative to current prefix.  But if we do ":../" then we also
have to do ":../../../../../../.." ;-)

-- 
Shawn.

Re: [PATCH v3] Support ent:relative_path

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> Think of the "todo" branch, for example. And this is not even far fetched. 
> In many repositories I have to have separate related, but non-branch 
> HEADs. For example, I track some projects with custom scripts. These 
> scripts do _not_ belong into that branch. However, they are related, so I 
> store them in the same repository.

I said I won't talk about it until 1.5.2 final, but you forced
me into it.

> Plus, my example of the bare-repository still has not been answered by 
> _anyone_ in favour of changing the current behaviour.

I am not sure if there is anything to answer on this one.

When you are talking to a bare-repository, your place relative
to the root of the working tree is clearly undefined, and it is
natural that "<tree-ish>:./<path>", "<tree-ish>:/<path>" and
"<tree-ish>:<path>" cannot mean anything but relative to the
root of the tree-ish.  "<tree-ish>:../<path>" is obviously
nonsense.  So it does not matter if unadorned <path> is relative
to cwd or root in this case.

Your other example, however, gives a much better illustration.
An unrelated 'todo' branch where your cwd does not have any
relevance to the contents of that tree-ish, or worse yet, a
commit from git-gui project in git.git repository, where taking
your cwd into account has an actively wrong effect, demonstrates
why we would need a way to say "By this path, I mean from the
top, I do not want you to take it as relative to where I am".

One way to ensure that is to keep the current "it is a path from
the top" behaviour, and extended it with "... unless it begins
with ./".  Doing this forever however penalizes the case where
you want to use relative paths by requiring ./ at the beginning.

Another is to do the usual POSIXy path interpretation and
"unless it begins with /, it is taken as relative to where you
are".  This penalizes the 'todo' and git-gui commit use case
because the user explicitly needs to say "where I am does not
matter" by prefixing the path with '/', and also necessitates a
change to the syntax for looking backwards for a commit with
that message, because the existing syntax to look for a string
clashes with it [*1*].

Both have merits and demerits.  If we did not have any existing
code and users, the latter is clearly what we would have done,
as it is more consistent.  The path handling feels more natural
(in line with the way we expect paths to be handled on POSIX
systems), the "look backwards" search feels more natural ( you
use '/' for forward search, '?' for backwards).

I also suspect the latter is more often convenient.  When
working on a flat project, it does not matter if the default is
relative to cwd or to the root.  But if your project is deep,
and if you somehow do "git show" more often than "git diff" (I
don't, but different people may do so for different reasons), it
would start to hurt if you always have to say "./".

It is however clearly a bigger change to existing users.
Correcting earlier mistakes is painful, so it certainly is
tempting to take the approach that the path is always absolute
and require "./" for relative.  I agree it is an easier change,
but I am not convinced yet that it is the right design in the
longer term.

[Footnote]

*1* This is only true for looking for a path in the index case,
as <commit>:/<string> does not seem to work.  I think this is a
bug in the current code -- shouldn't it limit the search to
commits that are reachable from that named one?

Re: [PATCH v3] Support ent:relative_path

[Dana How]

Thanks for your reply.  I can wait for 1.5.2.
So I will _quickly_ address some other points JS and SP mentioned.

(a) In a bare repository, I believe setup.c:setup_git_directory_gently()
determines the prefix to be NULL.  This means my patch will see
ALL paths as absolute,  except :../path which will result in an error.

(b) For :path and :stage:path , relative is useful for me.

(c) For <commit-ish>:path ,  again relative is useful for me.
All my examples were of this form.

(d) Where <tree-ish> in <tree-ish>:path is just a tree and no
commit is involved, relative paths make no sense since you don't
know where the tree is. (Imagine it moved between commits.)
So don't call my prepend_prefix() or call it with prefix faked to NULL.

Unfortunately I was never clear about (a) and my patch fails
to discriminate between (c) and (d).  I'll try to fix the latter later.

Thanks

Dana

On 5/4/07, Junio C Hamano <junkio@cox.net> wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
> > Think of the "todo" branch, for example. And this is not even far fetched.
> > In many repositories I have to have separate related, but non-branch
> > HEADs. For example, I track some projects with custom scripts. These
> > scripts do _not_ belong into that branch. However, they are related, so I
> > store them in the same repository.
>
> I said I won't talk about it until 1.5.2 final, but you forced
> me into it.
>
> > Plus, my example of the bare-repository still has not been answered by
> > _anyone_ in favour of changing the current behaviour.
>
> I am not sure if there is anything to answer on this one.
>
> When you are talking to a bare-repository, your place relative
> to the root of the working tree is clearly undefined, and it is
> natural that "<tree-ish>:./<path>", "<tree-ish>:/<path>" and
> "<tree-ish>:<path>" cannot mean anything but relative to the
> root of the tree-ish.  "<tree-ish>:../<path>" is obviously
> nonsense.  So it does not matter if unadorned <path> is relative
> to cwd or root in this case.
>
> Your other example, however, gives a much better illustration.
> An unrelated 'todo' branch where your cwd does not have any
> relevance to the contents of that tree-ish, or worse yet, a
> commit from git-gui project in git.git repository, where taking
> your cwd into account has an actively wrong effect, demonstrates
> why we would need a way to say "By this path, I mean from the
> top, I do not want you to take it as relative to where I am".
>
> One way to ensure that is to keep the current "it is a path from
> the top" behaviour, and extended it with "... unless it begins
> with ./".  Doing this forever however penalizes the case where
> you want to use relative paths by requiring ./ at the beginning.
>
> Another is to do the usual POSIXy path interpretation and
> "unless it begins with /, it is taken as relative to where you
> are".  This penalizes the 'todo' and git-gui commit use case
> because the user explicitly needs to say "where I am does not
> matter" by prefixing the path with '/', and also necessitates a
> change to the syntax for looking backwards for a commit with
> that message, because the existing syntax to look for a string
> clashes with it [*1*].
>
> Both have merits and demerits.  If we did not have any existing
> code and users, the latter is clearly what we would have done,
> as it is more consistent.  The path handling feels more natural
> (in line with the way we expect paths to be handled on POSIX
> systems), the "look backwards" search feels more natural ( you
> use '/' for forward search, '?' for backwards).
>
> I also suspect the latter is more often convenient.  When
> working on a flat project, it does not matter if the default is
> relative to cwd or to the root.  But if your project is deep,
> and if you somehow do "git show" more often than "git diff" (I
> don't, but different people may do so for different reasons), it
> would start to hurt if you always have to say "./".
>
> It is however clearly a bigger change to existing users.
> Correcting earlier mistakes is painful, so it certainly is
> tempting to take the approach that the path is always absolute
> and require "./" for relative.  I agree it is an easier change,
> but I am not convinced yet that it is the right design in the
> longer term.
>
> [Footnote]
>
> *1* This is only true for looking for a path in the index case,
> as <commit>:/<string> does not seem to work.  I think this is a
> bug in the current code -- shouldn't it limit the search to
> commits that are reachable from that named one?

-- 
Dana L. How  danahow@gmail.com  +1 650 804 5991 cell

Re: [PATCH v3] Support ent:relative_path

[Alex Riesen]

Dana How, Sat, May 05, 2007 08:46:46 +0200:
> Thanks for your reply.  I can wait for 1.5.2.

Actually, you (and everyone who likes your patch and the proposed
semantics) can wait indefinitely. Just keep the patch in a side branch
and rebase it or merge with Junio's master occasionally.
I believe many of us have some features they keep in their repos which
they never send upstream (for their own reasons. I customized some
warnings and added printing of ip addresses to git-fetch. Not
interesting for everyone, but is useful next time kernel.org has DNS
problems). Just become the maintainer of the feature.

Re: [PATCH v3] Support ent:relative_path

[Johannes Schindelin]

Hi,

On Fri, 4 May 2007, Dana How wrote:

> (a) In a bare repository, I believe setup.c:setup_git_directory_gently()
> determines the prefix to be NULL.  This means my patch will see
> ALL paths as absolute,  except :../path which will result in an error.

My point was that it feels inconsistent to take the current path into 
account in one case, but not in the other.

Ciao,
Dscho

Re: [PATCH v3] Support ent:relative_path

[Martin Waitz]

[-- Attachment #1: Type: text/plain, Size: 1351 bytes --]

On Fri, May 04, 2007 at 11:30:39PM -0400, Shawn O. Pearce wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> > On Fri, 4 May 2007, Dana How wrote:
> > 
> > > Most commands accept relative paths,  but this is
> > > not true of arguments in ent:path format.  This
> > > patch makes all of the following git-show commands
> > > work in the git source tree (not just the first):
> > >  % cd xdiff
> > >  % git-show v1.5.2-rc0:xdiff/xemit.h
> > >  % git-show v1.5.2-rc0:./xemit.h
> > >  % git-show v1.5.2-rc0:../sha1_name.c
> > 
> > ACK.
> 
> Double ACK.  I'm in complete agreement with every point Dscho has
> made in this thread, even if I haven't quoted it.  ;-)
> 
> I *really* do not want to see "<tree-ish>:<path>" to mean include
> the current prefix, *especially* when a bare repository is involved.

we could also introduce "<tree-ish>/<path>" for absolute path entries.
This would make a lot of sense for submodules:

	git show "<tree-ish>/path/to/submodule/path/inside/submodule

with only : it would not look that nice ;-)
OK, we could simply say that submodules are special and that you
don't have to use : to separate a commit (the submodule entry) from
the path inside.


aside from that I would also really like to have both absolute and
relative ways to name objects.

-- 
Martin Waitz

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH v3] Support ent:relative_path

[Junio C Hamano]

Martin Waitz <tali@admingilde.org> writes:

> we could also introduce "<tree-ish>/<path>" for absolute path entries.

When you name the tree-ish with usual "branch name", where does
the branch name end and pathname start?  What happens when there
is an ambiguity, and how costly to detect such an ambiguity to
begin with?

Re: [PATCH v3] Support ent:relative_path

[Alex Riesen]

Martin Waitz, Sat, May 05, 2007 21:32:14 +0200:
> we could also introduce "<tree-ish>/<path>" for absolute path entries.

"012345/abc/def"

Is it a relative path, or a tree-ish + absolute path?

Re: [PATCH v3] Support ent:relative_path

[Martin Waitz]

[-- Attachment #1: Type: text/plain, Size: 692 bytes --]

hoi :)

On Sat, May 05, 2007 at 01:17:35PM -0700, Junio C Hamano wrote:
> > we could also introduce "<tree-ish>/<path>" for absolute path entries.
> 
> When you name the tree-ish with usual "branch name", where does
> the branch name end and pathname start?  What happens when there
> is an ambiguity, and how costly to detect such an ambiguity to
> begin with?

well, if you know that it starts with a tree-ish there is no
ambiguity as there can't be any "a/b" tree-ish if there already is one
named "a".  But Alex is right, it can be confused with an relative
path if e.g. git-log is used without "--".

But for submodules I'd really like the / syntax.

-- 
Martin Waitz

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH v3] Support ent:relative_path

[Johannes Schindelin]

Hi,

On Sat, 5 May 2007, Martin Waitz wrote:

> hoi :)
> 
> On Sat, May 05, 2007 at 01:17:35PM -0700, Junio C Hamano wrote:
> > > we could also introduce "<tree-ish>/<path>" for absolute path entries.
> > 
> > When you name the tree-ish with usual "branch name", where does
> > the branch name end and pathname start?  What happens when there
> > is an ambiguity, and how costly to detect such an ambiguity to
> > begin with?
> 
> well, if you know that it starts with a tree-ish there is no
> ambiguity [...]

Wrong. For example, mw/submodules~10 _is_ a tree-ish (if you have a branch 
named "mw/submodules").

Hth,
Dscho

Re: [PATCH v3] Support ent:relative_path

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

>> (a) In a bare repository, I believe setup.c:setup_git_directory_gently()
>> determines the prefix to be NULL.  This means my patch will see
>> ALL paths as absolute,  except :../path which will result in an error.
>
> My point was that it feels inconsistent to take the current path into 
> account in one case, but not in the other.

I do not understand your reasoning.  In a bare repository you
cannot even be in a subdirectory to begin with.

Re: [PATCH v3] Support ent:relative_path

[Johannes Schindelin]

Hi,

On Sat, 5 May 2007, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> >> (a) In a bare repository, I believe 
> >> setup.c:setup_git_directory_gently() determines the prefix to be 
> >> NULL.  This means my patch will see ALL paths as absolute, except 
> >> :../path which will result in an error.
> >
> > My point was that it feels inconsistent to take the current path into 
> > account in one case, but not in the other.
> 
> I do not understand your reasoning.  In a bare repository you cannot 
> even be in a subdirectory to begin with.

Exactly! That is my point. If you can do it in a working directory, but 
also with a bare repository, I find it highly confusing and inconsistent 
to have different meaning.

Ciao,
Dscho

Re: [PATCH v3] Support ent:relative_path

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> On Sat, 5 May 2007, Junio C Hamano wrote:
>
>> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>> 
>> >> (a) In a bare repository, I believe 
>> >> setup.c:setup_git_directory_gently() determines the prefix to be 
>> >> NULL.  This means my patch will see ALL paths as absolute, except 
>> >> :../path which will result in an error.
>> >
>> > My point was that it feels inconsistent to take the current path into 
>> > account in one case, but not in the other.
>> 
>> I do not understand your reasoning.  In a bare repository you cannot 
>> even be in a subdirectory to begin with.
>
> Exactly! That is my point. If you can do it in a working directory, but 
> also with a bare repository, I find it highly confusing and inconsistent 
> to have different meaning.

Sorry.  Now you confused me further.  I can do:

	cd Documentation
        git diff v1.5.0 v1.5.1 -- git.txt

Is that confusing, inconsistent and bad for the users?

Re: [PATCH v3] Support ent:relative_path

[Martin Waitz]

[-- Attachment #1: Type: text/plain, Size: 820 bytes --]

hoi :)

On Sun, May 06, 2007 at 02:59:55AM +0200, Johannes Schindelin wrote:
> > On Sat, May 05, 2007 at 01:17:35PM -0700, Junio C Hamano wrote:
> > > > we could also introduce "<tree-ish>/<path>" for absolute path entries.
> > > 
> > > When you name the tree-ish with usual "branch name", where does
> > > the branch name end and pathname start?  What happens when there
> > > is an ambiguity, and how costly to detect such an ambiguity to
> > > begin with?
> > 
> > well, if you know that it starts with a tree-ish there is no
> > ambiguity [...]
> 
> Wrong. For example, mw/submodules~10 _is_ a tree-ish (if you have a branch 
> named "mw/submodules").

so what?

a better argument is that one can have both refs/tags/a and
refs/heads/a/b.

So just forget my suggestion...

-- 
Martin Waitz

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH v3] Support ent:relative_path

[Johannes Schindelin]

Hi,

On Sun, 6 May 2007, Martin Waitz wrote:

> On Sun, May 06, 2007 at 02:59:55AM +0200, Johannes Schindelin wrote:
> > > On Sat, May 05, 2007 at 01:17:35PM -0700, Junio C Hamano wrote:
> > > > > we could also introduce "<tree-ish>/<path>" for absolute path entries.
> > > > 
> > > > When you name the tree-ish with usual "branch name", where does
> > > > the branch name end and pathname start?  What happens when there
> > > > is an ambiguity, and how costly to detect such an ambiguity to
> > > > begin with?
> > > 
> > > well, if you know that it starts with a tree-ish there is no
> > > ambiguity [...]
> > 
> > Wrong. For example, mw/submodules~10 _is_ a tree-ish (if you have a branch 
> > named "mw/submodules").
> 
> so what?

So what: it proves that your case is wrong. mw/submodules~10/README would 
be _severely_ confused.

Ciao,
Dscho

Re: [PATCH v3] Support ent:relative_path

[Johannes Schindelin]

Hi,

On Sun, 6 May 2007, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > On Sat, 5 May 2007, Junio C Hamano wrote:
> >
> >> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> >> 
> >> >> (a) In a bare repository, I believe 
> >> >> setup.c:setup_git_directory_gently() determines the prefix to be 
> >> >> NULL.  This means my patch will see ALL paths as absolute, except 
> >> >> :../path which will result in an error.
> >> >
> >> > My point was that it feels inconsistent to take the current path into 
> >> > account in one case, but not in the other.
> >> 
> >> I do not understand your reasoning.  In a bare repository you cannot 
> >> even be in a subdirectory to begin with.
> >
> > Exactly! That is my point. If you can do it in a working directory, but 
> > also with a bare repository, I find it highly confusing and inconsistent 
> > to have different meaning.
> 
> Sorry.  Now you confused me further.  I can do:
> 
> 	cd Documentation
>         git diff v1.5.0 v1.5.1 -- git.txt
> 
> Is that confusing, inconsistent and bad for the users?

Well, I am partly at fault that you _can_ execute git-diff outside of a 
repository.

But given the _arguments_ you give to git-diff as above, I'd expect it to 
actually care about the working directory. IOW I would _not_ expect this 
to work outside of a working directory (even if it does).

Ciao,
Dscho

Re: [PATCH v3] Support ent:relative_path

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> On Sun, 6 May 2007, Junio C Hamano wrote:
>
>> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>> 
>> > On Sat, 5 May 2007, Junio C Hamano wrote:
>> >
>> >> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>> >> 
>> >> >> (a) In a bare repository, I believe 
>> >> >> setup.c:setup_git_directory_gently() determines the prefix to be 
>> >> >> NULL.  This means my patch will see ALL paths as absolute, except 
>> >> >> :../path which will result in an error.
>> >> >
>> >> > My point was that it feels inconsistent to take the current path into 
>> >> > account in one case, but not in the other.
>> >> 
>> >> I do not understand your reasoning.  In a bare repository you cannot 
>> >> even be in a subdirectory to begin with.
>> >
>> > Exactly! That is my point. If you can do it in a working directory, but 
>> > also with a bare repository, I find it highly confusing and inconsistent 
>> > to have different meaning.
>> 
>> Sorry.  Now you confused me further.  I can do:
>> 
>> 	cd Documentation
>>         git diff v1.5.0 v1.5.1 -- git.txt
>> 
>> Is that confusing, inconsistent and bad for the users?
>
> Well, I am partly at fault that you _can_ execute git-diff outside of a 
> repository.
>
> But given the _arguments_ you give to git-diff as above, I'd expect it to 
> actually care about the working directory. IOW I would _not_ expect this 
> to work outside of a working directory (even if it does).

Oh, I was not thinking about "outside of repository" use.  I was
talking about your earlier "bare repository vs inside worktree
vs inside a subdirectory of worktree" point.  In a bare
repository,

	git diff v1.5.0 v1.5.1 -- Documentation/git.txt

is the only form that makes sense, as you cannot say "I am
interested in Documentation/" by _being_ in that subdirectory.
In a repository with worktree, you can, and we let you do so.

I would expect v1.5.0:Documentation/git.txt notation would be
the only sane variant that would make sense to name that blob in
a bare repository for the same reason.  I do not expect anybody
to complain because we do not allow him to say v1.5.0:git.txt in
a bare repository, either.

Also I sympathize with people who would wish to (eventually) be
able to do:

	$ cd Documentation/
	$ git show v1.5.0:git.txt

in a repository with worktree, by making the "relative path" the
default behaviour.  They would need to do either one of:

	$ git show v1.5.0:/git.c
	$ git show v1.5.0:../git.c

if we ever made the "relative path" the default.  As long as you
make sure that you make:

	$ git show v1.5.0:/git.c

work the same way in a bare repository _if_ we make the
"relative path" the default, I do not see any inconsistency
problem there.

A bare repository and a repository with working tree are
different.  In the former, you cannot say "I am interested in
this subtree" by _being_ in a subdirectory; in the latter you
can.  Taking advantage of that and allowing the user to express
himself better (only) in the latter is not an inconsistency.
Not being able to do that in a bare repository comes from what a
bare repository inherently is.

Re: [PATCH v3] Support ent:relative_path

[Dana How]

On 5/6/07, Junio C Hamano <junkio@cox.net> wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
> > On Sun, 6 May 2007, Junio C Hamano wrote:
> >
> >> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> >>
> >> > On Sat, 5 May 2007, Junio C Hamano wrote:
> >> >
> >> >> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> >> >>
> >> >> >> (a) In a bare repository, I believe
> >> >> >> setup.c:setup_git_directory_gently() determines the prefix to be
> >> >> >> NULL.  This means my patch will see ALL paths as absolute, except
> >> >> >> :../path which will result in an error.
> >> >> >
> >> >> > My point was that it feels inconsistent to take the current path into
> >> >> > account in one case, but not in the other.
> >> >>
> >> >> I do not understand your reasoning.  In a bare repository you cannot
> >> >> even be in a subdirectory to begin with.
> >> >
> >> > Exactly! That is my point. If you can do it in a working directory, but
> >> > also with a bare repository, I find it highly confusing and inconsistent
> >> > to have different meaning.
> >>
> >> Sorry.  Now you confused me further.  I can do:
> >>
> >>      cd Documentation
> >>         git diff v1.5.0 v1.5.1 -- git.txt
> >>
> >> Is that confusing, inconsistent and bad for the users?
> >
> > Well, I am partly at fault that you _can_ execute git-diff outside of a
> > repository.
> >
> > But given the _arguments_ you give to git-diff as above, I'd expect it to
> > actually care about the working directory. IOW I would _not_ expect this
> > to work outside of a working directory (even if it does).
>
> Oh, I was not thinking about "outside of repository" use.  I was
> talking about your earlier "bare repository vs inside worktree
> vs inside a subdirectory of worktree" point.  In a bare
> repository,
>
>         git diff v1.5.0 v1.5.1 -- Documentation/git.txt
>
> is the only form that makes sense, as you cannot say "I am
> interested in Documentation/" by _being_ in that subdirectory.
> In a repository with worktree, you can, and we let you do so.
>
> I would expect v1.5.0:Documentation/git.txt notation would be
> the only sane variant that would make sense to name that blob in
> a bare repository for the same reason.  I do not expect anybody
> to complain because we do not allow him to say v1.5.0:git.txt in
> a bare repository, either.
>
> Also I sympathize with people who would wish to (eventually) be
> able to do:
>
>         $ cd Documentation/
>         $ git show v1.5.0:git.txt
>
> in a repository with worktree, by making the "relative path" the
> default behaviour.  They would need to do either one of:
>
>         $ git show v1.5.0:/git.c
>         $ git show v1.5.0:../git.c
>
> if we ever made the "relative path" the default.  As long as you
> make sure that you make:
>
>         $ git show v1.5.0:/git.c
>
> work the same way in a bare repository _if_ we make the
> "relative path" the default, I do not see any inconsistency
> problem there.
>
> A bare repository and a repository with working tree are
> different.  In the former, you cannot say "I am interested in
> this subtree" by _being_ in a subdirectory; in the latter you
> can.  Taking advantage of that and allowing the user to express
> himself better (only) in the latter is not an inconsistency.
> Not being able to do that in a bare repository comes from what a
> bare repository inherently is.

Each of the examples given already works as described above
in the current patch [if you change one #define to enable
"relative is default"].  I will get around to fixing the one
error I described elsewhere and re-submit.

Thanks,
-- 
Dana L. How  danahow@gmail.com  +1 650 804 5991 cell