'bzr reconcile' unsafe for stacked 2a? (ignores parent inventories)

Bug #615236 reported by Andrew Bennetts
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Bazaar
Confirmed
Medium
Unassigned

Bug Description

Compare GCCHKPacker and GCCHKReconcilePacker:

    def _copy_inventory_texts(self):
        source_vf, target_vf = self._build_vfs('inventory', True, True)
        # It is not sufficient to just use self.revision_keys, as stacked
        # repositories can have more inventories than they have revisions.
        # One alternative would be to do something with
        # get_parent_map(self.revision_keys), but that shouldn't be any faster
        # than this.
        inventory_keys = source_vf.keys()
        missing_inventories = set(self.revision_keys).difference(inventory_keys)
        if missing_inventories:
            missing_inventories = sorted(missing_inventories)
            raise ValueError('We are missing inventories for revisions: %s'
                % (missing_inventories,))
        self._copy_stream(source_vf, target_vf, inventory_keys,
                          'inventories', self._get_filtered_inv_stream, 2)

vs.

    def _copy_inventory_texts(self):
        source_vf, target_vf = self._build_vfs('inventory', True, True)
        self._copy_stream(source_vf, target_vf, self.revision_keys,
                          'inventories', self._get_filtered_inv_stream, 2)
        if source_vf.keys() != self.revision_keys:
            self._data_changed = True

Off the top of my head I'd expect the missing_inventories logic is just as necessary in the reconcile case, so at a glance it looks like 'bzr reconcile' on a stacked repository could discard parent inventories, leading to the old 'AbsentContentFactory' bug fetching from affected repositories.

(If my understanding is wrong then I'd say there's still a bug: that the code needs a comment that explains why it is reasonable for these two implementations of _copy_inventory_texts to diverge so much.)

I'll propose a branch making what I think is the obvious fix for discussion. I haven't written a test yet because the scenario seems pretty fiddly to create, but perhaps it's easier than it looks.

Related branches

Jelmer Vernooij (jelmer)
tags: added: check-for-breezy
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.