threads / patch / 25711

patch, 3 partsSupport relative path in <blah>:path syntax

Subject: [PATCH 0/3] Support relative path in <blah>:path syntax

## tl;dr

12 messages between Nov 11, 2010 and Nov 28, 2010. Diffs are folded; open one to read it.

replies: 11people: 4as markdown or json

Nguyễn Thái Ngọc Duy· Nov 11, 2010, 14:08 UTC · lore

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
Nguyễn Thái Ngọc Duy· Nov 11, 2010, 14:08 UTC · re: Nguyễn Thái Ngọc Duy · lore

[PATCH 1/3] setup: save prefix (original cwd relative to toplevel) in startup_info

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(-)
Show changes to 2 files +4 −1

cache.h, setup.c

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
Nguyễn Thái Ngọc Duy· Nov 11, 2010, 14:08 UTC · re: Nguyễn Thái Ngọc Duy · lore

[PATCH 2/3] Make prefix_path() return char* without const

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(-)
Show changes to 2 files +2 −2

cache.h, setup.c

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
Nguyễn Thái Ngọc Duy· Nov 11, 2010, 14:08 UTC · re: Nguyễn Thái Ngọc Duy · lore

[PATCH 3/3] get_sha1: support relative path ":path" syntax

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(-)
Show changes to 2 files +96 −3

sha1_name.c, t/t1506-rev-parse-diagnosis.sh

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
Thiago Farina· Nov 14, 2010, 20:22 UTC · re: Nguyễn Thái Ngọc Duy · lore

Re: [PATCH 3/3] get_sha1: support relative path ":path" syntax

2010/11/11 Nguyễn Thái Ngọc Duy <pclouds@gmail.com>:
Show 29 quoted lines
> 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 ?
Nguyễn Thái Ngọc Duy· Nov 15, 2010, 03:56 UTC · re: Thiago Farina · lore

[PATCH] get_sha1: support relative path ":path" syntax

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(-)
Show changes to 2 files +97 −3

sha1_name.c, t/t1506-rev-parse-diagnosis.sh

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
Sverre Rabbelier· Nov 15, 2010, 14:56 UTC · re: Nguyễn Thái Ngọc Duy · lore

Re: [PATCH] get_sha1: support relative path ":path" syntax

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
Junio C Hamano· Nov 15, 2010, 17:29 UTC · re: Sverre Rabbelier · lore

Re: [PATCH] get_sha1: support relative path ":path" syntax

Sverre Rabbelier <srabbelier@gmail.com> writes:
Show 8 quoted lines
> 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?

Junio C Hamano· Nov 15, 2010, 18:59 UTC · re: Junio C Hamano · lore

Re: [PATCH] get_sha1: support relative path ":path" syntax

Junio C Hamano <gitster@pobox.com> writes:
Show 7 quoted lines
> 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."

Nguyễn Thái Ngọc Duy· Nov 28, 2010, 03:37 UTC · re: Junio C Hamano · lore

[PATCH] get_sha1: support relative path ":path" syntax

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(-)
Show changes to 3 files +100 −3

Documentation/revisions.txt, sha1_name.c, t/t1506-rev-parse-diagnosis.sh

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
Junio C Hamano· Nov 17, 2010, 17:54 UTC · re: Nguyễn Thái Ngọc Duy · lore

Re: [PATCH 0/3] Support relative path in <blah>:path syntax

Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:
Show 5 quoted lines
> 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.
Nguyen Thai Ngoc Duy· Nov 18, 2010, 01:47 UTC · re: Junio C Hamano · lore

Re: [PATCH 0/3] Support relative path in <blah>:path syntax

On Wed, Nov 17, 2010 at 09:54:08AM -0800, Junio C Hamano wrote:
Show 17 quoted lines
> 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<--
Show changes to Documentation/revisions.txt +4 −1
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<--

← back to recent threads