Skip to content

refactor: Unify GetTreeEntries code paths

James Liu requested to merge jliu-unify-get-tree-entries into master

#5038 (closed)

Folds the recursive and non-recursive paths in the GetTreeEntries RPC so that:

  • Errors are handled consistently in both cases, and we return the same structured error for the same problem. Previously, we sometimes returned NewInvalidArgument when the revision didn't exist, and NewNotFound when the revision or path were invalid.
  • We use repo.ReadTree in both cases, passing the WithRecursive() option when appropriate. catfile.TreeEntries is no longer used.

The unified code path is self-contained in a separate function which is only invoked when the feature flag is enabled. This simplifies the removal process once the flag is completely rolled out.

Modifies the GetTreeEntries RPC so that an error is returned if the provided path doesn't resolve to a treeish object, for example if a path to a blob is provided. Previously, an empty result set would be returned instead.

So that this works for the recursive case, a preliminary check is added which calls git cat-file -t and verifies the resulting object type. Without this check, the only error we get is fatal: Not a valid object name as we pass both the resolved revision and the path to git-ls-tree. For example:

> git ls-tree 5ba216ff70056be3e7de6f099ef00fa1b099563
fatal: not a tree object

> git ls-tree 5ba216ff70056be3e7de6f099ef00fa1b099563f:README.md
fatal: Not a valid object name 5ba216ff70056be3e7de6f099ef00fa1b099563f:README.md

The change is made to localrepo.ReadTree, but the specific error is only checked in the unified code path of the GetTreeEntries RPC, and thus is only returned when the feature flag is enabled.

Edited by James Liu

Merge request reports