threads / discuss / 28378

Okay to invoke merge-recursive with an empty workdir?

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

## tl;dr

4 messages between Sep 13, 2011 and Sep 13, 2011.

replies: 3people: 3as markdown or json

Jay Soffian· Sep 13, 2011, 03:13 UTC · lore

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.
Jeff King· Sep 13, 2011, 03:29 UTC · re: Jay Soffian · lore

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

On Mon, Sep 12, 2011 at 11:13:37PM -0400, Jay Soffian wrote:
Show 9 quoted lines
> 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
Junio C Hamano· Sep 13, 2011, 03:55 UTC · re: Jeff King · lore

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

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.
Jeff King· Sep 13, 2011, 03:59 UTC · re: Junio C Hamano · lore

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

On Mon, Sep 12, 2011 at 08:55:12PM -0700, Junio C Hamano wrote:
Show 8 quoted lines
> 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. :)

Show 7 quoted lines
> 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

← back to recent threads