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

STALE3731d

9 messages, 4 authors, 2016-06-15 · open the first message on its own page

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

From: Nguyễn Thái Ngọc Duy <hidden>
Date: 2016-06-15 22:50:01

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

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

From: Nguyễn Thái Ngọc Duy <hidden>
Date: 2016-06-15 22:50:01

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 <redacted>
Signed-off-by: Jonathan Nieder <redacted>
Signed-off-by: Junio C Hamano <redacted>
---
 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

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

From: Nguyễn Thái Ngọc Duy <hidden>
Date: 2016-06-15 22:50:01

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 <redacted>
---
 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

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

From: Nguyễn Thái Ngọc Duy <hidden>
Date: 2016-06-15 22:50:01

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 <redacted>
---
 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

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

From: Thiago Farina <hidden>
Date: 2016-06-15 22:50:02

2010/11/11 Nguyễn Thái Ngọc Duy [off-list ref]:
quoted hunk
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 <redacted>
---
 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 ?

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

From: Nguyễn Thái Ngọc Duy <hidden>
Date: 2016-06-15 22:50:02

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 <redacted>
---
 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

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

From: Sverre Rabbelier <hidden>
Date: 2016-06-15 22:50:02

Heya,

2010/11/15 Nguyễn Thái Ngọc Duy [off-list ref]:
+               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

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

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:50:03

Nguyễn Thái Ngọc Duy  [off-list ref] 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.

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

From: Nguyen Thai Ngoc Duy <hidden>
Date: 2016-06-15 22:50:03

On Wed, Nov 17, 2010 at 09:54:08AM -0800, Junio C Hamano wrote:
Nguyễn Thái Ngọc Duy  [off-list ref] writes:
quoted
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<--
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help