Re: regression in git-gui since 2c5c66b... Merge branch 'jp/get-ref-dir-unsorted

[Michael Haggerty <mhagger@alum.mit.edu>]

On 10/17/2011 12:07 PM, Mark Levedahl wrote:
> Your modification of my script does not show the error for me, unless I
> have *installed* a version of git with the failure: I suspect git-gui
> invokes installed components, and not what is in the build directory, so
> having a good version of git installed with the bad version in the build
> directory does not show the error. And yes, I am quite sure that all of
> the git commands I am running are from the one version.

Yes, you seem to be right.  Even if I set PATH to list my git build
directory before the directory where it is installed, "git-gui"
sometimes invokes git-rev-parse from the libexec path of the installed
version.

When I install the compiled git, then I see the behavior that you describe.

The invocation that behaves differently is

    git ls-files --others -z --exclude-standard

(run in the "super" directory).  It doesn't seem to matter which version
of git is used to create the test repository.  Under 2c5c66b, it outputs
"sub/a", whereas under either of the merge commit's ancestors, the
command outputs "sub/".

    git ls-files --others

I believe that the problem originates in code in
resolve_gitlink_packed_ref() that was invented during the merge 2c5c66b:

static int resolve_gitlink_packed_ref(char *name, int pathlen, const
char *refname, unsigned char *result)
{
	int retval = -1;
	struct ref_entry *ref;
	struct ref_array *array = get_packed_refs(name);

	ref = search_ref_array(array, refname);
	if (ref != NULL) {
		memcpy(result, ref->sha1, 20);
		retval = 0;
	}
	return retval;
}

The problem is that the parameter "name" is not NUL-terminated.  The old
code turned it into a (NUL-terminated) filename via

    strcpy(name + pathlen, "packed-refs");

but the new code passes it (unterminated) to get_packed_refs()

Coincidentally, the old (correct) behavior is restored by a patch that I
submitted earlier today:

    "Pass a (ref_cache *) to the resolve_gitlink_*() helper functions".

Michael

-- 
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/