Re: [PATCH] Add a testcase for the safety of pull-policy='pull'.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 27/02/07, Yann Dirson <ydirson@xxxxxxxxxx> wrote:
On Tue, Feb 27, 2007 at 02:25:57PM +0000, Catalin Marinas wrote:
> This fails (with git 1.5), as expected, but probably not for the same
> reason. See below.

That would demonstrate the usefulness of a python-based testsuite as I
suggested the other day :)

Yes, it would be useful, but probably in addition to the functional
testing we currently do (which might be simplified).

> BTW, push --undo now requires a status --reset beforehand.

Oh, I had not noticed that.  Why so ?  It's not like if "push --undo"
would lose any valuable data...

I added it so that one cannot remove the local changes by doing a
"push --undo" in error. You would have to explicitly ask for local
changes removing with status --reset.

What strikes me most in this case is the difference in behaviour
between this type of conflict (conflict marked in index, so needs an
operation outside the current scope of stgit to proceed), with a
regular stgit conflict that occurs during a push (index clean,
conflict info not available to tools written for regular GIT).

I think this is a valid GIT conflict as the upstream tree re-wrote the
history and git-pull will trigger a 3-way merge. If you would always
use git-fetch + stgit-rebase, there wouldn't be any problem.

That reminds me I already wondered why stgit has to deal with conflict
handling itself.  I've not digged much into the GIT merge mechanism,
but I'd think it would be great if we were able to use it.

That's probably for historical reasons. A year or so ago, porcelain
tools had to handle the merging themselves (if they needed more than
what git-read-tree -m). The only merge algorithm I could easily
understand was the one in Cogito, so I implemented it in StGIT.

Things has changed since then and now GIT can handle the merging
pretty well. StGIT even uses it transparently via git-merge-recursive
called during the 'push' operation ('sync' still uses git-read-tree
because of the better performance). The gitmergeonefile.py still has
all the conflict cases but most of them are handled by
git-merge-recursive and never get here. If this fail, StGIT tries to
use a configured 3-way merge (diff3 by default) and maybe the
interactive merge if the this fails (BTW, I just added a patch for a
2-way interactive merge as well).

An advantage of using the diff3 over the git's 3-way merge is the
configurable labels attached to the in-file conflict markers but I
don't use this feature much anyway as they get overridden by the
interactive merge tool.

We can drop most of this at some point but it is currently still
needed for the 'sync' command using git-read-tree -m. StGIT also
leaves a clean index (i.e. only one-stage files) and marks the
conflict in .git/conflicts. Maybe post 1.0 we can leave the unmerged
entries in the index and re-work this part.

What happenned is our "stg pull" could not complete, so we had to
complete it by hand (instead of attempting to abort it, as my initial
testcase wanted to do).  This makes it look like an "unsafe base
change" (since post_rebase was never called to update "orig-base",
after our manual conflict resolution was done), so any subsequent pull
or rebase will require a --force.

I don't like that either.  I'm still quite unsure how the "git-pull"
model of pulling ought to work, that obviously does not help :)
Any idea ?

I think StGIT behaves correctly since, as I said above, you are
pulling from a tree that has a re-written history. People using GIT
should be able to fix this themselves. For people using StGIT-only, we
should default to the fetch+rebase strategy.

--
Catalin
-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]