tree transform cannot change root id

Bug #494269 reported by James Westby on 2009-12-09
62
This bug affects 6 people
Affects Status Importance Assigned to Milestone
Bazaar
High
John A Meinel
2.0
High
John A Meinel

Bug Description

Hi,

bzr merge-package of lp:debian/smarty in to lp:ubuntu/smarty gives:
deleted misc
...
Path conflict: misc / misc

"bzr st":
removed:
misc/
...
conflicts:
  Path conflict: misc / misc

ls misc*:
No matches: misc*

what's the conflict here? Why did it happen? What should I do to resolve
it, except just doing "bzr resolve misc", as that's all I can see to do?

Thanks,

James

Related branches

James Westby (james-w) on 2009-12-09
tags: added: udd

> What should I do to resolve it, except just doing "bzr resolve misc", as that's all I can see to do?

Did that work?

It may be that the problem is there are ignored files inside misc, in which case this is probably a dupe.

summary: - Incomprehensible merge conflicts
+ conflict on deleted directory?
Changed in bzr:
importance: Undecided → Medium
status: New → Confirmed
tags: added: conflicts

On Wed Dec 09 01:37:38 UTC 2009 Martin Pool wrote:
> > What should I do to resolve it, except just doing "bzr resolve misc",
> as that's all I can see to do?
>
> Did that work?

Yes.

> It may be that the problem is there are ignored files inside misc,

Well, I just did branch+merge-package, so I don't think that's the
reason. Also, there's no .other or anything.

Thanks,

James

John A Meinel (jameinel) wrote :

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

James Westby wrote:
> On Wed Dec 09 01:37:38 UTC 2009 Martin Pool wrote:
>>> What should I do to resolve it, except just doing "bzr resolve misc",
>> as that's all I can see to do?
>>
>> Did that work?
>
> Yes.
>
>> It may be that the problem is there are ignored files inside misc,
>
> Well, I just did branch+merge-package, so I don't think that's the
> reason. Also, there's no .other or anything.
>
> Thanks,
>
> James
>

I can't reproduce this. If I do:

bzr co lp:debian/smarty debian \
  -r revid:<email address hidden>

bzr co lp:ubuntu/smarty ubuntu \
  -r revid:<email address hidden>

# Note that this is rev 16, not the rev 17 tip, since that has already
# merged debian

cd ubuntu
bzr merge ../debian

$ bzr st
removed:
  misc/
  misc/smarty_icon.README
  misc/smarty_icon.gif
  unit_test/
  unit_test/README
  unit_test/cache/
  unit_test/config.php
  unit_test/configs/
  unit_test/configs/globals_double_quotes.conf
  unit_test/configs/globals_single_quotes.conf
  unit_test/smarty_unit_test.php
  unit_test/smarty_unit_test_gui.php
  unit_test/templates/
  unit_test/templates/assign_var.tpl
  unit_test/templates/constant.tpl
  unit_test/templates/index.tpl
  unit_test/templates/parse_math.tpl
  unit_test/templates/parse_obj_meth.tpl
  unit_test/templates_c/
  unit_test/test_cases.php
modified:
  NEWS
  README
  debian/Makefile
  debian/changelog
  libs/Config_File.class.php
  libs/Smarty.class.php
  libs/Smarty_Compiler.class.php
unknown:
  debian/changelog.BASE
  debian/changelog.OTHER
  debian/changelog.THIS
conflicts:
  Text conflict in debian/changelog
pending merge tips: (use -v to see all merge revisions)
  Thijs Kinkhorst 2009-10-24 [merge] * Non-maintainer upload.

So there is a conflict in debian/changelog, and 'misc' has been removed,
but no other problems.

Now if I do: touch misc/temp-file

And then do the merge, I get:
  Conflict: can't delete misc because it is not empty. Not deleting.

Which seems sane enough. And doing "bzr resolve misc" will resolve that
conflict.

Perhaps 'merge-package' is doing something fancier which is causing the
breakage?

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

iEYEARECAAYFAksf5J4ACgkQJdeBCYSNAAMX2gCgtzEgykWek1D6YsqHmu9tdCt3
O+IAoIXzCjI+Ir55rcmitefWfrmoWkOb
=cXkp
-----END PGP SIGNATURE-----

James Westby (james-w) wrote :

On Wed Dec 09 17:55:42 UTC 2009 John A Meinel wrote:
> Perhaps 'merge-package' is doing something fancier which is causing the
> breakage?

Yes, it is. It's still just merging revisions of the two branches together,
but in a different manner.

You can do the steps yourself with (starting in the Ubuntu branch):

    bzr push ../upstream -r tag:upstream-2.6.22
    cd ../upstream
    bzr merge lp:debian/smarty -r tag:upstream-2.6.26
    bzr revert -r branch:lp:debian/smarty
    bzr ci -m "merge upstreams"
    cd ../smarty
    bzr merge ../upstream
Doing this shows something odd, the first push gives an unclean
status:

added:
  debian/
  debian/Makefile
  debian/NEWS
  debian/README.Debian
  debian/changelog
  debian/compat
  debian/control
  debian/copyright
  debian/dirs
  debian/docs
  debian/lintian-override
  debian/rules
renamed:
  BUGS => BUGS
  COPYING.lib => COPYING.lib
  ChangeLog => ChangeLog
  FAQ => FAQ
  INSTALL => INSTALL
  NEWS => NEWS
  QUICK_START => QUICK_START
  README => README
  RELEASE_NOTES => RELEASE_NOTES
  TODO => TODO
  demo/ => demo/
  libs/ => libs/
  misc/ => misc/
  unit_test/ => unit_test/
modified:
  libs/plugins/function.math.php

It appears as though there are changing root ids between some of the
strands here.

Thanks,

James

John A Meinel (jameinel) wrote :

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

James Westby wrote:
> On Wed Dec 09 17:55:42 UTC 2009 John A Meinel wrote:
>> Perhaps 'merge-package' is doing something fancier which is causing the
>> breakage?
>
> Yes, it is. It's still just merging revisions of the two branches together,
> but in a different manner.
>
> You can do the steps yourself with (starting in the Ubuntu branch):
>
> bzr push ../upstream -r tag:upstream-2.6.22
> cd ../upstream
> bzr merge lp:debian/smarty -r tag:upstream-2.6.26
> bzr revert -r branch:lp:debian/smarty
> bzr ci -m "merge upstreams"
> cd ../smarty
> bzr merge ../upstream
> Doing this shows something odd, the first push gives an unclean
> status:
>

I don't understand how push can give an unclean status. If the tree was
clean before the push, the push should create a direct update that
leaves the tree clean afterwards.

Then again, we certainly may have a bug, especially if it involves
changing the tree root id.

John
=:->

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

iEYEARECAAYFAksij+AACgkQJdeBCYSNAAPJzwCdF00WvL1DAb3kNtoObbY62OVS
wrkAoIZs5MZ6p4agpZ/rYBSaUCvEmY/V
=oG6f
-----END PGP SIGNATURE-----

James Westby (james-w) wrote :

On Fri Dec 11 18:30:56 UTC 2009 John A Meinel wrote:
> I don't understand how push can give an unclean status. If the tree was
> clean before the push, the push should create a direct update that
> leaves the tree clean afterwards.

This was a push to a new directory.

Thanks,

James

> You can do the steps yourself with (starting in the Ubuntu branch):
>
> bzr push ../upstream -r tag:upstream-2.6.22
> cd ../upstream
> bzr merge lp:debian/smarty -r tag:upstream-2.6.26
> bzr revert -r branch:lp:debian/smarty
> bzr ci -m "merge upstreams"
> cd ../smarty
> bzr merge ../upstream

If I do this manually, I get a failure at the line:
  bzr revert -r branch:lp:debian/smarty

Specifically, I get a huge traceback ending in:
  File "C:\Users\jameinel\dev\bzr\bzr.dev\bzrlib\transform.py", line 170, in adjust_path
    raise ValueError("Cannot have multiple roots.")
ValueError: Cannot have multiple roots.

Though I'm doing it in a treeless repository where I use "bzr co" after "cd ../upstream".

If I change it to use trees in the repo, and start in a checkout of 'ubuntu', then the push gives me the very surprising:

$ bzr push ../upstream -r tag:upstream-2.6.22
Path conflict: misc / misc
Path conflict: unit_test / unit_test
2 conflicts encountered.
Created new branch.

Even doing

$ rm -rf ../upstream; bzr push -r tag:upstream-2.6.22 ../upstream
Path conflict: misc / misc
Path conflict: unit_test / unit_test
2 conflicts encountered.
Created new branch.

I don't understand how 'push' creating a new working tree can have this problem. Perhaps it has something to do with not creating the working tree under the right version? (Or it creates it as an exact clone of the 'ubuntu' tree, and then tries to revert it to the tagged revision, etc.)

Anyway, definitely some strage mojo happening. I'll try to investigate further.

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

I also found that doing:

bzr co --lightweight ubuntu -r tag:upstream-2.6.22 test-co
cd test-co
bzr up

Resulted in an unclean 'status':
$ bzr st
renamed:
  BUGS => BUGS
  COPYING.lib => COPYING.lib
  ChangeLog => ChangeLog
  FAQ => FAQ
  INSTALL => INSTALL
  NEWS => NEWS
  QUICK_START => QUICK_START
  README => README
  RELEASE_NOTES => RELEASE_NOTES
  TODO => TODO
  debian/ => debian/
  demo/ => demo/
  libs/ => libs/

Which is *really* strange. As update from a pristine tree should always
result in a pristine tree (modulo issues with ignored files).

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

iEYEARECAAYFAktE3swACgkQJdeBCYSNAAPUQQCgxCb7qL1ulVkRMgxGiTJgSJmd
7Z4AoKQgTSeHhpCn9Gx0LWa2X8oyeB8u
=bdCc
-----END PGP SIGNATURE-----

As near as I can tell, this is primarily just a bug in our handling of tree root changes. The dirstate file has this record:
[(('', '', 'tree_root-20090718094551-uvi5mimdaxki5fki-1'),
  [('a', '', 0L, 0, ''),
   ('d',
    '',
    0L,
    0,
    '<email address hidden>')]),
 (('', '', 'tree_root-20090718094551-uvi5mimdaxki5fki-143'),
  [('d', '', 0L, 0, 'AAAQAEtE3EpLRNw/AAAAAAAAAAAAAEH/'),
   ('a', '', 0L, 0, '')])]

Which AIUI, says that the root directory has a value of "tree_root-20090718094551-uvi5mimdaxki5fki-1" in the basis-tree, but a value of 'tree_root-20090718094551-uvi5mimdaxki5fki-143' in the workingtree.

A "bzr revert" fails:
    raise ValueError("Cannot have multiple roots.")
ValueError: Cannot have multiple roots.

So somehow Update was able to tell the workingtree to change the root-id, but is unable to make that actually stick.
This also follows with "wt.inventory.root" vs "wt.basis_tree().inventory.root".

Note that "wt.branch.repository.revision_tree(wt.last_revision()).inventory.root" claims the -1 revision.

It is true that revision_tree(tag:upstream-2.6.22).inventory.root does claim -143 as the root id.

My quick probing seems to say that most "upstream" versions use the -143 root-id and the non-upstream versions use the -1 root-id.

This may be related to a similar bug. Where Alexander Belchenko noted that "bzr init && bzr add --file-ids-from" doesn't work perfectly in 2a. Because each tree gets a different root id.

In a quick test, it looks like "bzr init" creates a randomly generated root-id. And so "add --file-ids-from" doesn't get a chance to set the root id to something special.

So some ideas:

1) However you are creating these imports, we should try to make sure the root ids are the same. This probably needs to happen at 'initialization' time.

2) Our code should handle root-ids changing in a better way. Updating from a revision with one root-id to a revision with a different root-id should not leave the tree in a state that thinks it still is using the old revision-id. And reverting when there is a root-id change should 'just work' not leave us in a broken state.

Note that after update and getting into the weird state, if I do:

wt = WorkingTree.open('.')
wt.lock_write()
bt = wt.basis_tree()
bt.lock_read()
wt.set_root_id(bt.inventory.root.file_id)
bt.unlock()
wt.unlock()

It clears up the issues with 'bzr status' and 'bzr revert'. So it would seem that "update" and "revert" aren't calling set_root_id() correctly.

On Wed, 2010-01-06 at 21:38 +0000, John A Meinel wrote:
>
> Which AIUI, says that the root directory has a value of
> "tree_root-20090718094551-uvi5mimdaxki5fki-1" in the basis-tree, but a
> value of 'tree_root-20090718094551-uvi5mimdaxki5fki-143' in the
> workingtree.

working is column 0; basis is column 1; the Nth parent is column N.

-Rob

John A Meinel (jameinel) wrote :

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

Robert Collins wrote:
> On Wed, 2010-01-06 at 21:38 +0000, John A Meinel wrote:
>> Which AIUI, says that the root directory has a value of
>> "tree_root-20090718094551-uvi5mimdaxki5fki-1" in the basis-tree, but a
>> value of 'tree_root-20090718094551-uvi5mimdaxki5fki-143' in the
>> workingtree.
>
>
> working is column 0; basis is column 1; the Nth parent is column N.
>
> -Rob
>

Sure, I got them backwards in my pasting. The important bit is that they
disagree, and that

a) bzr revert pukes
b) wt.set_root_id(basis_tree.inventory.root.file_id) fixes it

John
=:->

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

iEYEARECAAYFAktFCqEACgkQJdeBCYSNAAOX/QCfanaDiTqbeJRk3TZ5PQesk/al
vo0AoJQ7i0qHd7M1apDrhmCiIqNBifAg
=FnE+
-----END PGP SIGNATURE-----

John A Meinel (jameinel) on 2010-01-06
summary: - conflict on deleted directory?
+ tree transform cannot change root id
James Westby (james-w) wrote :

On Wed, 06 Jan 2010 21:38:27 -0000, John A Meinel <email address hidden> wrote:
> This may be related to a similar bug. Where Alexander Belchenko noted
> that "bzr init && bzr add --file-ids-from" doesn't work perfectly in 2a.
> Because each tree gets a different root id.
>
> In a quick test, it looks like "bzr init" creates a randomly generated
> root-id. And so "add --file-ids-from" doesn't get a chance to set the
> root id to something special.

Yep, I discovered this too when we moved to 2a.

> So some ideas:
>
> 1) However you are creating these imports, we should try to make sure
> the root ids are the same. This probably needs to happen at
> 'initialization' time.

There is code that is supposed to handle this, but it may be incomplete.

> 2) Our code should handle root-ids changing in a better way. Updating
> from a revision with one root-id to a revision with a different root-id
> should not leave the tree in a state that thinks it still is using the
> old revision-id. And reverting when there is a root-id change should
> 'just work' not leave us in a broken state.

Thanks,

James

John A Meinel (jameinel) wrote :

So technically these are all duplicates of bug #282947 but since I started marking dupes here, and this is marked UDD, etc, I'm relocating dupes to here.

John A Meinel (jameinel) wrote :

I think I have a handle on what is going on, and how to fix it. I just need to sort it through.

In brief, I think the code is set up to disallow 2 root ids, which is valid. However it does so too early. It should allow you to set up a new root, move everything into it, and delete the old root. At which time it can see that things are a no-op. And only at 'apply' time does it need to say "but you left 2 active roots, I cannot support that."

Changed in bzr:
assignee: nobody → John A Meinel (jameinel)
status: Confirmed → In Progress
importance: Medium → High
John A Meinel (jameinel) wrote :

The associated branch has a fix for TreeTransform itself and for "bzr revert" in particular.

It seems that 'bzr update' is still broken, but the cause is based around bug #504390, so I'll work on that separately.

Changed in bzr:
status: In Progress → Fix Committed
John A Meinel (jameinel) wrote :

The associated fix for 'bzr revert' at least has landed. See bug #504390 for the fixes to 'bzr update' and 'bzr pull'.

Changed in bzr:
milestone: none → 2.1.0rc1
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers