tree: open: handle dirty state in RepoTree

[efiop]

We used to use that logic in DvcTree back before we had proper trees.
Now we have a RepoTree, that combines git/workspace tree with the
DvcTree and dirty state clearly belongs to the git/workspace and not
the dvc itself, as dvc state only depends on dvc-files and lock-files.

Prerequisite for #4523

• ❗ I have followed the Contributing to DVC checklist.

• 📖 If this PR requires documentation updates, I have created a separate PR (or issue, at least) in dvc.org and linked it here.

Thank you for the contribution - we'll try to review it as soon as possible. 🙏

[pmrowla]

This looks good for dirty state w/open(), but we still don't handle dirty dirs in RepoTree.walk().

If I add the following test to tests/unit/tree/test_repo.py:

def test_walk_dirty(tmp_dir, dvc, scm):
    from dvc.utils.fs import remove

    tmp_dir.dvc_gen(
        {"dir": {"foo": "foo1", "bar": "bar1"}},
        commit="init",
    )
    tmp_dir.gen({"dir": {"bar": "bar2", "baz": "baz1"}})
    remove(str(tmp_dir / "foo"))
    tree = RepoTree(dvc)

    expected = [
        PathInfo("dir") / "bar",
        PathInfo("dir") / "baz",
    ]

    actual = []
    for root, dirs, files in tree.walk("dir"):
        for entry in dirs + files:
            actual.append(os.path.join(root, entry))

    assert (tmp_dir /"dir" / "bar").read_text() == "bar2"
    expected = [str(path) for path in expected]
    assert set(actual) == set(expected)
    assert len(actual) == len(expected)

We get

E       AssertionError: assert {'dir/baz', 'dir/foo', 'dir/bar'} == {'dir/baz', 'dir/bar'}
E         Extra items in the left set:
E         'dir/foo'
E         Full diff:
E         - {'dir/baz', 'dir/bar'}
E         + {'dir/baz', 'dir/foo', 'dir/bar'}
E         ?             +++++++++++

/Users/pmrowla/git/dvc/tests/unit/tree/test_repo.py:524: AssertionError

So for a dirty directory and RepoTree.walk() we handle modified and added files, but not removed files. DvcTree.walk() yields every expected file in the directory (as it probably should), but RepoTree.walk() does not check to make sure the dvctree files still exist in the dirty local workspace.

[efiop]

@pmrowla Correct, this is me just starting out migrating the existing dirty functionality from DvcTree to RepoTree. Walk, get_hash and other calls will need to be fixed. So we are on the same page here.

[efiop]

@pmrowla btw, your test seems incorrect. You delete tmp_dir / "foo", but you probably meant tmp_dir / "dir" / "foo", right?

[pmrowla]

@pmrowla btw, your test seems incorrect. You delete tmp_dir / "foo", but you probably meant tmp_dir / "dir" / "foo", right?

Good catch, that was my mistake, I tried it again with the fixed test and it passes 👍