Re: [PATCH v0] sha1_name: grok <revision>:./<relative-path>
- From
Nguyen Thai Ngoc Duy <pclouds@gmail.com>
- Date
- Dec 21, 2007, 14:17 UTC
- Message-ID
- <fcaeb9bf0712210617x2bafa33cp15815a59fc631f45@mail.gmail.com>
- In-Reply-To
- <Pine.LNX.4.64.0712191334460.23902@racer.site>
On Dec 19, 2007 8:40 PM, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
Show 9 quoted lines
> > When you are in a deeply-nested directory structure, and just want > to reference a blob in a past revision, it can be pretty slow to > type out "HEAD~29:/bla/blub/.../that-file". > > This patch makes "HEAD~29:./that-file" substitute the current prefix > for "./". If there is not working directory, the prefix is empty. > > Note that this patch does not handle "../", and neither do I plan to.
Junio's rc1 announcement got me to read this. It would be indeed useful as I usually work in deep subdirs. However, from my user perspective, the right approach is to make <treeish>:path always be relative to current directory. If you want absolute path, use <treeish>:/path. More intuitive but it breaks current behavior. Can we slowly migrate from current absolute-path-by-default behavior to relative-pat- by-default one? (I don't know how to make such migration smoothly though)
Show 148 quoted lines
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>
> On Tue, 18 Dec 2007, Dana How wrote:
>
> > On Dec 18, 2007 5:16 PM, Linus Torvalds
> > <torvalds@linux-foundation.org> wrote:
> > > On Tue, 18 Dec 2007, Dana How wrote:
> > >
> > > > The cases we are talking about are all subtrees of the
> > > > working tree. There is a useful cwd suffix.
> > >
> > > No.
> > >
> > > The cases we're talking of are *not* subtrees of the working
> > > tree.
> > >
> > > The SHA1 of a commit may well be a totally disjoint tree. Try
> > > it in the git repository with something like
> >
> > Agreed, but note you wrote *may*.
>
> Okay, this is a proposed patch. It leaves the existing
> "HEAD:<path>" handling alone, and only touches "HEAD:./<path>",
> which would have been invalid anyway (except if you hacked your
> objects database to include a tree named ".").
>
> Note: this patch is not meant for application directly. It should
> be split into get_current_prefix() as one patch, and the
> sha1_name.c stuff as the second. (Not only to boost my ohloh
> statistics, but because they are logically two separate things.)
>
> Note, too: this is a quick and little-bit-dirty patch, not well
> tested. Particularly, I was unable to trigger the "No <path> in
> <rev>" error path, so I am not confident that this handling is
> correct.
>
> Note also: in contrast to Alex' approach, this will not only work
> for git-show, but for all callers of get_sha1().
>
> cache.h | 1 +
> setup.c | 16 +++++++++++++---
> sha1_name.c | 17 ++++++++++++++---
> 3 files changed, 28 insertions(+), 6 deletions(-)
>
> diff --git a/cache.h b/cache.h
> index 39331c2..83a2c31 100644
> --- a/cache.h
> +++ b/cache.h
> @@ -225,6 +225,7 @@ extern char *get_index_file(void);
> extern char *get_graft_file(void);
> extern int set_git_dir(const char *path);
> extern const char *get_git_work_tree(void);
> +extern const char *get_current_prefix(void);
>
> #define ALTERNATE_DB_ENVIRONMENT "GIT_ALTERNATE_OBJECT_DIRECTORIES"
>
> diff --git a/setup.c b/setup.c
> index b59dbe7..fb9b680 100644
> --- a/setup.c
> +++ b/setup.c
> @@ -3,6 +3,12 @@
>
> static int inside_git_dir = -1;
> static int inside_work_tree = -1;
> +static const char *current_prefix;
> +
> +const char *get_current_prefix()
> +{
> + return current_prefix;
> +}
>
> const char *prefix_path(const char *prefix, int len, const char *path)
> {
> @@ -267,6 +273,7 @@ const char *setup_git_directory_gently(int *nongit_ok)
> /* config may override worktree */
> if (check_repository_format_gently(nongit_ok))
> return NULL;
> + current_prefix = retval;
> return retval;
> }
> if (check_repository_format_gently(nongit_ok))
> @@ -279,7 +286,8 @@ const char *setup_git_directory_gently(int *nongit_ok)
> if (chdir(work_tree_env) < 0)
> die ("Could not chdir to %s", work_tree_env);
> strcat(buffer, "/");
> - return retval;
> + current_prefix = retval;
> + return current_prefix;
> }
> if (nongit_ok) {
> *nongit_ok = 1;
> @@ -339,7 +347,8 @@ const char *setup_git_directory_gently(int *nongit_ok)
> offset++;
> cwd[len++] = '/';
> cwd[len] = 0;
> - return cwd + offset;
> + current_prefix = cwd + offset;
> + return current_prefix;
> }
>
> int git_config_perm(const char *var, const char *value)
> @@ -396,7 +405,8 @@ const char *setup_git_directory(void)
> if (retval && chdir(retval))
> die ("Could not jump back into original cwd");
> rel = get_relative_cwd(buffer, PATH_MAX, get_git_work_tree());
> - return rel && *rel ? strcat(rel, "/") : NULL;
> + current_prefix = rel && *rel ? strcat(rel, "/") : NULL;
> + return current_prefix;
> }
>
> return retval;
> diff --git a/sha1_name.c b/sha1_name.c
> index 13e1164..6f61d26 100644
> --- a/sha1_name.c
> +++ b/sha1_name.c
> @@ -712,9 +712,20 @@ int get_sha1_with_mode(const char *name, unsigned char *sha1, unsigned *mode)
> }
> if (*cp == ':') {
> unsigned char tree_sha1[20];
> - if (!get_sha1_1(name, cp-name, tree_sha1))
> - return get_tree_entry(tree_sha1, cp+1, sha1,
> - mode);
> + if (!get_sha1_1(name, cp-name, tree_sha1)) {
> + const char *prefix;
> + if (!prefixcmp(cp + 1, "./") &&
> + (prefix = get_current_prefix())) {
> + unsigned char subtree_sha1[20];
> + if (get_tree_entry(tree_sha1, prefix,
> + subtree_sha1, mode))
> + return error("No '%s' in '%.*s'",
> + prefix, cp-name, name);
> + memcpy(tree_sha1, subtree_sha1, 20);
> + cp += 2;
> + }
> + return get_tree_entry(tree_sha1, cp+1, sha1, mode);
> + }
> }
> return ret;
> }
> --
> 1.5.4.rc0.72.g536e9
>
> -
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>-- Duy