Merge lp:~amanica/bzr/mv_after into lp:~bzr/bzr/trunk-old
Status: | Superseded |
---|---|
Proposed branch: | lp:~amanica/bzr/mv_after |
Merge into: | lp:~bzr/bzr/trunk-old |
Diff against target: |
78 lines (has conflicts)
Text conflict in NEWS |
To merge this branch: | bzr merge lp:~amanica/bzr/mv_after |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
John A Meinel | Resubmit on 2009-07-03 | ||
Robert Collins (community) | 2009-05-21 | Needs Fixing on 2009-06-01 | |
Review via email:
|
This proposal has been superseded by a proposal from 2009-07-03.
Marius Kruger (amanica) wrote : | # |
- 4374. By Marius Kruger on 2009-05-21
-
merge with bzr.dev to get news in its place
Robert Collins (lifeless) wrote : | # |
Its not safe to mutate the inventory in this way. You're aliasing the entry in the basis with the working tree, and later mutations will alter both inventories. It will also be copying all the children in which will fail badly if the children weren't all removed.
So the right way to do this is to create a new inventory entry with the right file id, parent id, kind and basename.
John A Meinel (jameinel) wrote : | # |
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Robert Collins wrote:
> Review: Needs Fixing
> Its not safe to mutate the inventory in this way. You're aliasing the entry in the basis with the working tree, and later mutations will alter both inventories. It will also be copying all the children in which will fail badly if the children weren't all removed.
>
> So the right way to do this is to create a new inventory entry with the right file id, parent id, kind and basename.
Isn't InventoryEntry.
preserves file_id, revision, and *doesn't* preserve .children for
directories....
John
=:->
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Using GnuPG with Mozilla - http://
iEYEARECAAYFAko
PKwAn3nUPFHL7Sg
=Tr6D
-----END PGP SIGNATURE-----
- 4375. By Marius Kruger on 2009-07-03
-
merge with bzr.dev moving my news up
- 4376. By Marius Kruger on 2009-07-03
-
clone the the inventory entry when putting it back
Marius Kruger (amanica) wrote : | # |
> Robert Collins wrote:
> > Review: Needs Fixing
> > Its not safe to mutate the inventory in this way. You're aliasing the entry
> in the basis with the working tree, and later mutations will alter both
> inventories. It will also be copying all the children in which will fail badly
> if the children weren't all removed.
> >
> > So the right way to do this is to create a new inventory entry with the
> right file id, parent id, kind and basename.
>
> Isn't InventoryEntry.
> preserves file_id, revision, and *doesn't* preserve .children for
> directories....
I followed John's advice and just .copy() the InventoryEntry now before re-inserting it, which seems correct.
(I tried to add a test to check if it was really cloned, but could not figure out how to compare them in the end. It may be a little overkill in any case.)
I pushed my changes to this branch now, but I can't figure out how to re-submit for review and to update the diff.
(Sorry for being quiet on this. I've been sick, on holiday and busy (in that order))
John A Meinel (jameinel) wrote : | # |
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Marius Kruger wrote:
>> Robert Collins wrote:
>>> Review: Needs Fixing
>>> Its not safe to mutate the inventory in this way. You're aliasing the entry
>> in the basis with the working tree, and later mutations will alter both
>> inventories. It will also be copying all the children in which will fail badly
>> if the children weren't all removed.
>>> So the right way to do this is to create a new inventory entry with the
>> right file id, parent id, kind and basename.
>>
>> Isn't InventoryEntry.
>> preserves file_id, revision, and *doesn't* preserve .children for
>> directories....
>
> I followed John's advice and just .copy() the InventoryEntry now before re-inserting it, which seems correct.
> (I tried to add a test to check if it was really cloned, but could not figure out how to compare them in the end. It may be a little overkill in any case.)
>
> I pushed my changes to this branch now, but I can't figure out how to re-submit for review and to update the diff.
>
> (Sorry for being quiet on this. I've been sick, on holiday and busy (in that order))
review resubmit
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Using GnuPG with Mozilla - http://
iEYEARECAAYFAkp
FQ8AoIfiRDqucFV
=RGNC
-----END PGP SIGNATURE-----
Allow renaming of files already removed from the inventory,
by quickly copying the inventory entry from the basis_tree before doing the rename.
(I'm not too clued up with the inventory so I hope I didn't commit any grave sins).
== history/motivation ==
It happened to me again today that I tried to do a `bzr mv --after old new` but bzr insists that `old is not versioned`. So I debuged it and realised that indeed `old` was
not present in the inventory. This can typically happen if the user copies the file
to the new location an then `bzr remove old`. This happens quite easily if you deleted
it from for eg. eclipse with the bzr plugin installed where you don't even know that it
automatically does a bzr remove for you in the backend.
Once you are in this situation though you start to bang your head against the keyboard
out of frustration after a while. Because according to `bzr status` a file that has
been removed or `bzr remove`d looks the same but for the subtle difference that in the
latter case it is actually removed from the inventory already.
I think in this case we should just go ahead and do what the user means, because the workaround it messy and totally undiscoverable:
`bzr revert old; bzr mv --after old new; rm old`
(especially if your file paths are longer than 80 characters)
I can't find a bug related to this, but I think I've seen this problem on the mailing list.