Comment 22 for bug 830947

Revision history for this message
John A Meinel (jameinel) wrote :

I'm trying to think if Robert's fix is the correct one. We might want to merge it anyway, but I have the feeling this is just layering a workaround on top of a workaround.

We have an issue that ('', '', "TREE_ROOT") is being created by the checkout code. And then when iterating we see TREE_ROOT show up in a subdirectory, and we think there is a problem with that.

At a top level the rough breakdown is:
Dirstate.initialize() always creates a checkout with root id of "TREE_ROOT".
In DirStateWorkingTreeFormat.initialize() we have this code:
                basis_root_id = basis.get_root_id()
                if basis_root_id is not None:
                    wt._set_root_id(basis_root_id)
                    wt.flush()

So *if* the basis tree has a root_id, we force the root_id of the newly created tree to the same root id.

Once _set_root_id has been called, we can see "('', '', 'TREE_ROOT')" in id_index["TREE_ROOT"], even though arguably that entry never actually existed.

And tracing into _set_root_id it calls set_path_id() which has this lovely chunk in it:
        # XXX: This was added by Ian, we need to make sure there
        # are tests for it, because it isn't in bzr.dev TRUNK
        # It looks like the only place it is called is in setting the root
        # id of the tree. So probably we never had an _id_index when we
        # don't even have a root yet.
        if self._id_index is not None:
            self._add_to_id_index(self._id_index, entry[0])

That XXX leads me to believe that we don't actually want to add the entry to the index. Note that 'set_path_id()' has explicit logic for checking that the path is the root (if len(path): raise NotImplementedError)).

(I'm the one that wrote that XXX, but that was 2 years ago.)

I think the real fix is to just remove that code. I'll try running the test suite with and without it. If nothing actually fails, then I think the bug is that I merged Ian's change without fully vetting the fallout from it.