Re: [PATCH/RFC] avoid accessing _all_ loose refs in git-show-ref

Johannes Schindelin [off-list ref] writes:

	This is kind of quick and dirty.

Maybe I am blind, but why don't you just do a resolve_ref()
regardless of packedness?  It only builds the packed refs list
when it is not found as a loose ref, and never builds the loose
refs list.  You probably would want to reject the ones that says
REF_ISSYMREF.

	An alternative would be to pack the tags _per default_. I once
	argued for that, but it appears nobody liked that idea. I even
	proposed to pack _all_ refs, and I still think this would be
	a good idea.

I do not think packing all refs is such a good idea.  Branches
are meant to be worked on and extended so the packed ones
would become stale quickly.  Packing all tags by default is
something we would want when everybody knows how to handle
them.

	Isn't it a bug that --verify succeeds, if only _one_ ref passed to 
	the command exists?

I think --verify should insist a single parameter, just like
rev-parse.

So wouldn't the code be like:

	if (verify) {
		int flag;
        	if (pattern[1])
                	die("Eh?");
		if (resolve_ref(pattern[0], sha1, 1, &flag) &&
                	(flag & REF_ISSYMREF) == 0) {
                		printf("%s %s\n",
                                	sha1_to_hex(sha1), pattern[0]);
				exit(0);
		} else
                	die("no match");
	}

???

Hi,

On Sat, 16 Dec 2006, Junio C Hamano wrote:

Johannes Schindelin [off-list ref] writes:

quoted

	This is kind of quick and dirty.

Maybe I am blind, but why don't you just do a resolve_ref()
regardless of packedness?  It only builds the packed refs list
when it is not found as a loose ref, and never builds the loose
refs list.  You probably would want to reject the ones that says
REF_ISSYMREF.

But of course!

quoted

	An alternative would be to pack the tags _per default_. I once
	argued for that, but it appears nobody liked that idea. I even
	proposed to pack _all_ refs, and I still think this would be
	a good idea.

I do not think packing all refs is such a good idea.  Branches
are meant to be worked on and extended so the packed ones
would become stale quickly.

My idea would have been to edit them in-place. Since the hashes all take 
the same amount of bytes, it could be done by seeking around in the file.

 Packing all tags by default is something we would want when everybody 
knows how to handle them.

You mean all the scripts? Because if you make it the default, you could 
hide that implementation detail away from the user.

quoted

	Isn't it a bug that --verify succeeds, if only _one_ ref passed to 
	the command exists?

I think --verify should insist a single parameter, just like
rev-parse.

So wouldn't the code be like:

	if (verify) {
		int flag;
        	if (pattern[1])
                	die("Eh?");
		if (resolve_ref(pattern[0], sha1, 1, &flag) &&
                	(flag & REF_ISSYMREF) == 0) {
                		printf("%s %s\n",
                                	sha1_to_hex(sha1), pattern[0]);
				exit(0);
		} else
                	die("no match");
	}

???

Ack. Scrap my patch.

Ciao,
Dscho