Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't create duplicated functions for code repositories and wiki repositories #33924

Merged
merged 5 commits into from
Mar 19, 2025
Merged
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 16 additions & 4 deletions models/repo/repo.go
Original file line number Diff line number Diff line change
@@ -215,12 +215,24 @@ func init() {
db.RegisterModel(new(Repository))
}

func (repo *Repository) GetName() string {
return repo.Name
func RelativePath(ownerName, repoName string) string {
return strings.ToLower(ownerName) + "/" + strings.ToLower(repoName) + ".git"
}

func (repo *Repository) GetOwnerName() string {
return repo.OwnerName
// RelativePath should be an unix style path like username/reponame.git
func (repo *Repository) RelativePath() string {
return RelativePath(repo.OwnerName, repo.Name)
}

type StorageRepo string
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

RepoPath?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

may RepoPath can cause confusion with repo url path? maybe StoragePath?

Copy link
Contributor

@wxiaoguang wxiaoguang Mar 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is already a complex Storage system in Gitea.

TBH any new variable named "StorageXxx" doesn't look good to me (but I won't say block)

Copy link
Member

@delvh delvh Mar 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

may RepoPath can cause confusion with repo url path?

No, I would say it does not:
After all, we are on the models layer here.
As such, it is far more likely that we are talking about how the repo is stored compared to how it is accessed.
If it had something to do with URLs, we would need the URL somewhere in the name, since that stuff is normally handled by routers and services.

Copy link
Member Author

@lunny lunny Mar 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It maybe refactored to add more interface functions rather than repo path, so I don't think it's a good name as RepoPath. And this is an implementation of gitrepo.Repository.


// RelativePath should be an unix style path like username/reponame.git
func (sr StorageRepo) RelativePath() string {
return string(sr)
}

func (repo *Repository) WikiStorageRepo() StorageRepo {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ToWikiPath()?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a storage repository interface, not only path.

return StorageRepo(strings.ToLower(repo.OwnerName) + "/" + strings.ToLower(repo.Name) + ".wiki.git")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could check here that we don't already have a Wiki repo:
It could be possible in the future that someone calls this method unaware that it's already a wiki repo, and receives a double-encoded wiki repo (.wiki.git.wiki.git).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I want to add a comment here: currently, Repository refers to the entire concept of a repository. I don’t believe it can be interpreted to mean a wiki repository specifically.

}

// SanitizedOriginalURL returns a sanitized OriginalURL
12 changes: 0 additions & 12 deletions modules/gitrepo/branch.go
Original file line number Diff line number Diff line change
@@ -44,24 +44,12 @@ func GetDefaultBranch(ctx context.Context, repo Repository) (string, error) {
return git.GetDefaultBranch(ctx, repoPath(repo))
}

func GetWikiDefaultBranch(ctx context.Context, repo Repository) (string, error) {
return git.GetDefaultBranch(ctx, wikiPath(repo))
}

// IsReferenceExist returns true if given reference exists in the repository.
func IsReferenceExist(ctx context.Context, repo Repository, name string) bool {
return git.IsReferenceExist(ctx, repoPath(repo), name)
}

func IsWikiReferenceExist(ctx context.Context, repo Repository, name string) bool {
return git.IsReferenceExist(ctx, wikiPath(repo), name)
}

// IsBranchExist returns true if given branch exists in the repository.
func IsBranchExist(ctx context.Context, repo Repository, name string) bool {
return IsReferenceExist(ctx, repo, git.BranchPrefix+name)
}

func IsWikiBranchExist(ctx context.Context, repo Repository, name string) bool {
return IsWikiReferenceExist(ctx, repo, git.BranchPrefix+name)
}
26 changes: 7 additions & 19 deletions modules/gitrepo/gitrepo.go
Original file line number Diff line number Diff line change
@@ -8,40 +8,29 @@ import (
"fmt"
"io"
"path/filepath"
"strings"

"code.gitea.io/gitea/modules/git"
"code.gitea.io/gitea/modules/reqctx"
"code.gitea.io/gitea/modules/setting"
"code.gitea.io/gitea/modules/util"
)

// Repository represents a git repository which stored in a disk
type Repository interface {
GetName() string
GetOwnerName() string
}

func absPath(owner, name string) string {
return filepath.Join(setting.RepoRootPath, strings.ToLower(owner), strings.ToLower(name)+".git")
RelativePath() string // We don't assume how the directory structure of the repository is, so we only need the relative path
}

// RelativePath should be an unix style path like username/reponame.git
// This method should change it according to the current OS.
func repoPath(repo Repository) string {
return absPath(repo.GetOwnerName(), repo.GetName())
}

func wikiPath(repo Repository) string {
return filepath.Join(setting.RepoRootPath, strings.ToLower(repo.GetOwnerName()), strings.ToLower(repo.GetName())+".wiki.git")
return filepath.Join(setting.RepoRootPath, filepath.FromSlash(repo.RelativePath()))
}

// OpenRepository opens the repository at the given relative path with the provided context.
func OpenRepository(ctx context.Context, repo Repository) (*git.Repository, error) {
return git.OpenRepository(ctx, repoPath(repo))
}

func OpenWikiRepository(ctx context.Context, repo Repository) (*git.Repository, error) {
return git.OpenRepository(ctx, wikiPath(repo))
}

// contextKey is a value for use with context.WithValue.
type contextKey struct {
repoPath string
@@ -86,9 +75,8 @@ func DeleteRepository(ctx context.Context, repo Repository) error {
}

// RenameRepository renames a repository's name on disk
func RenameRepository(ctx context.Context, repo Repository, newName string) error {
newRepoPath := absPath(repo.GetOwnerName(), newName)
if err := util.Rename(repoPath(repo), newRepoPath); err != nil {
func RenameRepository(ctx context.Context, repo, newRepo Repository) error {
if err := util.Rename(repoPath(repo), repoPath(newRepo)); err != nil {
return fmt.Errorf("rename repository directory: %w", err)
}
return nil
18 changes: 4 additions & 14 deletions modules/gitrepo/hooks.go
Original file line number Diff line number Diff line change
@@ -106,16 +106,11 @@ done
return hookNames, hookTpls, giteaHookTpls
}

// CreateDelegateHooksForRepo creates all the hooks scripts for the repo
func CreateDelegateHooksForRepo(_ context.Context, repo Repository) (err error) {
// CreateDelegateHooks creates all the hooks scripts for the repo
func CreateDelegateHooks(_ context.Context, repo Repository) (err error) {
return createDelegateHooks(filepath.Join(repoPath(repo), "hooks"))
}

// CreateDelegateHooksForWiki creates all the hooks scripts for the wiki repo
func CreateDelegateHooksForWiki(_ context.Context, repo Repository) (err error) {
return createDelegateHooks(filepath.Join(wikiPath(repo), "hooks"))
}

func createDelegateHooks(hookDir string) (err error) {
hookNames, hookTpls, giteaHookTpls := getHookTemplates()

@@ -178,16 +173,11 @@ func ensureExecutable(filename string) error {
return os.Chmod(filename, mode)
}

// CheckDelegateHooksForRepo checks the hooks scripts for the repo
func CheckDelegateHooksForRepo(_ context.Context, repo Repository) ([]string, error) {
// CheckDelegateHooks checks the hooks scripts for the repo
func CheckDelegateHooks(_ context.Context, repo Repository) ([]string, error) {
return checkDelegateHooks(filepath.Join(repoPath(repo), "hooks"))
}

// CheckDelegateHooksForWiki checks the hooks scripts for the repo
func CheckDelegateHooksForWiki(_ context.Context, repo Repository) ([]string, error) {
return checkDelegateHooks(filepath.Join(wikiPath(repo), "hooks"))
}

func checkDelegateHooks(hookDir string) ([]string, error) {
hookNames, hookTpls, giteaHookTpls := getHookTemplates()

2 changes: 1 addition & 1 deletion modules/repository/init.go
Original file line number Diff line number Diff line change
@@ -138,7 +138,7 @@ func CheckInitRepository(ctx context.Context, repo *repo_model.Repository) (err
// Init git bare new repository.
if err = git.InitRepository(ctx, repo.RepoPath(), true, repo.ObjectFormatName); err != nil {
return fmt.Errorf("git.InitRepository: %w", err)
} else if err = gitrepo.CreateDelegateHooksForRepo(ctx, repo); err != nil {
} else if err = gitrepo.CreateDelegateHooks(ctx, repo); err != nil {
return fmt.Errorf("createDelegateHooks: %w", err)
}
return nil
2 changes: 1 addition & 1 deletion routers/api/v1/repo/wiki.go
Original file line number Diff line number Diff line change
@@ -476,7 +476,7 @@ func findEntryForFile(commit *git.Commit, target string) (*git.TreeEntry, error)
// findWikiRepoCommit opens the wiki repo and returns the latest commit, writing to context on error.
// The caller is responsible for closing the returned repo again
func findWikiRepoCommit(ctx *context.APIContext) (*git.Repository, *git.Commit) {
wikiRepo, err := gitrepo.OpenWikiRepository(ctx, ctx.Repo.Repository)
wikiRepo, err := gitrepo.OpenRepository(ctx, ctx.Repo.Repository.WikiStorageRepo())
if err != nil {
if git.IsErrNotExist(err) || err.Error() == "no such file or directory" {
ctx.APIErrorNotFound(err)
4 changes: 2 additions & 2 deletions routers/web/repo/commit.go
Original file line number Diff line number Diff line change
@@ -284,7 +284,7 @@ func Diff(ctx *context.Context) {
)

if ctx.Data["PageIsWiki"] != nil {
gitRepo, err = gitrepo.OpenWikiRepository(ctx, ctx.Repo.Repository)
gitRepo, err = gitrepo.OpenRepository(ctx, ctx.Repo.Repository.WikiStorageRepo())
if err != nil {
ctx.ServerError("Repo.GitRepo.GetCommit", err)
return
@@ -417,7 +417,7 @@ func Diff(ctx *context.Context) {
func RawDiff(ctx *context.Context) {
var gitRepo *git.Repository
if ctx.Data["PageIsWiki"] != nil {
wikiRepo, err := gitrepo.OpenWikiRepository(ctx, ctx.Repo.Repository)
wikiRepo, err := gitrepo.OpenRepository(ctx, ctx.Repo.Repository.WikiStorageRepo())
if err != nil {
ctx.ServerError("OpenRepository", err)
return
2 changes: 1 addition & 1 deletion routers/web/repo/compare.go
Original file line number Diff line number Diff line change
@@ -885,7 +885,7 @@ func ExcerptBlob(ctx *context.Context) {
gitRepo := ctx.Repo.GitRepo
if ctx.Data["PageIsWiki"] == true {
var err error
gitRepo, err = gitrepo.OpenWikiRepository(ctx, ctx.Repo.Repository)
gitRepo, err = gitrepo.OpenRepository(ctx, ctx.Repo.Repository.WikiStorageRepo())
if err != nil {
ctx.ServerError("OpenRepository", err)
return
4 changes: 2 additions & 2 deletions routers/web/repo/wiki.go
Original file line number Diff line number Diff line change
@@ -96,7 +96,7 @@ func findEntryForFile(commit *git.Commit, target string) (*git.TreeEntry, error)
}

func findWikiRepoCommit(ctx *context.Context) (*git.Repository, *git.Commit, error) {
wikiGitRepo, errGitRepo := gitrepo.OpenWikiRepository(ctx, ctx.Repo.Repository)
wikiGitRepo, errGitRepo := gitrepo.OpenRepository(ctx, ctx.Repo.Repository.WikiStorageRepo())
if errGitRepo != nil {
ctx.ServerError("OpenRepository", errGitRepo)
return nil, nil, errGitRepo
@@ -105,7 +105,7 @@ func findWikiRepoCommit(ctx *context.Context) (*git.Repository, *git.Commit, err
commit, errCommit := wikiGitRepo.GetBranchCommit(ctx.Repo.Repository.DefaultWikiBranch)
if git.IsErrNotExist(errCommit) {
// if the default branch recorded in database is out of sync, then re-sync it
gitRepoDefaultBranch, errBranch := gitrepo.GetWikiDefaultBranch(ctx, ctx.Repo.Repository)
gitRepoDefaultBranch, errBranch := gitrepo.GetDefaultBranch(ctx, ctx.Repo.Repository.WikiStorageRepo())
if errBranch != nil {
return wikiGitRepo, nil, errBranch
}
2 changes: 1 addition & 1 deletion routers/web/repo/wiki_test.go
Original file line number Diff line number Diff line change
@@ -29,7 +29,7 @@ const (
)

func wikiEntry(t *testing.T, repo *repo_model.Repository, wikiName wiki_service.WebPath) *git.TreeEntry {
wikiRepo, err := gitrepo.OpenWikiRepository(git.DefaultContext, repo)
wikiRepo, err := gitrepo.OpenRepository(git.DefaultContext, repo.WikiStorageRepo())
assert.NoError(t, err)
defer wikiRepo.Close()
commit, err := wikiRepo.GetBranchCommit("master")
2 changes: 1 addition & 1 deletion services/asymkey/sign.go
Original file line number Diff line number Diff line change
@@ -204,7 +204,7 @@ Loop:
return false, "", nil, &ErrWontSign{twofa}
}
case parentSigned:
gitRepo, err := gitrepo.OpenWikiRepository(ctx, repo)
gitRepo, err := gitrepo.OpenRepository(ctx, repo.WikiStorageRepo())
if err != nil {
return false, "", nil, err
}
4 changes: 2 additions & 2 deletions services/doctor/misc.go
Original file line number Diff line number Diff line change
@@ -49,14 +49,14 @@ func checkScriptType(ctx context.Context, logger log.Logger, autofix bool) error

func checkHooks(ctx context.Context, logger log.Logger, autofix bool) error {
if err := iterateRepositories(ctx, func(repo *repo_model.Repository) error {
results, err := gitrepo.CheckDelegateHooksForRepo(ctx, repo)
results, err := gitrepo.CheckDelegateHooks(ctx, repo)
if err != nil {
logger.Critical("Unable to check delegate hooks for repo %-v. ERROR: %v", repo, err)
return fmt.Errorf("Unable to check delegate hooks for repo %-v. ERROR: %w", repo, err)
}
if len(results) > 0 && autofix {
logger.Warn("Regenerated hooks for %s", repo.FullName())
if err := gitrepo.CreateDelegateHooksForRepo(ctx, repo); err != nil {
if err := gitrepo.CreateDelegateHooks(ctx, repo); err != nil {
logger.Critical("Unable to recreate delegate hooks for %-v. ERROR: %v", repo, err)
return fmt.Errorf("Unable to recreate delegate hooks for %-v. ERROR: %w", repo, err)
}
2 changes: 1 addition & 1 deletion services/mirror/mirror_push.go
Original file line number Diff line number Diff line change
@@ -143,7 +143,7 @@ func runPushSync(ctx context.Context, m *repo_model.PushMirror) error {

var gitRepo *git.Repository
if isWiki {
gitRepo, err = gitrepo.OpenWikiRepository(ctx, repo)
gitRepo, err = gitrepo.OpenRepository(ctx, repo.WikiStorageRepo())
} else {
gitRepo, err = gitrepo.OpenRepository(ctx, repo)
}
2 changes: 1 addition & 1 deletion services/repository/adopt.go
Original file line number Diff line number Diff line change
@@ -115,7 +115,7 @@ func adoptRepository(ctx context.Context, repo *repo_model.Repository, defaultBr
return fmt.Errorf("adoptRepository: path does not already exist: %s", repo.FullName())
}

if err := gitrepo.CreateDelegateHooksForRepo(ctx, repo); err != nil {
if err := gitrepo.CreateDelegateHooks(ctx, repo); err != nil {
return fmt.Errorf("createDelegateHooks: %w", err)
}

2 changes: 1 addition & 1 deletion services/repository/fork.go
Original file line number Diff line number Diff line change
@@ -170,7 +170,7 @@ func ForkRepository(ctx context.Context, doer, owner *user_model.User, opts Fork
return fmt.Errorf("git update-server-info: %w", err)
}

if err = gitrepo.CreateDelegateHooksForRepo(ctx, repo); err != nil {
if err = gitrepo.CreateDelegateHooks(ctx, repo); err != nil {
return fmt.Errorf("createDelegateHooks: %w", err)
}

4 changes: 2 additions & 2 deletions services/repository/hooks.go
Original file line number Diff line number Diff line change
@@ -31,11 +31,11 @@ func SyncRepositoryHooks(ctx context.Context) error {
default:
}

if err := gitrepo.CreateDelegateHooksForRepo(ctx, repo); err != nil {
if err := gitrepo.CreateDelegateHooks(ctx, repo); err != nil {
return fmt.Errorf("SyncRepositoryHook: %w", err)
}
if repo.HasWiki() {
if err := gitrepo.CreateDelegateHooksForWiki(ctx, repo); err != nil {
if err := gitrepo.CreateDelegateHooks(ctx, repo.WikiStorageRepo()); err != nil {
return fmt.Errorf("SyncRepositoryHook: %w", err)
}
}
4 changes: 2 additions & 2 deletions services/repository/migrate.go
Original file line number Diff line number Diff line change
@@ -265,11 +265,11 @@ func cleanUpMigrateGitConfig(ctx context.Context, repoPath string) error {

// CleanUpMigrateInfo finishes migrating repository and/or wiki with things that don't need to be done for mirrors.
func CleanUpMigrateInfo(ctx context.Context, repo *repo_model.Repository) (*repo_model.Repository, error) {
if err := gitrepo.CreateDelegateHooksForRepo(ctx, repo); err != nil {
if err := gitrepo.CreateDelegateHooks(ctx, repo); err != nil {
return repo, fmt.Errorf("createDelegateHooks: %w", err)
}
if repo.HasWiki() {
if err := gitrepo.CreateDelegateHooksForWiki(ctx, repo); err != nil {
if err := gitrepo.CreateDelegateHooks(ctx, repo.WikiStorageRepo()); err != nil {
return repo, fmt.Errorf("createDelegateHooks.(wiki): %w", err)
}
}
5 changes: 3 additions & 2 deletions services/repository/transfer.go
Original file line number Diff line number Diff line change
@@ -331,12 +331,13 @@ func changeRepositoryName(ctx context.Context, repo *repo_model.Repository, newR
return fmt.Errorf("IsRepositoryExist: %w", err)
} else if has {
return repo_model.ErrRepoAlreadyExist{
Uname: repo.Owner.Name,
Uname: repo.OwnerName,
Name: newRepoName,
}
}

if err = gitrepo.RenameRepository(ctx, repo, newRepoName); err != nil {
if err = gitrepo.RenameRepository(ctx, repo,
repo_model.StorageRepo(repo_model.RelativePath(repo.OwnerName, newRepoName))); err != nil {
return fmt.Errorf("rename repository directory: %w", err)
}

8 changes: 4 additions & 4 deletions services/wiki/wiki.go
Original file line number Diff line number Diff line change
@@ -41,7 +41,7 @@ func InitWiki(ctx context.Context, repo *repo_model.Repository) error {

if err := git.InitRepository(ctx, repo.WikiPath(), true, repo.ObjectFormatName); err != nil {
return fmt.Errorf("InitRepository: %w", err)
} else if err = gitrepo.CreateDelegateHooksForWiki(ctx, repo); err != nil {
} else if err = gitrepo.CreateDelegateHooks(ctx, repo.WikiStorageRepo()); err != nil {
return fmt.Errorf("createDelegateHooks: %w", err)
} else if _, _, err = git.NewCommand("symbolic-ref", "HEAD").AddDynamicArguments(git.BranchPrefix+repo.DefaultWikiBranch).RunStdString(ctx, &git.RunOpts{Dir: repo.WikiPath()}); err != nil {
return fmt.Errorf("unable to set default wiki branch to %q: %w", repo.DefaultWikiBranch, err)
@@ -100,7 +100,7 @@ func updateWikiPage(ctx context.Context, doer *user_model.User, repo *repo_model
return fmt.Errorf("InitWiki: %w", err)
}

hasDefaultBranch := gitrepo.IsWikiBranchExist(ctx, repo, repo.DefaultWikiBranch)
hasDefaultBranch := gitrepo.IsBranchExist(ctx, repo.WikiStorageRepo(), repo.DefaultWikiBranch)

basePath, err := repo_module.CreateTemporaryPath("update-wiki")
if err != nil {
@@ -381,15 +381,15 @@ func ChangeDefaultWikiBranch(ctx context.Context, repo *repo_model.Repository, n
return nil
}

oldDefBranch, err := gitrepo.GetWikiDefaultBranch(ctx, repo)
oldDefBranch, err := gitrepo.GetDefaultBranch(ctx, repo.WikiStorageRepo())
if err != nil {
return fmt.Errorf("unable to get default branch: %w", err)
}
if oldDefBranch == newBranch {
return nil
}

gitRepo, err := gitrepo.OpenWikiRepository(ctx, repo)
gitRepo, err := gitrepo.OpenRepository(ctx, repo.WikiStorageRepo())
if errors.Is(err, util.ErrNotExist) {
return nil // no git repo on storage, no need to do anything else
} else if err != nil {
8 changes: 4 additions & 4 deletions services/wiki/wiki_test.go
Original file line number Diff line number Diff line change
@@ -166,7 +166,7 @@ func TestRepository_AddWikiPage(t *testing.T) {
webPath := UserTitleToWebPath("", userTitle)
assert.NoError(t, AddWikiPage(git.DefaultContext, doer, repo, webPath, wikiContent, commitMsg))
// Now need to show that the page has been added:
gitRepo, err := gitrepo.OpenWikiRepository(git.DefaultContext, repo)
gitRepo, err := gitrepo.OpenRepository(git.DefaultContext, repo.WikiStorageRepo())
require.NoError(t, err)

defer gitRepo.Close()
@@ -213,7 +213,7 @@ func TestRepository_EditWikiPage(t *testing.T) {
assert.NoError(t, EditWikiPage(git.DefaultContext, doer, repo, "Home", webPath, newWikiContent, commitMsg))

// Now need to show that the page has been added:
gitRepo, err := gitrepo.OpenWikiRepository(git.DefaultContext, repo)
gitRepo, err := gitrepo.OpenRepository(git.DefaultContext, repo.WikiStorageRepo())
assert.NoError(t, err)
masterTree, err := gitRepo.GetTree(repo.DefaultWikiBranch)
assert.NoError(t, err)
@@ -237,7 +237,7 @@ func TestRepository_DeleteWikiPage(t *testing.T) {
assert.NoError(t, DeleteWikiPage(git.DefaultContext, doer, repo, "Home"))

// Now need to show that the page has been added:
gitRepo, err := gitrepo.OpenWikiRepository(git.DefaultContext, repo)
gitRepo, err := gitrepo.OpenRepository(git.DefaultContext, repo.WikiStorageRepo())
require.NoError(t, err)

defer gitRepo.Close()
@@ -251,7 +251,7 @@ func TestRepository_DeleteWikiPage(t *testing.T) {
func TestPrepareWikiFileName(t *testing.T) {
unittest.PrepareTestEnv(t)
repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1})
gitRepo, err := gitrepo.OpenWikiRepository(git.DefaultContext, repo)
gitRepo, err := gitrepo.OpenRepository(git.DefaultContext, repo.WikiStorageRepo())
require.NoError(t, err)

defer gitRepo.Close()