Re: [PATCH v0] sha1_name: grok <revision>:./<relative-path>

["Nguyen Thai Ngoc Duy" <pclouds@gmail.com>]

On Dec 19, 2007 8:40 PM, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
>
> 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)

> 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