[attn aaron] differences between RevisionTree and WorkingTree

[John Arbash Meinel]

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

Robert Collins wrote:

> So heres a patch that does this: changes iter_entries to yield the root.
> 
> I hope this will be uncontentious - its a small change, it hasn't broken
> any plugins AFAICT. 
> 
> The tree implementation tests I have planned at this point are primarily
> for ensuring test trees are available, but there are a number of tests
> we can move into there should someone be interested.
> 
> Cheers,
> Rob
> 
> 

...

the tests all look good.


> === modified file bzrlib/commit.py // last-changed:robertc at robertcollins.net-20
> ... 060724033416-f705ee1c86537f2d
> --- bzrlib/commit.py
> +++ bzrlib/commit.py
> @@ -502,10 +502,11 @@
>          # in bugs like #46635.  Any reason not to use/enhance compare_trees?
>          # ADHB 11-07-2006
>          mutter("Selecting files for commit with filter %s", self.specific_files)
> -        # iter_entries does not visit the ROOT_ID node so we need to call
> -        # self._emit_progress_update once by hand.
> +        # at this point we dont copy the root entry:
> +        entries = self.work_inv.iter_entries()
> +        entries.next()
>          self._emit_progress_update()
> -        for path, new_ie in self.work_inv.iter_entries():
> +        for path, new_ie in entries:
>              self._emit_progress_update()
>              file_id = new_ie.file_id
>              mutter('check %s {%s}', path, file_id)

Looking at the comments, it seems that you *don't* want to use
entries.next() in the beginning, instead you should skip the early
_emit_progress_update, and iterate as normal. So something like:

mutter("Selecting ...)
for path, new_ie in self.work_inv.iter_entries():
  self._emit_progress_update()

However, I don't know exactly what the rest of the code does, so you
might be more correct with what you did.

> --- bzrlib/export/dir_exporter.py
> +++ bzrlib/export/dir_exporter.py
> @@ -36,7 +36,9 @@
>      os.mkdir(dest)
>      mutter('export version %r', tree)
>      inv = tree.inventory
> -    for dp, ie in inv.iter_entries():
> +    entries = inv.iter_entries()
> +    entries.next() # skip root
> +    for dp, ie in entries:
>          # .bzrignore has no meaning outside of a working tree
>          # so do not export it
>          if dp == ".bzrignore":

This also seems like it would be better if done as:
if dp in ('', '.bzrignore'):
  continue

Rather than doing the early skip. But I'm fine with the way you did it.
The same thing goes for the rest of the exporters. I suppose the skip at
the beginning is faster than doing a comparison against the rest...

...

Generally speaking, +1 to the patch. I'm not sure that it needs to be in
0.9, but for api sake, it probably is better to break apis earlier
rather than later. So +1 either to get it in now, or later.

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

iD8DBQFEx8pzJdeBCYSNAAMRAmeTAJ9hUmObvZ0EurD31XeUYUkGlaHTcQCgz/WK
y1dmDZqGnLg7VFva4GWeD0w=
=s6kv
-----END PGP SIGNATURE-----