From f40d075425e0f83e919ef3413dadf231edf1efa7 Mon Sep 17 00:00:00 2001 From: delvh Date: Tue, 16 Jan 2024 19:46:07 +0100 Subject: [PATCH 1/5] Use the real link location of symlinked files Only problem at the moment is that `git.TreeEntry` does not store the full path of the entry, only the actual filename without directory. --- modules/git/tree_entry.go | 10 ++++++++++ templates/repo/view_list.tmpl | 14 ++++++++------ 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/modules/git/tree_entry.go b/modules/git/tree_entry.go index 95131214872e6..da80c8acd1bf4 100644 --- a/modules/git/tree_entry.go +++ b/modules/git/tree_entry.go @@ -101,6 +101,16 @@ func (te *TreeEntry) FollowLinks() (*TreeEntry, error) { return entry, nil } +// TryFollowingLinks attempts to follow the symlinks of this entry to the origin +// If that fails, it defaults to the entry itself +func (te *TreeEntry) TryFollowingLinks() (*TreeEntry) { + newEntry, err := te.FollowLinks() + if err != nil { + return te + } + return newEntry +} + // returns the Tree pointed to by this TreeEntry, or nil if this is not a tree func (te *TreeEntry) Tree() *Tree { t, err := te.ptree.repo.getTree(te.ID) diff --git a/templates/repo/view_list.tmpl b/templates/repo/view_list.tmpl index 504032aa78032..3737e3238bca1 100644 --- a/templates/repo/view_list.tmpl +++ b/templates/repo/view_list.tmpl @@ -8,14 +8,14 @@ {{if .LatestCommitUser}} {{ctx.AvatarUtils.Avatar .LatestCommitUser 24 "gt-mr-2"}} {{if .LatestCommitUser.FullName}} - {{.LatestCommitUser.FullName}} + {{.LatestCommitUser.FullName}} {{else}} - {{if .LatestCommit.Author}}{{.LatestCommit.Author.Name}}{{else}}{{.LatestCommitUser.Name}}{{end}} + {{if .LatestCommit.Author}}{{.LatestCommit.Author.Name}}{{else}}{{.LatestCommitUser.Name}}{{end}} {{end}} {{else}} {{if .LatestCommit.Author}} {{ctx.AvatarUtils.AvatarByEmail .LatestCommit.Author.Email .LatestCommit.Author.Name 24 "gt-mr-2"}} - {{.LatestCommit.Author.Name}} + {{.LatestCommit.Author.Name}} {{end}} {{end}} @@ -26,7 +26,7 @@ {{template "repo/commit_statuses" dict "Status" .LatestCommitStatus "Statuses" .LatestCommitStatuses}} {{$commitLink:= printf "%s/commit/%s" .RepoLink (PathEscape .LatestCommit.ID.String)}} - {{RenderCommitMessageLinkSubject $.Context .LatestCommit.Message $commitLink ($.Repository.ComposeMetas ctx)}} + {{RenderCommitMessageLinkSubject $.Context .LatestCommit.Message $commitLink ($.Repository.ComposeMetas ctx)}} {{if IsMultilineCommitMessage .LatestCommit.Message}} 
{{RenderCommitBody $.Context .LatestCommit.Message ($.Repository.ComposeMetas ctx)}}
@@ -62,7 +62,7 @@ {{if $entry.IsDir}} {{$subJumpablePathName := $entry.GetSubJumpablePathName}} {{svg "octicon-file-directory-fill"}} - + {{$subJumpablePathFields := StringUtils.Split $subJumpablePathName "/"}} {{$subJumpablePathFieldLast := (Eval (len $subJumpablePathFields) "-" 1)}} {{if eq $subJumpablePathFieldLast 0}} @@ -74,7 +74,9 @@ {{else}} {{svg (printf "octicon-%s" (EntryIcon $entry))}} - {{$entry.Name}} + {{$symLinkEntry := $entry}} + {{if $entry.IsLink}}{{$symLinkEntry = $entry.TryFollowingLinks}}{{end}} + {{$entry.Name}} {{end}} {{end}} From 429fac5a500a6c011d94ca592ae3d54931f3d827 Mon Sep 17 00:00:00 2001 From: delvh Date: Wed, 17 Jan 2024 20:38:50 +0100 Subject: [PATCH 2/5] Finish redirection mechanism --- modules/git/tree_blob_nogogit.go | 33 ++++++++++++++++--------------- modules/git/tree_entry.go | 2 +- modules/git/tree_entry_nogogit.go | 9 ++++++--- templates/repo/view_list.tmpl | 2 +- 4 files changed, 25 insertions(+), 21 deletions(-) diff --git a/modules/git/tree_blob_nogogit.go b/modules/git/tree_blob_nogogit.go index 92d3d107a7425..cd5076babfb34 100644 --- a/modules/git/tree_blob_nogogit.go +++ b/modules/git/tree_blob_nogogit.go @@ -26,23 +26,24 @@ func (t *Tree) GetTreeEntryByPath(relpath string) (*TreeEntry, error) { relpath = path.Clean(relpath) parts := strings.Split(relpath, "/") var err error + tree := t - for i, name := range parts { - if i == len(parts)-1 { - entries, err := tree.ListEntries() - if err != nil { - return nil, err - } - for _, v := range entries { - if v.Name() == name { - return v, nil - } - } - } else { - tree, err = tree.SubTree(name) - if err != nil { - return nil, err - } + for _, name := range parts[:len(parts)-1] { + tree, err = tree.SubTree(name) + if err != nil { + return nil, err + } + } + + name := parts[len(parts)-1] + entries, err := tree.ListEntries() + if err != nil { + return nil, err + } + for _, v := range entries { + if v.Name() == name { + v.fullName = relpath + return v, nil } } return nil, ErrNotExist{"", relpath} diff --git a/modules/git/tree_entry.go b/modules/git/tree_entry.go index da80c8acd1bf4..00f3cd80466b5 100644 --- a/modules/git/tree_entry.go +++ b/modules/git/tree_entry.go @@ -103,7 +103,7 @@ func (te *TreeEntry) FollowLinks() (*TreeEntry, error) { // TryFollowingLinks attempts to follow the symlinks of this entry to the origin // If that fails, it defaults to the entry itself -func (te *TreeEntry) TryFollowingLinks() (*TreeEntry) { +func (te *TreeEntry) TryFollowingLinks() *TreeEntry { newEntry, err := te.FollowLinks() if err != nil { return te diff --git a/modules/git/tree_entry_nogogit.go b/modules/git/tree_entry_nogogit.go index 89244e27ee80e..c79fd16f1f53d 100644 --- a/modules/git/tree_entry_nogogit.go +++ b/modules/git/tree_entry_nogogit.go @@ -23,9 +23,6 @@ type TreeEntry struct { // Name returns the name of the entry func (te *TreeEntry) Name() string { - if te.fullName != "" { - return te.fullName - } return te.name } @@ -58,6 +55,12 @@ func (te *TreeEntry) Size() int64 { te.sized = true return te.size } +func (te *TreeEntry) FullPath() string { + if te.fullName == "" { + return te.Name() + } + return te.fullName +} // IsSubModule if the entry is a sub module func (te *TreeEntry) IsSubModule() bool { diff --git a/templates/repo/view_list.tmpl b/templates/repo/view_list.tmpl index 3737e3238bca1..dd67c51ce5c65 100644 --- a/templates/repo/view_list.tmpl +++ b/templates/repo/view_list.tmpl @@ -76,7 +76,7 @@ {{svg (printf "octicon-%s" (EntryIcon $entry))}} {{$symLinkEntry := $entry}} {{if $entry.IsLink}}{{$symLinkEntry = $entry.TryFollowingLinks}}{{end}} - {{$entry.Name}} + {{$entry.Name}} {{end}} {{end}} From 3dbee7953361e7a682fed7cd17665ff7a8855127 Mon Sep 17 00:00:00 2001 From: delvh Date: Wed, 17 Jan 2024 20:58:20 +0100 Subject: [PATCH 3/5] Also add the new method in the `gogit` version --- modules/git/tree_entry_gogit.go | 4 ++++ modules/git/tree_entry_nogogit.go | 1 + 2 files changed, 5 insertions(+) diff --git a/modules/git/tree_entry_gogit.go b/modules/git/tree_entry_gogit.go index eb9b012681474..a74ebb38bd35b 100644 --- a/modules/git/tree_entry_gogit.go +++ b/modules/git/tree_entry_gogit.go @@ -55,6 +55,10 @@ func (te *TreeEntry) Size() int64 { return te.size } +func (te *TreeEntry) FullPath() bool { + return te.Name() +} + // IsSubModule if the entry is a sub module func (te *TreeEntry) IsSubModule() bool { return te.gogitTreeEntry.Mode == filemode.Submodule diff --git a/modules/git/tree_entry_nogogit.go b/modules/git/tree_entry_nogogit.go index c79fd16f1f53d..42c9b74550a82 100644 --- a/modules/git/tree_entry_nogogit.go +++ b/modules/git/tree_entry_nogogit.go @@ -55,6 +55,7 @@ func (te *TreeEntry) Size() int64 { te.sized = true return te.size } + func (te *TreeEntry) FullPath() string { if te.fullName == "" { return te.Name() From 4db49fe5d846f45443c9ac0b5301cf3d58758926 Mon Sep 17 00:00:00 2001 From: delvh Date: Wed, 17 Jan 2024 23:28:59 +0100 Subject: [PATCH 4/5] Thanks type system for catching this copy paste error --- modules/git/tree_entry_gogit.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/git/tree_entry_gogit.go b/modules/git/tree_entry_gogit.go index a74ebb38bd35b..02b68a3713368 100644 --- a/modules/git/tree_entry_gogit.go +++ b/modules/git/tree_entry_gogit.go @@ -55,7 +55,7 @@ func (te *TreeEntry) Size() int64 { return te.size } -func (te *TreeEntry) FullPath() bool { +func (te *TreeEntry) FullPath() string { return te.Name() } From 32072232e6cf646b2f9e4b42aeb50ae41d8fbdd0 Mon Sep 17 00:00:00 2001 From: delvh Date: Fri, 19 Jan 2024 13:30:08 +0100 Subject: [PATCH 5/5] Add test --- modules/git/tree_entry_common_test.go | 106 ++++++++++++++++++++++++++ modules/git/tree_entry_test.go | 47 ------------ 2 files changed, 106 insertions(+), 47 deletions(-) create mode 100644 modules/git/tree_entry_common_test.go diff --git a/modules/git/tree_entry_common_test.go b/modules/git/tree_entry_common_test.go new file mode 100644 index 0000000000000..abafed0233857 --- /dev/null +++ b/modules/git/tree_entry_common_test.go @@ -0,0 +1,106 @@ +// Copyright 2024 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package git + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestFollowLink(t *testing.T) { + r, err := openRepositoryWithDefaultContext("tests/repos/repo1_bare") + assert.NoError(t, err) + defer r.Close() + + commit, err := r.GetCommit("37991dec2c8e592043f47155ce4808d4580f9123") + assert.NoError(t, err) + + // get the symlink + lnk, err := commit.Tree.GetTreeEntryByPath("foo/bar/link_to_hello") + assert.NoError(t, err) + assert.True(t, lnk.IsLink()) + + // should be able to dereference to target + target, err := lnk.FollowLink() + assert.NoError(t, err) + assert.Equal(t, "hello", target.Name()) + assert.Equal(t, "foo/nar/hello", target.FullPath()) + assert.False(t, target.IsLink()) + assert.Equal(t, "b14df6442ea5a1b382985a6549b85d435376c351", target.ID.String()) + + // should error when called on normal file + target, err = commit.Tree.GetTreeEntryByPath("file1.txt") + assert.NoError(t, err) + _, err = target.FollowLink() + assert.ErrorAs(t, err, ErrBadLink{}) + + // should error for broken links + target, err = commit.Tree.GetTreeEntryByPath("foo/broken_link") + assert.NoError(t, err) + assert.True(t, target.IsLink()) + _, err = target.FollowLink() + assert.ErrorAs(t, err, ErrBadLink{}) + + // should error for external links + target, err = commit.Tree.GetTreeEntryByPath("foo/outside_repo") + assert.NoError(t, err) + assert.True(t, target.IsLink()) + _, err = target.FollowLink() + assert.ErrorAs(t, err, ErrBadLink{}) + + // testing fix for short link bug + target, err = commit.Tree.GetTreeEntryByPath("foo/link_short") + assert.NoError(t, err) + _, err = target.FollowLink() + assert.ErrorAs(t, err, ErrBadLink{}) +} + +func TestTryFollowingLinks(t *testing.T) { + r, err := openRepositoryWithDefaultContext("tests/repos/repo1_bare") + assert.NoError(t, err) + defer r.Close() + + commit, err := r.GetCommit("37991dec2c8e592043f47155ce4808d4580f9123") + assert.NoError(t, err) + + // get the symlink + list, err := commit.Tree.GetTreeEntryByPath("foo/bar/link_to_hello") + assert.NoError(t, err) + assert.True(t, list.IsLink()) + + // should be able to dereference to target + target := list.TryFollowingLinks() + assert.NotEqual(t, target, list) + assert.Equal(t, "hello", target.Name()) + assert.Equal(t, "foo/nar/hello", target.FullPath()) + assert.False(t, target.IsLink()) + assert.Equal(t, "b14df6442ea5a1b382985a6549b85d435376c351", target.ID.String()) + + // should default to original when called on normal file + link, err := commit.Tree.GetTreeEntryByPath("file1.txt") + assert.NoError(t, err) + target = link.TryFollowingLinks() + assert.Same(t, link, target) + + // should default to original for broken links + link, err = commit.Tree.GetTreeEntryByPath("foo/broken_link") + assert.NoError(t, err) + assert.True(t, link.IsLink()) + target = link.TryFollowingLinks() + assert.Same(t, link, target) + + // should default to original for external links + link, err = commit.Tree.GetTreeEntryByPath("foo/outside_repo") + assert.NoError(t, err) + assert.True(t, link.IsLink()) + target = link.TryFollowingLinks() + assert.Same(t, link, target) + + // testing fix for short link bug + link, err = commit.Tree.GetTreeEntryByPath("foo/link_short") + assert.NoError(t, err) + target = link.TryFollowingLinks() + assert.Same(t, link, target) +} diff --git a/modules/git/tree_entry_test.go b/modules/git/tree_entry_test.go index 30eee13669e43..9ca82675e0797 100644 --- a/modules/git/tree_entry_test.go +++ b/modules/git/tree_entry_test.go @@ -53,50 +53,3 @@ func TestEntriesCustomSort(t *testing.T) { assert.Equal(t, "bcd", entries[6].Name()) assert.Equal(t, "abc", entries[7].Name()) } - -func TestFollowLink(t *testing.T) { - r, err := openRepositoryWithDefaultContext("tests/repos/repo1_bare") - assert.NoError(t, err) - defer r.Close() - - commit, err := r.GetCommit("37991dec2c8e592043f47155ce4808d4580f9123") - assert.NoError(t, err) - - // get the symlink - lnk, err := commit.Tree.GetTreeEntryByPath("foo/bar/link_to_hello") - assert.NoError(t, err) - assert.True(t, lnk.IsLink()) - - // should be able to dereference to target - target, err := lnk.FollowLink() - assert.NoError(t, err) - assert.Equal(t, "hello", target.Name()) - assert.False(t, target.IsLink()) - assert.Equal(t, "b14df6442ea5a1b382985a6549b85d435376c351", target.ID.String()) - - // should error when called on normal file - target, err = commit.Tree.GetTreeEntryByPath("file1.txt") - assert.NoError(t, err) - _, err = target.FollowLink() - assert.EqualError(t, err, "file1.txt: not a symlink") - - // should error for broken links - target, err = commit.Tree.GetTreeEntryByPath("foo/broken_link") - assert.NoError(t, err) - assert.True(t, target.IsLink()) - _, err = target.FollowLink() - assert.EqualError(t, err, "broken_link: broken link") - - // should error for external links - target, err = commit.Tree.GetTreeEntryByPath("foo/outside_repo") - assert.NoError(t, err) - assert.True(t, target.IsLink()) - _, err = target.FollowLink() - assert.EqualError(t, err, "outside_repo: points outside of repo") - - // testing fix for short link bug - target, err = commit.Tree.GetTreeEntryByPath("foo/link_short") - assert.NoError(t, err) - _, err = target.FollowLink() - assert.EqualError(t, err, "link_short: broken link") -}