Fix #6813: Allow git.GetTree to take both commit and tree names

[filipnavara]

Fixes two separate bugs reported in #6813:

• Allow git.GetTree to take both commit and tree names; this matches GitHub behavior on the Trees API
• Return full paths on entries listed through Tree.ListEntriesRecursive. Technically this is abuse of the API but I am just doing a hot-fix here. A proper fix would be to move the recursive enumeration to different layer and add unit tests for it.

[lafriks]

Looks like unit tests needs to be updated as well

[filipnavara]

I will look into the failures later today. It should not have broken any tests...

[filipnavara]

Ah, I propagate wrong SHA into the Tree object now. :/

[codecov-io]

Codecov Report

Merging #6816 into master will increase coverage by 0.01%.
The diff coverage is 54.54%.

@@            Coverage Diff             @@
##           master    #6816      +/-   ##
==========================================
+ Coverage   41.21%   41.23%   +0.01%     
==========================================
  Files         421      421              
  Lines       58120    58125       +5     
==========================================
+ Hits        23956    23966      +10     
+ Misses      30993    30988       -5     
  Partials     3171     3171

Impacted Files	Coverage Δ
modules/git/tree_entry.go	71.18% <0%> (-1.86%)	⬇️
modules/git/tree.go	46.05% <0%> (-0.62%)	⬇️
modules/git/repo_tree.go	81.25% <100%> (+19.95%)	⬆️
modules/repofiles/tree.go	84% <100%> (ø)	⬆️
modules/log/file.go	75.52% <0%> (-2.1%)	⬇️
routers/repo/view.go	43.03% <0%> (+1.01%)	⬆️
models/repo_list.go	67.89% <0%> (+1.05%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a27d5d2...d3b17f9. Read the comment docs.

[zeripath]

Hmm...

I wonder would it better to use:

object, err := repo.gogitRepo.Object(plumbing.AnyObject, plumbing.Hash(id))
if err != nil {
    return nil, err
}

And then test the type of the object, if it's a Commit - then grab the Tree, if it's a Tree cool.

If it's a Blob then there's likely nothing we can do

If it's a Tag then we can check the target -> Commit-> Tree, Tree, and so on.

[filipnavara]

@zeripath Perhaps. I don't like the code as-is and I like your way of restructuring it a bit more than whatever is in this PR. Then again, before doing any bigger change I would prefer to update the unit tests to test those cases. The tests currently don't test tree hashes, tag hashes (which don't work; thanks for pointing it out) or the error case with blob hash.

[lunny]

@filipnavara It seems we should also change the test if there is one.

[lafriks]

@filipnavara @zeripath that most probably would ok to be in other PR