All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/3] Support relative path in <blah>:path syntax
@ 2010-11-11 14:08 Nguyễn Thái Ngọc Duy
  2010-11-11 14:08 ` [PATCH 1/3] setup: save prefix (original cwd relative to toplevel) in startup_info Nguyễn Thái Ngọc Duy
                   ` (3 more replies)
  0 siblings, 4 replies; 12+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2010-11-11 14:08 UTC (permalink / raw)
  To: git, Junio C Hamano, Jonathan Niedier, Matthieu.Moy
  Cc: Nguyễn Thái Ngọc Duy

Sorry Jonathan I lied. I did not pick up your fast-import changes.
Could not find how to test it. And it seems fast-import only cares
about commits, not the target audience of this syntax.

Document is not updated because I think it's intuitive enough.

Nguyễn Thái Ngọc Duy (3):
  setup: save prefix (original cwd relative to toplevel) in
    startup_info
  Make prefix_path() return char* without const
  get_sha1: support relative path ":path" syntax

 cache.h                        |    3 +-
 setup.c                        |    6 ++-
 sha1_name.c                    |   37 ++++++++++++++++++++++--
 t/t1506-rev-parse-diagnosis.sh |   62 ++++++++++++++++++++++++++++++++++++++++
 4 files changed, 102 insertions(+), 6 deletions(-)

-- 
1.7.3.2.210.g045198

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 1/3] setup: save prefix (original cwd relative to toplevel) in startup_info
  2010-11-11 14:08 [PATCH 0/3] Support relative path in <blah>:path syntax Nguyễn Thái Ngọc Duy
@ 2010-11-11 14:08 ` Nguyễn Thái Ngọc Duy
  2010-11-11 14:08 ` [PATCH 2/3] Make prefix_path() return char* without const Nguyễn Thái Ngọc Duy
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 12+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2010-11-11 14:08 UTC (permalink / raw)
  To: git, Junio C Hamano, Jonathan Niedier, Matthieu.Moy
  Cc: Nguyễn Thái Ngọc Duy, Jonathan Nieder, Junio C Hamano

Save the path from the original cwd to the cwd at the end of the
setup procedure in the startup_info struct introduced in e37c1329
(2010-08-05).  The value cannot vary from thread to thread anyway,
since the cwd is global.

So now in your builtin command, instead of passing prefix around,
when you want to convert a user-supplied path to a cwd-relative
path, you can use startup_info->prefix directly.

Caveat: As with the return value from setup_git_directory_gently(),
startup_info->prefix would be NULL when the original cwd is not a
subdir of the toplevel.

Longer term, this woiuld allow the prefix to be reused when several
noncooperating functions require access to the same repository (for
example, when accessing configuration before running a builtin).

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 cache.h |    1 +
 setup.c |    4 +++-
 2 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/cache.h b/cache.h
index 33decd9..222d9cf 100644
--- a/cache.h
+++ b/cache.h
@@ -1117,6 +1117,7 @@ const char *split_cmdline_strerror(int cmdline_errno);
 /* git.c */
 struct startup_info {
 	int have_repository;
+	const char *prefix;
 };
 extern struct startup_info *startup_info;
 
diff --git a/setup.c b/setup.c
index a3b76de..833db12 100644
--- a/setup.c
+++ b/setup.c
@@ -512,8 +512,10 @@ const char *setup_git_directory_gently(int *nongit_ok)
 	const char *prefix;
 
 	prefix = setup_git_directory_gently_1(nongit_ok);
-	if (startup_info)
+	if (startup_info) {
 		startup_info->have_repository = !nongit_ok || !*nongit_ok;
+		startup_info->prefix = prefix;
+	}
 	return prefix;
 }
 
-- 
1.7.3.2.210.g045198

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 2/3] Make prefix_path() return char* without const
  2010-11-11 14:08 [PATCH 0/3] Support relative path in <blah>:path syntax Nguyễn Thái Ngọc Duy
  2010-11-11 14:08 ` [PATCH 1/3] setup: save prefix (original cwd relative to toplevel) in startup_info Nguyễn Thái Ngọc Duy
@ 2010-11-11 14:08 ` Nguyễn Thái Ngọc Duy
  2010-11-11 14:08 ` [PATCH 3/3] get_sha1: support relative path ":path" syntax Nguyễn Thái Ngọc Duy
  2010-11-17 17:54 ` [PATCH 0/3] Support relative path in <blah>:path syntax Junio C Hamano
  3 siblings, 0 replies; 12+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2010-11-11 14:08 UTC (permalink / raw)
  To: git, Junio C Hamano, Jonathan Niedier, Matthieu.Moy
  Cc: Nguyễn Thái Ngọc Duy

prefix_path() allocates new buffer. There's no reason for it to keep
the buffer for itself and waste memory.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 cache.h |    2 +-
 setup.c |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/cache.h b/cache.h
index 222d9cf..bd181c6 100644
--- a/cache.h
+++ b/cache.h
@@ -428,7 +428,7 @@ 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);
+extern char *prefix_path(const char *prefix, int len, const char *path);
 extern const char *prefix_filename(const char *prefix, int len, const char *path);
 extern int check_filename(const char *prefix, const char *name);
 extern void verify_filename(const char *prefix, const char *name);
diff --git a/setup.c b/setup.c
index 833db12..f930dc0 100644
--- a/setup.c
+++ b/setup.c
@@ -4,7 +4,7 @@
 static int inside_git_dir = -1;
 static int inside_work_tree = -1;
 
-const char *prefix_path(const char *prefix, int len, const char *path)
+char *prefix_path(const char *prefix, int len, const char *path)
 {
 	const char *orig = path;
 	char *sanitized = xmalloc(len + strlen(path) + 1);
-- 
1.7.3.2.210.g045198

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 3/3] get_sha1: support relative path ":path" syntax
  2010-11-11 14:08 [PATCH 0/3] Support relative path in <blah>:path syntax Nguyễn Thái Ngọc Duy
  2010-11-11 14:08 ` [PATCH 1/3] setup: save prefix (original cwd relative to toplevel) in startup_info Nguyễn Thái Ngọc Duy
  2010-11-11 14:08 ` [PATCH 2/3] Make prefix_path() return char* without const Nguyễn Thái Ngọc Duy
@ 2010-11-11 14:08 ` Nguyễn Thái Ngọc Duy
  2010-11-14 20:22   ` Thiago Farina
  2010-11-17 17:54 ` [PATCH 0/3] Support relative path in <blah>:path syntax Junio C Hamano
  3 siblings, 1 reply; 12+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2010-11-11 14:08 UTC (permalink / raw)
  To: git, Junio C Hamano, Jonathan Niedier, Matthieu.Moy
  Cc: Nguyễn Thái Ngọc Duy

Currently :path and ref:path can be used to refer to a specific object
in index or ref respectively. "path" component is absolute path. This
patch allows "path" to be written as "./path" or "../path", which is
relative to user's original cwd.

This does not work in commands for which startup_info is NULL
(i.e. non-builtin ones, it seems none of them needs this anyway).

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 sha1_name.c                    |   37 ++++++++++++++++++++++--
 t/t1506-rev-parse-diagnosis.sh |   62 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 96 insertions(+), 3 deletions(-)

diff --git a/sha1_name.c b/sha1_name.c
index 484081d..22c1df9 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -1046,6 +1046,23 @@ int get_sha1_with_mode_1(const char *name, unsigned char *sha1, unsigned *mode,
 	return ret;
 }
 
+static char *resolve_relative_path(const char *rel)
+{
+	if (prefixcmp(rel, "./") && prefixcmp(rel, "../"))
+		return NULL;
+
+	if (!startup_info)
+		die("Relative path syntax is not supported in this command. Please report.");
+
+	if (!is_inside_work_tree())
+		die("relative path syntax can't be used outside working tree.");
+
+	/* die() inside prefix_path() if resolved path is outside worktree */
+	return prefix_path(startup_info->prefix,
+			   startup_info->prefix ? strlen(startup_info->prefix) : 0,
+			   rel);
+}
+
 int get_sha1_with_context_1(const char *name, unsigned char *sha1,
 			    struct object_context *oc,
 			    int gently, const char *prefix)
@@ -1060,25 +1077,31 @@ int get_sha1_with_context_1(const char *name, unsigned char *sha1,
 	if (!ret)
 		return ret;
 	/* sha1:path --> object name of path in ent sha1
-	 * :path -> object name of path in index
+	 * :path -> object name of absolute path in index
+	 * :./path -> object name of path relative to cwd in index
 	 * :[0-3]:path -> object name of path in index at stage
 	 * :/foo -> recent commit matching foo
 	 */
 	if (name[0] == ':') {
 		int stage = 0;
 		struct cache_entry *ce;
+		char *new_path = NULL;
 		int pos;
 		if (namelen > 2 && name[1] == '/')
 			return get_sha1_oneline(name + 2, sha1);
 		if (namelen < 3 ||
 		    name[2] != ':' ||
-		    name[1] < '0' || '3' < name[1])
+		    name[1] < '0' || '3' < name[1]) {
 			cp = name + 1;
+			new_path = resolve_relative_path(cp);
+			if (new_path)
+				cp = new_path;
+		}
 		else {
 			stage = name[1] - '0';
 			cp = name + 3;
 		}
-		namelen = namelen - (cp - name);
+		namelen = strlen(cp);
 
 		strncpy(oc->path, cp,
 			sizeof(oc->path));
@@ -1096,12 +1119,14 @@ int get_sha1_with_context_1(const char *name, unsigned char *sha1,
 				break;
 			if (ce_stage(ce) == stage) {
 				hashcpy(sha1, ce->sha1);
+				free(new_path);
 				return 0;
 			}
 			pos++;
 		}
 		if (!gently)
 			diagnose_invalid_index_path(stage, prefix, cp);
+		free(new_path);
 		return -1;
 	}
 	for (cp = name, bracket_depth = 0; *cp; cp++) {
@@ -1122,6 +1147,11 @@ int get_sha1_with_context_1(const char *name, unsigned char *sha1,
 		}
 		if (!get_sha1_1(name, cp-name, tree_sha1)) {
 			const char *filename = cp+1;
+			char *new_filename = NULL;
+
+			new_filename = resolve_relative_path(filename);
+			if (new_filename)
+				filename = new_filename;
 			ret = get_tree_entry(tree_sha1, filename, sha1, &oc->mode);
 			if (!gently) {
 				diagnose_invalid_sha1_path(prefix, filename,
@@ -1133,6 +1163,7 @@ int get_sha1_with_context_1(const char *name, unsigned char *sha1,
 				sizeof(oc->path));
 			oc->path[sizeof(oc->path)-1] = '\0';
 
+			free(new_filename);
 			return ret;
 		} else {
 			if (!gently)
diff --git a/t/t1506-rev-parse-diagnosis.sh b/t/t1506-rev-parse-diagnosis.sh
index 0eeeb0e..f7a4076 100755
--- a/t/t1506-rev-parse-diagnosis.sh
+++ b/t/t1506-rev-parse-diagnosis.sh
@@ -31,6 +31,43 @@ test_expect_success 'correct file objects' '
 	 test $HASH_file = $(git rev-parse :0:file.txt) )
 '
 
+test_expect_success 'correct relative file objects (0)' '
+	git rev-parse :file.txt >expected &&
+	git rev-parse :./file.txt >result &&
+	test_cmp expected result
+'
+
+test_expect_success 'correct relative file objects (1)' '
+	git rev-parse HEAD:file.txt >expected &&
+	git rev-parse HEAD:./file.txt >result &&
+	test_cmp expected result
+'
+
+test_expect_success 'correct relative file objects (2)' '
+	(
+		cd subdir &&
+		git rev-parse HEAD:../file.txt >result &&
+		test_cmp ../expected result
+	)
+'
+
+test_expect_success 'correct relative file objects (3)' '
+	(
+		cd subdir &&
+		git rev-parse HEAD:../subdir/../file.txt >result &&
+		test_cmp ../expected result
+	)
+'
+
+test_expect_success 'correct relative file objects (4)' '
+	git rev-parse HEAD:subdir/file.txt >expected &&
+	(
+		cd subdir &&
+		git rev-parse HEAD:./file.txt >result &&
+		test_cmp ../expected result
+	)
+'
+
 test_expect_success 'incorrect revision id' '
 	test_must_fail git rev-parse foobar:file.txt 2>error &&
 	grep "Invalid object name '"'"'foobar'"'"'." error &&
@@ -75,4 +112,29 @@ test_expect_success 'invalid @{n} reference' '
 	grep "fatal: Log for [^ ]* only has [0-9][0-9]* entries." error
 '
 
+test_expect_success 'relative path not found' '
+	(
+		cd subdir &&
+		test_must_fail git rev-parse HEAD:./nonexistent.txt 2>error &&
+		grep subdir/nonexistent.txt error
+	)
+'
+
+test_expect_success 'relative path outside worktree' '
+	test_must_fail git rev-parse HEAD:../file.txt >output 2>error &&
+	test -z "$(cat output)" &&
+	grep "outside repository" error
+'
+
+test_expect_success 'relative path when cwd is outside worktree' '
+	test_must_fail git --git-dir=.git --work-tree=subdir rev-parse HEAD:./file.txt >output 2>error &&
+	test -z "$(cat output)" &&
+	grep "relative path syntax can.t be used outside working tree." error
+'
+
+test_expect_success 'relative path when startup_info is NULL' '
+	test_must_fail test-match-trees HEAD:./file.txt HEAD:./file.txt 2>error &&
+	grep "Relative path syntax is not supported in this command" error
+'
+
 test_done
-- 
1.7.3.2.210.g045198

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH 3/3] get_sha1: support relative path ":path" syntax
  2010-11-11 14:08 ` [PATCH 3/3] get_sha1: support relative path ":path" syntax Nguyễn Thái Ngọc Duy
@ 2010-11-14 20:22   ` Thiago Farina
  2010-11-15  3:56     ` [PATCH] " Nguyễn Thái Ngọc Duy
  0 siblings, 1 reply; 12+ messages in thread
From: Thiago Farina @ 2010-11-14 20:22 UTC (permalink / raw)
  To: Nguyễn Thái Ngọc Duy
  Cc: git, Junio C Hamano, Jonathan Niedier, Matthieu.Moy

2010/11/11 Nguyễn Thái Ngọc Duy <pclouds@gmail.com>:
> Currently :path and ref:path can be used to refer to a specific object
> in index or ref respectively. "path" component is absolute path. This
> patch allows "path" to be written as "./path" or "../path", which is
> relative to user's original cwd.
>
> This does not work in commands for which startup_info is NULL
> (i.e. non-builtin ones, it seems none of them needs this anyway).
>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
>  sha1_name.c                    |   37 ++++++++++++++++++++++--
>  t/t1506-rev-parse-diagnosis.sh |   62 ++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 96 insertions(+), 3 deletions(-)
>
> diff --git a/sha1_name.c b/sha1_name.c
> index 484081d..22c1df9 100644
> --- a/sha1_name.c
> +++ b/sha1_name.c
> @@ -1046,6 +1046,23 @@ int get_sha1_with_mode_1(const char *name, unsigned char *sha1, unsigned *mode,
>        return ret;
>  }
>
> +static char *resolve_relative_path(const char *rel)
> +{
> +       if (prefixcmp(rel, "./") && prefixcmp(rel, "../"))
> +               return NULL;
> +
> +       if (!startup_info)
> +               die("Relative path syntax is not supported in this command. Please report.");
Is the "Please report." necessary? Report to who? Where? (I know we
know these answers, but maybe a new user won't know them).

> +
> +       if (!is_inside_work_tree())
> +               die("relative path syntax can't be used outside working tree.");
nit: s/relative/Relative ?

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH] get_sha1: support relative path ":path" syntax
  2010-11-14 20:22   ` Thiago Farina
@ 2010-11-15  3:56     ` Nguyễn Thái Ngọc Duy
  2010-11-15 14:56       ` Sverre Rabbelier
  0 siblings, 1 reply; 12+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2010-11-15  3:56 UTC (permalink / raw)
  To: git, tfransosi, Junio C Hamano
  Cc: Jonathan Niedier, Matthieu.Moy, Nguyễn Thái Ngọc Duy

Currently :path and ref:path can be used to refer to a specific object
in index or ref respectively. "path" component is absolute path. This
patch allows "path" to be written as "./path" or "../path", which is
relative to user's original cwd.

This does not work in commands for which startup_info is NULL
(i.e. non-builtin ones).

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 Fixed the error messages in resolve_relative_path()

 sha1_name.c                    |   38 ++++++++++++++++++++++--
 t/t1506-rev-parse-diagnosis.sh |   62 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 97 insertions(+), 3 deletions(-)

diff --git a/sha1_name.c b/sha1_name.c
index 484081d..1d227d5 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -1046,6 +1046,24 @@ int get_sha1_with_mode_1(const char *name, unsigned char *sha1, unsigned *mode,
 	return ret;
 }
 
+static char *resolve_relative_path(const char *rel)
+{
+	if (prefixcmp(rel, "./") && prefixcmp(rel, "../"))
+		return NULL;
+
+	if (!startup_info)
+		die("Relative path syntax is not supported in this command.\n"
+		    "Please report to git@vger.kernel.org.");
+
+	if (!is_inside_work_tree())
+		die("Relative path syntax can't be used outside working tree.");
+
+	/* die() inside prefix_path() if resolved path is outside worktree */
+	return prefix_path(startup_info->prefix,
+			   startup_info->prefix ? strlen(startup_info->prefix) : 0,
+			   rel);
+}
+
 int get_sha1_with_context_1(const char *name, unsigned char *sha1,
 			    struct object_context *oc,
 			    int gently, const char *prefix)
@@ -1060,25 +1078,31 @@ int get_sha1_with_context_1(const char *name, unsigned char *sha1,
 	if (!ret)
 		return ret;
 	/* sha1:path --> object name of path in ent sha1
-	 * :path -> object name of path in index
+	 * :path -> object name of absolute path in index
+	 * :./path -> object name of path relative to cwd in index
 	 * :[0-3]:path -> object name of path in index at stage
 	 * :/foo -> recent commit matching foo
 	 */
 	if (name[0] == ':') {
 		int stage = 0;
 		struct cache_entry *ce;
+		char *new_path = NULL;
 		int pos;
 		if (namelen > 2 && name[1] == '/')
 			return get_sha1_oneline(name + 2, sha1);
 		if (namelen < 3 ||
 		    name[2] != ':' ||
-		    name[1] < '0' || '3' < name[1])
+		    name[1] < '0' || '3' < name[1]) {
 			cp = name + 1;
+			new_path = resolve_relative_path(cp);
+			if (new_path)
+				cp = new_path;
+		}
 		else {
 			stage = name[1] - '0';
 			cp = name + 3;
 		}
-		namelen = namelen - (cp - name);
+		namelen = strlen(cp);
 
 		strncpy(oc->path, cp,
 			sizeof(oc->path));
@@ -1096,12 +1120,14 @@ int get_sha1_with_context_1(const char *name, unsigned char *sha1,
 				break;
 			if (ce_stage(ce) == stage) {
 				hashcpy(sha1, ce->sha1);
+				free(new_path);
 				return 0;
 			}
 			pos++;
 		}
 		if (!gently)
 			diagnose_invalid_index_path(stage, prefix, cp);
+		free(new_path);
 		return -1;
 	}
 	for (cp = name, bracket_depth = 0; *cp; cp++) {
@@ -1122,6 +1148,11 @@ int get_sha1_with_context_1(const char *name, unsigned char *sha1,
 		}
 		if (!get_sha1_1(name, cp-name, tree_sha1)) {
 			const char *filename = cp+1;
+			char *new_filename = NULL;
+
+			new_filename = resolve_relative_path(filename);
+			if (new_filename)
+				filename = new_filename;
 			ret = get_tree_entry(tree_sha1, filename, sha1, &oc->mode);
 			if (!gently) {
 				diagnose_invalid_sha1_path(prefix, filename,
@@ -1133,6 +1164,7 @@ int get_sha1_with_context_1(const char *name, unsigned char *sha1,
 				sizeof(oc->path));
 			oc->path[sizeof(oc->path)-1] = '\0';
 
+			free(new_filename);
 			return ret;
 		} else {
 			if (!gently)
diff --git a/t/t1506-rev-parse-diagnosis.sh b/t/t1506-rev-parse-diagnosis.sh
index 0eeeb0e..f7a4076 100755
--- a/t/t1506-rev-parse-diagnosis.sh
+++ b/t/t1506-rev-parse-diagnosis.sh
@@ -31,6 +31,43 @@ test_expect_success 'correct file objects' '
 	 test $HASH_file = $(git rev-parse :0:file.txt) )
 '
 
+test_expect_success 'correct relative file objects (0)' '
+	git rev-parse :file.txt >expected &&
+	git rev-parse :./file.txt >result &&
+	test_cmp expected result
+'
+
+test_expect_success 'correct relative file objects (1)' '
+	git rev-parse HEAD:file.txt >expected &&
+	git rev-parse HEAD:./file.txt >result &&
+	test_cmp expected result
+'
+
+test_expect_success 'correct relative file objects (2)' '
+	(
+		cd subdir &&
+		git rev-parse HEAD:../file.txt >result &&
+		test_cmp ../expected result
+	)
+'
+
+test_expect_success 'correct relative file objects (3)' '
+	(
+		cd subdir &&
+		git rev-parse HEAD:../subdir/../file.txt >result &&
+		test_cmp ../expected result
+	)
+'
+
+test_expect_success 'correct relative file objects (4)' '
+	git rev-parse HEAD:subdir/file.txt >expected &&
+	(
+		cd subdir &&
+		git rev-parse HEAD:./file.txt >result &&
+		test_cmp ../expected result
+	)
+'
+
 test_expect_success 'incorrect revision id' '
 	test_must_fail git rev-parse foobar:file.txt 2>error &&
 	grep "Invalid object name '"'"'foobar'"'"'." error &&
@@ -75,4 +112,29 @@ test_expect_success 'invalid @{n} reference' '
 	grep "fatal: Log for [^ ]* only has [0-9][0-9]* entries." error
 '
 
+test_expect_success 'relative path not found' '
+	(
+		cd subdir &&
+		test_must_fail git rev-parse HEAD:./nonexistent.txt 2>error &&
+		grep subdir/nonexistent.txt error
+	)
+'
+
+test_expect_success 'relative path outside worktree' '
+	test_must_fail git rev-parse HEAD:../file.txt >output 2>error &&
+	test -z "$(cat output)" &&
+	grep "outside repository" error
+'
+
+test_expect_success 'relative path when cwd is outside worktree' '
+	test_must_fail git --git-dir=.git --work-tree=subdir rev-parse HEAD:./file.txt >output 2>error &&
+	test -z "$(cat output)" &&
+	grep "relative path syntax can.t be used outside working tree." error
+'
+
+test_expect_success 'relative path when startup_info is NULL' '
+	test_must_fail test-match-trees HEAD:./file.txt HEAD:./file.txt 2>error &&
+	grep "Relative path syntax is not supported in this command" error
+'
+
 test_done
-- 
1.7.3.2.210.g045198

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH] get_sha1: support relative path ":path" syntax
  2010-11-15  3:56     ` [PATCH] " Nguyễn Thái Ngọc Duy
@ 2010-11-15 14:56       ` Sverre Rabbelier
  2010-11-15 17:29         ` Junio C Hamano
  0 siblings, 1 reply; 12+ messages in thread
From: Sverre Rabbelier @ 2010-11-15 14:56 UTC (permalink / raw)
  To: Nguyễn Thái Ngọc Duy
  Cc: git, tfransosi, Junio C Hamano, Jonathan Niedier, Matthieu.Moy

Heya,

2010/11/15 Nguyễn Thái Ngọc Duy <pclouds@gmail.com>:
> +               die("Relative path syntax is not supported in this command.\n"
> +                   "Please report to git@vger.kernel.org.");

Might it save us a lot of debugging hassle later if we report what
"this command" is? E.g., if the user is using some internal tool that
happens to dispatch to a command that doesn't support this, it could
help us to know what command is being used?

-- 
Cheers,

Sverre Rabbelier

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] get_sha1: support relative path ":path" syntax
  2010-11-15 14:56       ` Sverre Rabbelier
@ 2010-11-15 17:29         ` Junio C Hamano
  2010-11-15 18:59           ` Junio C Hamano
  2010-11-28  3:37           ` Nguyễn Thái Ngọc Duy
  0 siblings, 2 replies; 12+ messages in thread
From: Junio C Hamano @ 2010-11-15 17:29 UTC (permalink / raw)
  To: Sverre Rabbelier
  Cc: Nguyễn Thái Ngọc Duy, git, tfransosi,
	Jonathan Niedier, Matthieu.Moy

Sverre Rabbelier <srabbelier@gmail.com> writes:

> 2010/11/15 Nguyễn Thái Ngọc Duy <pclouds@gmail.com>:
>> +               die("Relative path syntax is not supported in this command.\n"
>> +                   "Please report to git@vger.kernel.org.");
>
> Might it save us a lot of debugging hassle later if we report what
> "this command" is? E.g., if the user is using some internal tool that
> happens to dispatch to a command that doesn't support this, it could
> help us to know what command is being used?

In the absence of programming errors, should all git command support
<tree>:./<path> syntax to name an object, or are there cases where the
relative does not make any sense?

What I am trying to get at is that if this is diagnosing a user error, or
if this is showing that the mechanism to implement the relative path is
unnecessarily hard for the programmers to misuse.

For example, if "git show HEAD:./Makefile" in a bare repository is a user
error, it is not "not supported in this command" but "not supported in
this situation (more specifically, in a bare repository)", so the first
line is wrong, and more importantly, if that is a user error, there is
nothing to report to the list.

If on the other hand, a command that ought to allow use of relative path
didn't set up necessary startup_info, this is diagnosing a programming
error.  But if that is the case, shouldn't we be able to do much better to
avoid such mistakes in the first place than triggering a BUG() here when
the user happens to try using this particular feature?  In other words,
even when this "feature" isn't used in a particular invocation of a
command, a command that is capable of supporting the feature should always
be setting up startup_info, perhaps by the time its cmd_foo() is called,
no?  Shouldn't we be putting a BUG() somewhere higher and more visible in
the callchain to help catching such bugs?

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] get_sha1: support relative path ":path" syntax
  2010-11-15 17:29         ` Junio C Hamano
@ 2010-11-15 18:59           ` Junio C Hamano
  2010-11-28  3:37           ` Nguyễn Thái Ngọc Duy
  1 sibling, 0 replies; 12+ messages in thread
From: Junio C Hamano @ 2010-11-15 18:59 UTC (permalink / raw)
  To: Nguyễn Thái Ngọc Duy
  Cc: Sverre Rabbelier, git, tfransosi, Jonathan Niedier, Matthieu.Moy

Junio C Hamano <gitster@pobox.com> writes:

> In the absence of programming errors, should all git command support
> <tree>:./<path> syntax to name an object, or are there cases where the
> relative does not make any sense?
>
> What I am trying to get at is that if this is diagnosing a user error, or
> if this is showing that the mechanism to implement the relative path is
> unnecessarily hard for the programmers to misuse.

The mistake should be obvious from the context, but I wanted to say with
the last sentence "the mechanism is too hard for the programmers to use
and easy to make mistakes."

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 0/3] Support relative path in <blah>:path syntax
  2010-11-11 14:08 [PATCH 0/3] Support relative path in <blah>:path syntax Nguyễn Thái Ngọc Duy
                   ` (2 preceding siblings ...)
  2010-11-11 14:08 ` [PATCH 3/3] get_sha1: support relative path ":path" syntax Nguyễn Thái Ngọc Duy
@ 2010-11-17 17:54 ` Junio C Hamano
  2010-11-18  1:47   ` Nguyen Thai Ngoc Duy
  3 siblings, 1 reply; 12+ messages in thread
From: Junio C Hamano @ 2010-11-17 17:54 UTC (permalink / raw)
  To: Nguyễn Thái Ngọc Duy; +Cc: git, Jonathan Niedier, Matthieu.Moy

Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:

> Sorry Jonathan I lied. I did not pick up your fast-import changes.
> Could not find how to test it. And it seems fast-import only cares
> about commits, not the target audience of this syntax.
>
> Document is not updated because I think it's intuitive enough.

When you say <tree>:<path>, you would intuitively expect that the path is
relative to <tree>, but this patch deliberately breaks (in a good way)
that expectation by introducing a magic token "./".  Once you know that
"./" magic _exists_, it is obvious what it means, but people may not even
imagine that such a magic may exist in the first place (certainly old
timers won't), and would not know what the magic token is if you do not
tell them.

It needs to be documented.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 0/3] Support relative path in <blah>:path syntax
  2010-11-17 17:54 ` [PATCH 0/3] Support relative path in <blah>:path syntax Junio C Hamano
@ 2010-11-18  1:47   ` Nguyen Thai Ngoc Duy
  0 siblings, 0 replies; 12+ messages in thread
From: Nguyen Thai Ngoc Duy @ 2010-11-18  1:47 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Jonathan Niedier, Matthieu.Moy

On Wed, Nov 17, 2010 at 09:54:08AM -0800, Junio C Hamano wrote:
> Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:
> 
> > Sorry Jonathan I lied. I did not pick up your fast-import changes.
> > Could not find how to test it. And it seems fast-import only cares
> > about commits, not the target audience of this syntax.
> >
> > Document is not updated because I think it's intuitive enough.
> 
> When you say <tree>:<path>, you would intuitively expect that the path is
> relative to <tree>, but this patch deliberately breaks (in a good way)
> that expectation by introducing a magic token "./".  Once you know that
> "./" magic _exists_, it is obvious what it means, but people may not even
> imagine that such a magic may exist in the first place (certainly old
> timers won't), and would not know what the magic token is if you do not
> tell them.
> 
> It needs to be documented.

OK. How about this, squashing in the last patch of this series?

--8<--
diff --git a/Documentation/revisions.txt b/Documentation/revisions.txt
index 3d4b79c..3fc3e8b 100644
--- a/Documentation/revisions.txt
+++ b/Documentation/revisions.txt
@@ -121,6 +121,10 @@ the `$GIT_DIR/refs` directory or from the `$GIT_DIR/packed-refs` file.
   ':path' (with an empty part before the colon, e.g. `:README`)
   is a special case of the syntax described next: content
   recorded in the index at the given path.
+  A path starting with './' or '../' is relative to current working directory.
+  The given path will be converted to be relative to working tree's root directory.
+  This is most useful to address a blob or tree from a commit or tree that has
+  the same tree structure with the working tree.
 
 * A colon, optionally followed by a stage number (0 to 3) and a
   colon, followed by a path (e.g. `:0:README`); this names a blob object in the
--8<--

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH] get_sha1: support relative path ":path" syntax
  2010-11-15 17:29         ` Junio C Hamano
  2010-11-15 18:59           ` Junio C Hamano
@ 2010-11-28  3:37           ` Nguyễn Thái Ngọc Duy
  1 sibling, 0 replies; 12+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2010-11-28  3:37 UTC (permalink / raw)
  To: git, Junio C Hamano, srabbelier, tfransosi, Jonathan Niedier,
	Matthieu.Moy
  Cc: Nguyễn Thái Ngọc Duy

Currently :path and ref:path can be used to refer to a specific object
in index or ref respectively. "path" component is absolute path. This
patch allows "path" to be written as "./path" or "../path", which is
relative to user's original cwd.

This does not work in commands for which startup_info is NULL
(i.e. non-builtin ones, it seems none of them needs this anyway).

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 Updated the message when startup_info == NULL

 Documentation/revisions.txt    |    4 ++
 sha1_name.c                    |   37 ++++++++++++++++++++++--
 t/t1506-rev-parse-diagnosis.sh |   62 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 100 insertions(+), 3 deletions(-)

diff --git a/Documentation/revisions.txt b/Documentation/revisions.txt
index 3d4b79c..8b519d7 100644
--- a/Documentation/revisions.txt
+++ b/Documentation/revisions.txt
@@ -121,6 +121,10 @@ the `$GIT_DIR/refs` directory or from the `$GIT_DIR/packed-refs` file.
   ':path' (with an empty part before the colon, e.g. `:README`)
   is a special case of the syntax described next: content
   recorded in the index at the given path.
+  A path starting with './' or '../' is relative to current working directory.
+  The given path will be converted to be relative to working tree's root directory.
+  This is most useful to address a blob or tree from a commit or tree that has
+  the same tree structure with the working tree.
 
 * A colon, optionally followed by a stage number (0 to 3) and a
   colon, followed by a path (e.g. `:0:README`); this names a blob object in the
diff --git a/sha1_name.c b/sha1_name.c
index 484081d..f918faf 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -1046,6 +1046,23 @@ int get_sha1_with_mode_1(const char *name, unsigned char *sha1, unsigned *mode,
 	return ret;
 }
 
+static char *resolve_relative_path(const char *rel)
+{
+	if (prefixcmp(rel, "./") && prefixcmp(rel, "../"))
+		return NULL;
+
+	if (!startup_info)
+		die("BUG: startup_info struct is not initialized.");
+
+	if (!is_inside_work_tree())
+		die("relative path syntax can't be used outside working tree.");
+
+	/* die() inside prefix_path() if resolved path is outside worktree */
+	return prefix_path(startup_info->prefix,
+			   startup_info->prefix ? strlen(startup_info->prefix) : 0,
+			   rel);
+}
+
 int get_sha1_with_context_1(const char *name, unsigned char *sha1,
 			    struct object_context *oc,
 			    int gently, const char *prefix)
@@ -1060,25 +1077,31 @@ int get_sha1_with_context_1(const char *name, unsigned char *sha1,
 	if (!ret)
 		return ret;
 	/* sha1:path --> object name of path in ent sha1
-	 * :path -> object name of path in index
+	 * :path -> object name of absolute path in index
+	 * :./path -> object name of path relative to cwd in index
 	 * :[0-3]:path -> object name of path in index at stage
 	 * :/foo -> recent commit matching foo
 	 */
 	if (name[0] == ':') {
 		int stage = 0;
 		struct cache_entry *ce;
+		char *new_path = NULL;
 		int pos;
 		if (namelen > 2 && name[1] == '/')
 			return get_sha1_oneline(name + 2, sha1);
 		if (namelen < 3 ||
 		    name[2] != ':' ||
-		    name[1] < '0' || '3' < name[1])
+		    name[1] < '0' || '3' < name[1]) {
 			cp = name + 1;
+			new_path = resolve_relative_path(cp);
+			if (new_path)
+				cp = new_path;
+		}
 		else {
 			stage = name[1] - '0';
 			cp = name + 3;
 		}
-		namelen = namelen - (cp - name);
+		namelen = strlen(cp);
 
 		strncpy(oc->path, cp,
 			sizeof(oc->path));
@@ -1096,12 +1119,14 @@ int get_sha1_with_context_1(const char *name, unsigned char *sha1,
 				break;
 			if (ce_stage(ce) == stage) {
 				hashcpy(sha1, ce->sha1);
+				free(new_path);
 				return 0;
 			}
 			pos++;
 		}
 		if (!gently)
 			diagnose_invalid_index_path(stage, prefix, cp);
+		free(new_path);
 		return -1;
 	}
 	for (cp = name, bracket_depth = 0; *cp; cp++) {
@@ -1122,6 +1147,11 @@ int get_sha1_with_context_1(const char *name, unsigned char *sha1,
 		}
 		if (!get_sha1_1(name, cp-name, tree_sha1)) {
 			const char *filename = cp+1;
+			char *new_filename = NULL;
+
+			new_filename = resolve_relative_path(filename);
+			if (new_filename)
+				filename = new_filename;
 			ret = get_tree_entry(tree_sha1, filename, sha1, &oc->mode);
 			if (!gently) {
 				diagnose_invalid_sha1_path(prefix, filename,
@@ -1133,6 +1163,7 @@ int get_sha1_with_context_1(const char *name, unsigned char *sha1,
 				sizeof(oc->path));
 			oc->path[sizeof(oc->path)-1] = '\0';
 
+			free(new_filename);
 			return ret;
 		} else {
 			if (!gently)
diff --git a/t/t1506-rev-parse-diagnosis.sh b/t/t1506-rev-parse-diagnosis.sh
index 0eeeb0e..f7a4076 100755
--- a/t/t1506-rev-parse-diagnosis.sh
+++ b/t/t1506-rev-parse-diagnosis.sh
@@ -31,6 +31,43 @@ test_expect_success 'correct file objects' '
 	 test $HASH_file = $(git rev-parse :0:file.txt) )
 '
 
+test_expect_success 'correct relative file objects (0)' '
+	git rev-parse :file.txt >expected &&
+	git rev-parse :./file.txt >result &&
+	test_cmp expected result
+'
+
+test_expect_success 'correct relative file objects (1)' '
+	git rev-parse HEAD:file.txt >expected &&
+	git rev-parse HEAD:./file.txt >result &&
+	test_cmp expected result
+'
+
+test_expect_success 'correct relative file objects (2)' '
+	(
+		cd subdir &&
+		git rev-parse HEAD:../file.txt >result &&
+		test_cmp ../expected result
+	)
+'
+
+test_expect_success 'correct relative file objects (3)' '
+	(
+		cd subdir &&
+		git rev-parse HEAD:../subdir/../file.txt >result &&
+		test_cmp ../expected result
+	)
+'
+
+test_expect_success 'correct relative file objects (4)' '
+	git rev-parse HEAD:subdir/file.txt >expected &&
+	(
+		cd subdir &&
+		git rev-parse HEAD:./file.txt >result &&
+		test_cmp ../expected result
+	)
+'
+
 test_expect_success 'incorrect revision id' '
 	test_must_fail git rev-parse foobar:file.txt 2>error &&
 	grep "Invalid object name '"'"'foobar'"'"'." error &&
@@ -75,4 +112,29 @@ test_expect_success 'invalid @{n} reference' '
 	grep "fatal: Log for [^ ]* only has [0-9][0-9]* entries." error
 '
 
+test_expect_success 'relative path not found' '
+	(
+		cd subdir &&
+		test_must_fail git rev-parse HEAD:./nonexistent.txt 2>error &&
+		grep subdir/nonexistent.txt error
+	)
+'
+
+test_expect_success 'relative path outside worktree' '
+	test_must_fail git rev-parse HEAD:../file.txt >output 2>error &&
+	test -z "$(cat output)" &&
+	grep "outside repository" error
+'
+
+test_expect_success 'relative path when cwd is outside worktree' '
+	test_must_fail git --git-dir=.git --work-tree=subdir rev-parse HEAD:./file.txt >output 2>error &&
+	test -z "$(cat output)" &&
+	grep "relative path syntax can.t be used outside working tree." error
+'
+
+test_expect_success 'relative path when startup_info is NULL' '
+	test_must_fail test-match-trees HEAD:./file.txt HEAD:./file.txt 2>error &&
+	grep "Relative path syntax is not supported in this command" error
+'
+
 test_done
-- 
1.7.3.2.316.gda8b3

^ permalink raw reply related	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2010-11-28  3:44 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-11-11 14:08 [PATCH 0/3] Support relative path in <blah>:path syntax Nguyễn Thái Ngọc Duy
2010-11-11 14:08 ` [PATCH 1/3] setup: save prefix (original cwd relative to toplevel) in startup_info Nguyễn Thái Ngọc Duy
2010-11-11 14:08 ` [PATCH 2/3] Make prefix_path() return char* without const Nguyễn Thái Ngọc Duy
2010-11-11 14:08 ` [PATCH 3/3] get_sha1: support relative path ":path" syntax Nguyễn Thái Ngọc Duy
2010-11-14 20:22   ` Thiago Farina
2010-11-15  3:56     ` [PATCH] " Nguyễn Thái Ngọc Duy
2010-11-15 14:56       ` Sverre Rabbelier
2010-11-15 17:29         ` Junio C Hamano
2010-11-15 18:59           ` Junio C Hamano
2010-11-28  3:37           ` Nguyễn Thái Ngọc Duy
2010-11-17 17:54 ` [PATCH 0/3] Support relative path in <blah>:path syntax Junio C Hamano
2010-11-18  1:47   ` Nguyen Thai Ngoc Duy

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.