Re: [PATCH] sha1_name(): accept ':directory/' to get at the cache_tree

From: Junio C Hamano <junkio@cox.net>
Date: Jan 9, 2007, 23:47 UTC
Message-ID: <7vodp7iwzu.fsf@assigned-by-dhcp.cox.net>
In-Reply-To: <Pine.LNX.4.63.0701091502030.22628@wbgn013.biozentrum.uni-wuerzburg.de>

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

Show 5 quoted lines

> 	'tis a resend of an earlier patch, but without support for the
> 	bogus ":." as equivalent to ":/".
>
> 	I find this feature highly convenient when I just want to see
> 	what files the index contains.

I do not understand; do you mean ls-files?

In any case, I wonder if this does a sane thing if you asked "git show :3:t/" on a fully merged index.

Show 9 quoted lines

> @@ -561,6 +562,23 @@ int get_sha1(const char *name, unsigned char *sha1)
>  			}
>  			pos++;
>  		}
> +		if (namelen > 0 && cp[namelen - 1] == '/')
> +			namelen--;
> +		if (namelen == 0 || (ce && ce_namelen(ce) > namelen &&
> +					ce->name[namelen] == '/' &&
> +					!memcmp(ce->name, cp, namelen))) {

I may be misreading the code, but what does ce point at? Does this get the index sort order correctly? For example, would this work?

	$ echo >t- && git add t-
        $ git show :t
	$ git show :t/

Show 7 quoted lines

> +			struct cache_tree *tree =
> +				cache_tree_find(active_cache_tree, cp);
> +			if (!cache_tree_fully_valid(tree)) {
> +				ret = cache_tree_update(active_cache_tree,
> +						active_cache, active_nr, 0, 0);
> +				if (ret < 0)
> +					return ret;

This gracefully errs out when the index is unmerged but fails to pretend the index knows about trees, if the unmerged part of index is outside the directory the user specified.

In short, I am not sure if it is worth it, and especially if the motivation is to pretend as if the index contains trees, I would be opposed to it. The index does _not_ contain trees, and cache-tree is a pure optimization for the next write-tree. Nothing more.

If it (pretending as if the index contains trees) is just a means to achieve something else worthy, I would not necessarily oppose to that goal, but I do not see what it is, and I do not know if the approach is right...