Okay to invoke merge-recursive with an empty workdir?

Okay to invoke merge-recursive with an empty workdir?

[Jay Soffian]

I have an unattended script which performs automated merges into
several destination branches.

As the repo is fairly large, I'd like not to have to checkout each
destination branch, nor have multiple workdirs.

So my approach is to create a temporary, empty, workdir, using
symlinks to point to the parent repo per git new-workdir.

I then invoke merge-recursive directly inside the temporary workdir.

The question is whether this is safe to do, or whether merge-recursive
expects a fully populated working tree that matches the index. My easy
test cases seem to work correctly, but I'm concerned that I sometimes
get:

Skipped /some/file (merged same as existing)
error: addinfo_cache failed for path '/some/file'
Skipped some/other_file (merged same as existing)
error: addinfo_cache failed for path 'some/other_file'

Which seems to be from this code in merge-recursive.cc:

	if (mfi.clean && !df_conflict_remains &&
	    sha_eq(mfi.sha, a_sha) && mfi.mode == a_mode) {
		int path_renamed_outside_HEAD;
		output(o, 3, "Skipped %s (merged same as existing)", path);
		/*
		 * The content merge resulted in the same file contents we
		 * already had.  We can return early if those file contents
		 * are recorded at the correct path (which may not be true
		 * if the merge involves a rename).
		 */
		path_renamed_outside_HEAD = !path2 || !strcmp(path, path2);
		if (!path_renamed_outside_HEAD) {
			add_cacheinfo(mfi.mode, mfi.sha, path,
				      0 /*stage*/, 1 /*refresh*/, 0 /*options*/);
			return mfi.clean;
		}

(Peff, hope you don't mind me cc'ing you directly. I seem to recall
you're saying github is doing something like this but I couldn't find
that email.)

j.

Re: Okay to invoke merge-recursive with an empty workdir?

[Jeff King]

On Mon, Sep 12, 2011 at 11:13:37PM -0400, Jay Soffian wrote:

> So my approach is to create a temporary, empty, workdir, using
> symlinks to point to the parent repo per git new-workdir.
> 
> I then invoke merge-recursive directly inside the temporary workdir.
> 
> The question is whether this is safe to do, or whether merge-recursive
> expects a fully populated working tree that matches the index. My easy
> test cases seem to work correctly, but I'm concerned that I sometimes
> get:

I'm not sure if we were ever using merge-recursive like that. Especially
with Elijah's latest patches to handle worktree dirtiness better, I
wouldn't be surprised if it has issues.

What we do now at GitHub is something like:

  export GIT_WORK_TREE=/some/tmpdir
  export GIT_INDEX_FILE=/some/tmpfile
  git read-tree -i -m --aggressive $merge_base $us $them
  git merge-index git-merge-one-file -a

I also have a patch to do the content-level merge in read-tree without
touching the filesystem at all. But we're not using it yet, and I should
probably write more tests for it. You can see it at:

  git://github.com/peff/git.git jk/read-tree-content-merge

It should produce the same results as the snippet above, but faster.
Both of those will obviously not handle renames or do recursive merges
like merge-recursive, of course.

-Peff

Re: Okay to invoke merge-recursive with an empty workdir?

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> I'm not sure if we were ever using merge-recursive like that. Especially
> with Elijah's latest patches to handle worktree dirtiness better, I
> wouldn't be surprised if it has issues.

I am parsing the above as "seeing the need for such an extensive fix-up,
it is not surprising if the base code is broken", and I have to agree.

In principle, "git merge" should be usable in an empty working tree in the
sense that with correctly populated index the absense of the working tree
file is _meant_ to be treated the same as having the file unchanged from
the index and the HEAD version, but I suspect that "merge-recursive" is
pretty much broken with that regard.

I have much more faith in "git merge -s resolve" performing correctly.

Re: Okay to invoke merge-recursive with an empty workdir?

[Jeff King]

On Mon, Sep 12, 2011 at 08:55:12PM -0700, Junio C Hamano wrote:

> Jeff King <peff@peff.net> writes:
> 
> > I'm not sure if we were ever using merge-recursive like that. Especially
> > with Elijah's latest patches to handle worktree dirtiness better, I
> > wouldn't be surprised if it has issues.
> 
> I am parsing the above as "seeing the need for such an extensive fix-up,
> it is not surprising if the base code is broken", and I have to agree.

Well, both. I assume that merge-recursive now cares more than ever about
what is in the working tree, because I seem to recall Elijah handling
some cases with untracked files. And also, his patches have given me no
faith whatsoever in the quality of the underlying code. :)

> In principle, "git merge" should be usable in an empty working tree in the
> sense that with correctly populated index the absense of the working tree
> file is _meant_ to be treated the same as having the file unchanged from
> the index and the HEAD version, but I suspect that "merge-recursive" is
> pretty much broken with that regard.
> 
> I have much more faith in "git merge -s resolve" performing correctly.

Agreed, though I haven't tried it.

-Peff