[patch] bug #43959, bug in commit of merges in checkouts

[John A Meinel]

Martin Pool wrote:
> Fix and test case for a bug reported yesterday by Erik where commit of
> merged revisions fails in a checkout directory.  The root problem is
> that we used the wrong revision object to access the repository and so
> saw slightly out-of-date information.
> 
> Because this substantially impairs checkouts I'm going to make an 
> 0.8.1 release with this fix soon; Robert & I did it in pair so that
> should do for review.  (But comments are still welcome.)
> 
> (As future work this class of errors could probably raise an assertion,
> if you try to access an object using a transaction that doesn't
> correspond to a lock on the object.)
> 
> 

+1, though I didn't see anywhere that you check for misusing the
transaction parameter, just that you fix this particular one, and add a
test case for it. It would be better if the code in question could
assert proper usage.


> 
> ------------------------------------------------------------------------
> 
> === modified file 'bzrlib/commit.py'
> --- bzrlib/commit.py	
> +++ bzrlib/commit.py	
> @@ -512,7 +512,7 @@
>              if ie.revision is None:
>                  change = ie.snapshot(self.rev_id, path, previous_entries,
>                                       self.work_tree, self.weave_store,
> -                                     self.branch.get_transaction())
> +                                     self.branch.repository.get_transaction())
>              else:
>                  change = "unchanged"
>              self.reporter.snapshot_change(change, path)
> 
> === modified file 'bzrlib/inventory.py'
> --- bzrlib/inventory.py	
> +++ bzrlib/inventory.py	
> @@ -656,21 +656,21 @@
>          self.text_sha1 = None
>          self.executable = None
>  
> -    def _snapshot_text(self, file_parents, work_tree, weave_store, transaction):
> +    def _snapshot_text(self, file_parents, work_tree, versionedfile_store, transaction):
>          """See InventoryEntry._snapshot_text."""
> -        mutter('storing file {%s} in revision {%s}',
> -               self.file_id, self.revision)
> +        mutter('storing text of file {%s} in revision {%s} into %r',
> +               self.file_id, self.revision, versionedfile_store)
>          # special case to avoid diffing on renames or 
>          # reparenting
>          if (len(file_parents) == 1
>              and self.text_sha1 == file_parents.values()[0].text_sha1
>              and self.text_size == file_parents.values()[0].text_size):
>              previous_ie = file_parents.values()[0]
> -            versionedfile = weave_store.get_weave(self.file_id, transaction)
> +            versionedfile = versionedfile_store.get_weave(self.file_id, transaction)
>              versionedfile.clone_text(self.revision, previous_ie.revision, file_parents.keys())
>          else:
>              new_lines = work_tree.get_file(self.file_id).readlines()
> -            self._add_text_to_weave(new_lines, file_parents.keys(), weave_store,
> +            self._add_text_to_weave(new_lines, file_parents.keys(), versionedfile_store,
>                                      transaction)
>              self.text_sha1 = sha_strings(new_lines)
>              self.text_size = sum(map(len, new_lines))
> 
> === modified file 'bzrlib/repository.py'
> --- bzrlib/repository.py	
> +++ bzrlib/repository.py	
> @@ -166,6 +166,10 @@
>          self.control_weaves = control_store
>          # TODO: make sure to construct the right store classes, etc, depending
>          # on whether escaping is required.
> +
> +    def __repr__(self):
> +        return '%s(%r)' % (self.__class__.__name__, 
> +                           self.bzrdir.transport.base)
>  
>      def is_locked(self):
>          return self.control_files.is_locked()
> 
> === modified file 'bzrlib/tests/blackbox/test_checkout.py'
> --- bzrlib/tests/blackbox/test_checkout.py	
> +++ bzrlib/tests/blackbox/test_checkout.py	
> @@ -1,5 +1,4 @@
> -# Copyright (C) 2005 by Canonical Ltd
> -# -*- coding: utf-8 -*-
> +# Copyright (C) 2005, 2006 by Canonical Ltd
>  
>  # This program is free software; you can redistribute it and/or modify
>  # it under the terms of the GNU General Public License as published by
> 

Also, while you are fixing this, it seems you should change it to:

# Copyright (C) 2005, 2006 by Canonical Ltd
#     <= add this
# This program is free software; you can redistribute it and/or modify
# it under the terms of the GNU General Public License as published by


John
=:->


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 249 bytes
Desc: OpenPGP digital signature
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20060511/b6f99260/attachment.pgp