local-related methods should verify that WorkingTree is in use

[pared]

• ❗ Have you followed the guidelines in the Contributing to DVC list?

• 📖 Check this box if this PR does not require documentation updates, or if it does and you have created a separate PR in dvc.org with such updates (or at least opened an issue about it in that repo). Please link below to your PR (or issue) in the dvc.org repo.

• ❌ Have you checked DeepSource, CodeClimate, and other sanity checks below? We consider their findings recommendatory and don't expect everything to be addressed. Please review them carefully and fix those that actually improve code or fix bugs.

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

Related to #2974

[Suor]

Alternatively:

class WorkingTree:
    is_working_tree = True

class CleanTree:
    @property
    def is_working_tree(self):
        return self.tree.is_working_tree

class RemoteLOCAL:
    def walk_files(self):
        assert self.repo.tree.is_working_tree

[Suor]

Or an another alternative ;) - create a helper to dry it a bit:

def is_working_tree(tree):
    return isinstance(tree, WorkingTree) or isinstance(getattr(tree, 'tree', None), WorkingTree)

More local.

[pared]

Some notes:

• Why do tests fail now?:
  test_open_external[Local] fails on is_working_tree assertion inside get_mtime_and_size
  This is happening because during the execution of used_cache when pushing for all branches we need to iterate over branches, changing repo.tree to GitTree. The particular file that fails is a cache entry file for directory that we are trying to get checksum for from state. So, in fact, we are trying to get mtime_and_size for file, and not a directory. That is not critical bug as for now, as we are using tree only when calculating dir mtime_and_size. I think we could move is_working_tree assertion after verifying that we want dir mtime_and_size, but I would not leave that as it is. I think we should bump up importance of Move get_mtime_and_size to tree #3011, move get_mtime_and_size to CleanTree as it will probably enforce us to implement tree.stat methods. We should also reconsider state.get, because, as test_open_external shows, it allows situation when we are trying to get some cache file metadata, using GitTree in repo at the same time, which does not seem logical at all, even if its working now, just because get_mtime_and_size uses os.stat.

[ghost]

👍

[efiop]

@pared Let's move that note to that ticket, so we don't forget about it. Also, another way to solve would probably to use repo.wtree or something like that, to clarify that we specifically want to deal with the working tree and not with a regular tree. That seems suitable, as when dealing with cache files or checking out something we are clearly only interested in the working tree.