* [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
* [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
* 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
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 a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).