Re: [PATCH] Stgit - gitmergeonefile.py: handle removal vs. changes

From: Catalin Marinas <catalin.marinas@gmail.com>
Date: Nov 17, 2005, 22:10 UTC
Message-ID: <b0943d9e0511171410y357fb0bfv@mail.gmail.com>
In-Reply-To: <200511161544.13825.blaisorblade@yahoo.it>

On 16/11/05, Blaisorblade <blaisorblade@yahoo.it> wrote:

> On Tuesday 15 November 2005 10:54, Catalin Marinas wrote:
> Actually, with .git/commits we are reimplementing handling of "unmerged"
> entries... it could be better to use the "unmerged entry" stgit idea. So "stg
> resolved" should modify the entries by itself.

But would a git-diff-tree still show the changes between the current files and the index if there are unmerged entries? I haven't tried it.

Show 7 quoted lines

> > My initial idea was to make
> > gitmergeonefile not to leave any unmerged entries in the index. As you
> > could see, there are cases where it failed.
>
> Yep... it seems you took examples from git-merge-one-file, but that's lacking
> (but it's low-level so it's appropriate for it - it must leave unmerged
> entries when there are conflicts).

When I started writing StGIT, my main thoughts were driven towards the patch merging/commuting via the diff3 algorithm. I found it simpler to copy the algorithm from git-merge-one-file since that wasn't my main interest in StGIT. I also looked at how Cogito did it.

Show 10 quoted lines

> > I can see the following scenarios for a file:
>
> In both cases, we're going to have a conflict, so we leave file.
> {older,remote,local} as appropriate and already done.
>
> > 1. deleted in the base and modified by the patch. It should leave the
> > file in the tree together with file.older.
>
> Why not leaving file.remote? We already do that in general, so we have a
> duplicate, but it's easier to understand.

I agree with this.

Show 9 quoted lines

> > Another option would be to
> > remove the file and leave both file.older and file.remote in the tree
> > (here .remote means the version in the patch)
>
> I remember that at times, but .remote is very confusing... I see that's the
> mishandling is induced by various sources, maybe including "merge" itself,
> but that program (and possibly others) supports changing the labels, and this
> should probably be done (using "original", "patched" and "upstream"
> probably).

I know that diff3/merge support labels. I don't exactly remember my reasons but I think that I chose those namings because StGIT was supporting another type of merge where "patched" etc. did not apply.

I agree that we should change them. I would rather use "ancestor", "patch" and "base" but I don't have a strong opinion.

Show 9 quoted lines

> > 2. changed in the base but deleted by the patch. It should remove the
> > file from the tree but leave file.older and file.local. The other
> > option is to leave the file in the tree but, as above, I prefer the
> > first one.
>
> The policy about when to remove the file and when to leave it is very
> personal... the user must anyway solve the conflict in some smart way...
> about the defaults, anything would do, but if we really care we could leave
> the user the choice.

At the moment, the conflicts usually leave the index in the state before pushing the patch. I think it should also leave the file and just mark it as conflict in .git/conflicts.

-- Catalin