update performs two merges

I'm not sure what you are saying is the bug. Clearly you have been
confused.

In this case, if you had done 'bzr resolved debian/changelog' I expect
it would have addressed the conflicts immediately.

-Rob
--
GPG key available at: <http://www.robertcollins.net/keys.txt>.

I don't think so. I'm pretty sure he did that early on. The problem as I see it is:

At some point there was a conflict in debian/changelog so we created debian/changelog.{THIS,BASE,OTHER}

Someone did a plain 'bzr add' (perhaps because there were other unknowns) and then 'bzr commit'.

At this point, there are versioned files named debian/changelog.THIS {BASE, OTHER}.

Then we get a new conlfict in debian/changelog.

We try to write debian/changelog.THIS, but there is already a versioned file with that name, so we move that file out of the way (debian/changelog.THIS.moved) and create a new debian/changelog.THIS. Whever we move a file because we are trying to create a file in that location, we mark it as conflicted.

So the actual conflict is no debian/changelog. Instead it is debian/changelog.THIS

The fix I would recommend is:

bzr resolve debian/changelog.*
bzr rm debian/changelog.*
(possible rm debian/changelog.*)

bzr commit

I should also mention, that if files have been removed, it is probably reasonable to auto-clear their conflicts.

So if we have a conflict of "file already exists moving existing file to file.moved". If you 'bzr rm file file.moved' it should clear the conflict entry

Shouldn't bzr check that you don't try to add conflict marker files. It could at least give a warning in this case because 99.9999% of the time this will be user error and the conflict marker files should not be added and committed.

I added a script that reproduces this problem to bug #116655. Please have a look.

> update performing two merges can lead to hard to diagnose situations
> and this should be fixed

It's more than that; the merges can also spuriously conflict.

Consider the this script:

-------------------
#!/bin/sh -x
bzr="/usr/local/bin/bzr --no-aliases --no-plugins"

${bzr} init A
(
 cd A ;
 touch foo ;
 ${bzr} add ;
 ${bzr} ci -m 'start' ;
)

${bzr} co A B
(
 # Make one change, and commit it locally
 cd B ;
 echo '1' >> foo ;
 ${bzr} ci --local -m '1';

 # Make another (but don't commit)
 echo '2' >> foo ;

 # Update to cause a doubled up conflict in foo
 ${bzr} update
)
-------------------

Now, if we commit after that '2', then do the update, you get just
what you'd expect; no conflicts, and a pending merge of 2 revs.

But if you DON'T, you get conflicts, and then you get conflicts of the
conflicts, with the conflict markers being part of the conflict!

-------------------
+ /home/fullermd/src/bzr/bzr.dev/bzr --no-aliases --no-plugins update
 M foo
Text conflict in foo
1 conflicts encountered.
 M foo
Text conflict in foo
Conflict adding file foo.BASE. Moved existing file to foo.BASE.moved.
Conflict adding file foo.OTHER. Moved existing file to foo.OTHER.moved.
Conflict adding file foo.THIS. Moved existing file to foo.THIS.moved.
4 conflicts encountered.
Updated to revision 1.
-------------------

And foo looks like

-------------------
<<<<<<< TREE
<<<<<<< TREE
1
2
=======
>>>>>>> MERGE-SOURCE
=======
1
>>>>>>> MERGE-SOURCE
-------------------

Caramba! It seems to do something odd for the first merge, basing on
the working tree verison or something, which conflicts, then the
second merge adds more conflicts, and you end up with
foo.{BASE,OTHER,THIS}.moved and conflicted conflicts in foo itself.
That's well into "user disdainful" territory.

--
Matthew Fuller (MF4839) | <email address hidden>
Systems/Network Administrator | http://www.over-yonder.net/~fullermd/
           On the Internet, nobody can hear you scream.

Whoa, and I thought bug 236724 I just reported was strange. O.o

fullermd's script still reproduces this. This just bit a friend of mine.

There are multiple surprises this causes for the user:

 * that "bzr update" can do two merges which produce conflict files that conflict with each other.
 * the fact that unversioned files can be conflict and prevent a commit.

This bug is seems to be mainly about the first point, but the second point makes the first more severe.

The second point makes it particularly difficult for a user to recover from this error. If a file "foo" gets this double-conflict producing "foo.BASE" and "foo.BASE.moved" etc, the bzr resolve invocation you need is "bzr resolve foo.BASE foo.THIS foo.OTHER". I've seem several victims of the bug trip over this and thus get completely stuck. They intuitively (and reasonably IMO) expect that "bzr resolve foo" should be sufficient to tell bzr that the problem is dealt with. Perhaps issues with conflicts in unversioned files deserve a separate bug report(s)?

I wonder for the first point if a cheap 90% solution would be for the two merges to use different suffixes? e.g. ".local.BASE" vs. ".uncommitted.BASE" instead of both using ".BASE". If you still have detritus from earlier conflicts you may still get conflicts when adding these files, but I think this would help.

I use Bazaar for a single person project, in theory one of the simplest use cases. But I do make local commits, where dragons seem to lie.

I have now had to go to Andrew Bennetts for help, oh, getting on towards ten times now with problems along the lines of "um, it says it can't add foo.BASE, why would it even want to add foo.BASE, that's not meant to be versioned..." or "why is there a foo.BASE and a foo.BASE.moved" and now (with bug 236724) "why is there a content conflict, text conflict AND path conflict all on the same file, all I did was an update?"

Having my working tree stomped on with spurious conflicts matters. It can take me upwards of 45 minutes each time this happens to double-check that all the files are intact and that stuff I wanted hasn't vanished into foo.BASE.moved.

Unless/until the problems with the checkout model are resolved, I advocate that Bazaar not allow the "update" command to be used as readily in checkouts. Certainly it seems that "update" should essentially never be run in a working tree of a checkout with both unmerged local commits and totally uncommited changes. I would much rather deal with "Please commit all local changes with commit --local before updating from the master branch" than try to remember AGAIN which of foo.{BASE,OTHER,THIS}.moved are likely to be important and which ones I need to call "bzr resolve" on.

On Sun, Sep 28, 2008 at 5:04 PM, Mary Gardiner
<email address hidden> wrote:
> Unless/until the problems with the checkout model are resolved, I
> advocate that Bazaar not allow the "update" command to be used as
> readily in checkouts. Certainly it seems that "update" should
> essentially never be run in a working tree of a checkout with both
> unmerged local commits and totally uncommited changes. I would much
> rather deal with "Please commit all local changes with commit --local
> before updating from the master branch" than try to remember AGAIN which
> of foo.{BASE,OTHER,THIS}.moved are likely to be important and which ones
> I need to call "bzr resolve" on.

I think that could make sense. Alternatively perhaps it should just
do one merge at a time, then prompt the user to run update again.

--
Martin <http://launchpad.net/~mbp/>

As a user, this is also the reason I never use "bzr up" even in checkouts. It would be nice if "bzr update" could just print an error if the local branch has diverged from the master branch when updating and suggest the user some options: bzr merge <master-url>, bzr merge --swap <master-url>, bzr rebase <master-url>. That way it never has to do two merges itself.

I'd also fancy such a solution described by Jelmer.
Seems to be an intelligent software if it suggest how to solve such update problems.

I often create a copy of the folder before doing bzr update because I fear this merge nightmare.

Is there a reason this bug isn't getting more attention? Is there a preferred workflow that avoids this issue and mitigates the problem?

>Is there a reason this bug isn't getting more attention? Is there a preferred workflow that avoids this issue and mitigates the problem?

there is heavy discussion about this in the mailinglist at the monent eg:
http://www.nabble.com/Recommended-use-of-Bazaar-for-single-committer-multiple-machine-projects--td20968005.html

I've emailed a merge directive to Aaron's BundleBuggy. I've attached the bundle here for anyone interested.

David: perhaps you should submit a launchpad merge request? We seem to be moving more towards that these days...

Is there an agreed solution to this problem?
If bazaar can give an error message telling the user to first commit local changes before running the update, it would be a good enough solution. (obviously this must only happen when there are uncommitted files that where locally committed.)

Looked at David's merge directive and I have 1 recommendation:

It should not "look" for uncommitted changes if there are no local commits.
i.e:
if local_extra:
 // then look for uncommitted changes.

This will improve performance. For normal use of update.

I'm really keen to see this change put into trunk ASAP, since this is a major issue for the developers at our company. And I'm in the process of getting our entire company to use Bazaar and would like them to not run into this problem in the future.

Bazaar should also try to prevent the user accidentally adding and committing merge conflict files (i.e the THIS, OTHER, BASE files). If there is a file that is versioned and the user adds the THIS, OTHER, BASE files then Bazaar should either ask the user if that's what they really meant, print a message telling the user they just did something strange, or refuse to add those files and if the user really wants to add them then they need to pass some option to the add command.

I think this would prevent a lot of these merge problems in the first place.

I agree that bazaar should not allow the user to add THIS, BASE and OTHER files (except if overridden), but can't this be logged as a different bug? So that we can get David's fix sooner.

Also:
Being told (when you do an update) that you can't add the OTHER, THIS and BASE files might confuse the user, because as far as he knows he did a local commit, changed the file again and then did an update.

Sure, there is bug #414589 for "Don't add conflict related files".

I've made use of David's Strauss's merge with a minor change:
lp:~craighewetson/bzr/update_with_local_commit

I'm using the change as I speak and it works very well. Will need people to test it and give feed back on the merge proposal.

I have just experienced thius another time:
the 2 meges let me waste time for resolving conflicts that are silly. why a dvcs when I cannot commit locally withou hastle?

Please tell me what to do inorder to test it.

Hi Tim

Please tell me what to do inorder to test it.
>
>

Well you would need to install the source code from this branch. If you make
use of the python based installation of bazaar it would be easier to install
the source.
1) bzr branch --stacked lp:~craighewetson/bzr/update_with_local_commit
2) python setup.py install

Alternatively:
A quick and dirty way to test it would be to have bzr 1.17.1 installed and
then just backup your existing builtins.py and copy the builtins.py from the
above branch in to the bzrlib directory.

Test this bug for a test project and then once you are done restore you
builtins.py file.

BTW:
I haven't bothered changing the file back and I actively use bzr 1.17.1 with
this patched file, without any problems.

If you have any questions or need me to compile the code into an exe then
let me know.

Unassigned (9 months idle)

I know I'm sounding a bit like a broken record, but can someone review the changes in:
lp:~craighewetson/bzr/update_with_local_commit

Rejecting the fix with reason is also a welcomed answer.

See comments in https://code.edge.launchpad.net/~craighewetson/bzr/update_with_local_commit/+merge/10320 for progress on this bug.

John, I'm not sure where this was discussed previously, but all the bugs seem to be coalescing here.. Anyhow you proposed doing a single merge.

I don't see how that can work: we have in the pathological case 3 heads:
- current tree rev
- current branch tip
- current master tip

Aaron suggested a long time ago that if the first merge conflicts, we should just stop there.

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Robert Collins wrote:
> John, I'm not sure where this was discussed previously, but all the bugs
> seem to be coalescing here.. Anyhow you proposed doing a single merge.
>
> I don't see how that can work: we have in the pathological case 3 heads:
> - current tree rev
> - current branch tip
> - current master tip

$ bzr update
tree rev => branch tip
note: tree is up to date with local branch, but still out-of-date with
remote branch, run bzr update again
$ bzr update
tree rev + branch tip => master tip

>
> Aaron suggested a long time ago that if the first merge conflicts, we
> should just stop there.
>

Sure. I think as for the specific branch that was reviewed, it blocked
if you had local commits and uncommitted changes, as that was easier to
detect and get working, and prevented some of the really bad
double-conflicted sections.

John
=:->
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAktYwnEACgkQJdeBCYSNAAP2KwCgqaO7dsytUtGZ5DuVmcT1ZZQI
5CEAmgLgfFJOcippP8eK8e2Nwppjvu4N
=A39P
-----END PGP SIGNATURE-----

Patch based on the one by David, also has blackbox tests.

I think this is not the right solution though, it's the update logic that needs to be fixed. The lightweight checkout case is especially tricky, as you can't require a local commit first because you don't have those for a lightweight.

On Thu, 2010-01-21 at 23:49 +0000, Gerard Krol wrote:
>
>
> I think this is not the right solution though, it's the update logic
> that needs to be fixed. The lightweight checkout case is especially
> tricky, as you can't require a local commit first because you don't
> have
> those for a lightweight.

But you only ever do one merge if the branch isn't bound, so it doesn't
need to block it either.

-Rob