[MERGE][bug 104257] Fix _iter_changes to handle unknowns in a previously empty versioned directory

[John Arbash Meinel]

Robert Collins wrote:
> Robert Collins has voted +1 (conditional).
> Status is now: Conditionally approved
> Comment:
> 
> +        # Create an empty directory 'a', followed by a directory with
> content
> +        # 'b'.
> +        self.build_tree(['tree1/a/', 'tree1/b/'])
> +        self.build_tree_contents([('tree1/b/file', 'contents\n')])
> +        self.build_tree(['tree2/a/', 'tree2/b/'])
> +        self.build_tree_contents([('tree2/b/file', 'contents\n')])
> 
> as the content doesn't matter, this would be simpler as
> self.build_tree(['tree1/a/', 'tree1/b/', 'tree1/b/file',
>                  'tree2/a/', 'tree2/b/', 'tree2/b/file',
>                  'tree2/a/file', 'tree2/a/dir/', tree2/a/dir/subfile'])
> 

It would be, but then the contents in tree1 are different from tree2
causing a "content_changed" entry.

I thought it was better to make sure the _iter_changes check only
contained what I was loking for.

> I'm not clear on the last hunk: it looks wrong to remove the if
> want_unversioned guard around that code block - why is it wrong to leave
> it there?
> 
> Other than that, +1.

If you look at the full text it does:

if want_unchanged:
  new_executable = ...
  if want_unchanged:
    result = XXX
    yield result

The extra "if want_unchanged" is just superfluous. It is a shame the
first one is just outside the field of view.

So do you think it is better to have an extra change in the
_iter_changes, to make the setup more obvious?

John
=:->