From 7b0f3e647374c175c91ff9fa097da7873ecec98e Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sun, 27 Oct 2024 15:49:01 -0700 Subject: [PATCH] uniform all temporary directories --- build/generate-gitignores.go | 3 +- build/generate-licenses.go | 3 +- cmd/dump.go | 2 +- models/migrations/base/tests.go | 2 +- models/unittest/testdb.go | 4 +-- modules/git/blame.go | 3 +- modules/git/repo_index.go | 2 +- modules/markup/external/external.go | 2 +- modules/repository/temp.go | 36 +++++++++++-------- modules/setting/global.go | 7 ++++ modules/setting/ssh.go | 3 +- modules/storage/local.go | 2 +- modules/storage/local_test.go | 3 +- modules/util/filebuffer/file_backed_buffer.go | 4 ++- modules/util/legacy_test.go | 4 ++- routers/web/repo/githttp.go | 2 +- routers/web/repo/setting/lfs.go | 8 ++--- services/pull/patch.go | 2 +- services/pull/temp_repo.go | 9 ++--- services/repository/create.go | 8 +++-- services/repository/files/temp_repo.go | 13 ++++--- services/repository/generate.go | 3 +- services/repository/repository.go | 5 ++- services/wiki/wiki.go | 16 +++------ tests/integration/dump_restore_test.go | 2 +- 25 files changed, 82 insertions(+), 66 deletions(-) diff --git a/build/generate-gitignores.go b/build/generate-gitignores.go index 1e09c83a6a..164af8f71e 100644 --- a/build/generate-gitignores.go +++ b/build/generate-gitignores.go @@ -15,6 +15,7 @@ import ( "path/filepath" "strings" + "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/util" ) @@ -32,7 +33,7 @@ func main() { flag.StringVar(&githubApiToken, "token", "", "github api token") flag.Parse() - file, err := os.CreateTemp(os.TempDir(), prefix) + file, err := os.CreateTemp(setting.TempDir(), prefix) if err != nil { log.Fatalf("Failed to create temp file. %s", err) } diff --git a/build/generate-licenses.go b/build/generate-licenses.go index 66e1d37755..23236e32e1 100644 --- a/build/generate-licenses.go +++ b/build/generate-licenses.go @@ -22,6 +22,7 @@ import ( "code.gitea.io/gitea/build/license" "code.gitea.io/gitea/modules/json" + "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/util" ) @@ -39,7 +40,7 @@ func main() { flag.StringVar(&githubApiToken, "token", "", "github api token") flag.Parse() - file, err := os.CreateTemp(os.TempDir(), prefix) + file, err := os.CreateTemp(setting.TempDir(), prefix) if err != nil { log.Fatalf("Failed to create temp file. %s", err) } diff --git a/cmd/dump.go b/cmd/dump.go index ececc80f72..4acc0558d2 100644 --- a/cmd/dump.go +++ b/cmd/dump.go @@ -49,7 +49,7 @@ var CmdDump = &cli.Command{ &cli.StringFlag{ Name: "tempdir", Aliases: []string{"t"}, - Value: os.TempDir(), + Value: setting.TempDir(), Usage: "Temporary dir path", }, &cli.StringFlag{ diff --git a/models/migrations/base/tests.go b/models/migrations/base/tests.go index 85cafc1ab9..676504e530 100644 --- a/models/migrations/base/tests.go +++ b/models/migrations/base/tests.go @@ -141,7 +141,7 @@ func MainTest(m *testing.M) { setting.CustomConf = giteaConf } - tmpDataPath, err := os.MkdirTemp("", "data") + tmpDataPath, err := os.MkdirTemp(setting.TempDir(), "data") if err != nil { fmt.Printf("Unable to create temporary data path %v\n", err) os.Exit(1) diff --git a/models/unittest/testdb.go b/models/unittest/testdb.go index 53c9dbdd77..889a878d1a 100644 --- a/models/unittest/testdb.go +++ b/models/unittest/testdb.go @@ -124,12 +124,12 @@ func MainTest(m *testing.M, testOpts ...*TestOptions) { setting.SSH.Domain = "try.gitea.io" setting.Database.Type = "sqlite3" setting.Repository.DefaultBranch = "master" // many test code still assume that default branch is called "master" - repoRootPath, err := os.MkdirTemp(os.TempDir(), "repos") + repoRootPath, err := os.MkdirTemp(setting.TempDir(), "repos") if err != nil { fatalTestError("TempDir: %v\n", err) } setting.RepoRootPath = repoRootPath - appDataPath, err := os.MkdirTemp(os.TempDir(), "appdata") + appDataPath, err := os.MkdirTemp(setting.TempDir(), "appdata") if err != nil { fatalTestError("TempDir: %v\n", err) } diff --git a/modules/git/blame.go b/modules/git/blame.go index a9b2706f21..76c07c78f2 100644 --- a/modules/git/blame.go +++ b/modules/git/blame.go @@ -12,6 +12,7 @@ import ( "os" "code.gitea.io/gitea/modules/log" + "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/util" ) @@ -195,7 +196,7 @@ func tryCreateBlameIgnoreRevsFile(commit *Commit) *string { } defer r.Close() - f, err := os.CreateTemp("", "gitea_git-blame-ignore-revs") + f, err := os.CreateTemp(setting.TempDir(), "gitea_git-blame-ignore-revs") if err != nil { return nil } diff --git a/modules/git/repo_index.go b/modules/git/repo_index.go index 8390570098..0cff4f6db5 100644 --- a/modules/git/repo_index.go +++ b/modules/git/repo_index.go @@ -51,7 +51,7 @@ func (repo *Repository) readTreeToIndex(id ObjectID, indexFilename ...string) er // ReadTreeToTemporaryIndex reads a treeish to a temporary index file func (repo *Repository) ReadTreeToTemporaryIndex(treeish string) (filename, tmpDir string, cancel context.CancelFunc, err error) { - tmpDir, err = os.MkdirTemp("", "index") + tmpDir, err = os.MkdirTemp(os.TempDir(), "index") if err != nil { return filename, tmpDir, cancel, err } diff --git a/modules/markup/external/external.go b/modules/markup/external/external.go index 122517ed11..2e44bf3439 100644 --- a/modules/markup/external/external.go +++ b/modules/markup/external/external.go @@ -89,7 +89,7 @@ func (p *Renderer) Render(ctx *markup.RenderContext, input io.Reader, output io. if p.IsInputFile { // write to temp file - f, err := os.CreateTemp("", "gitea_input") + f, err := os.CreateTemp(setting.TempDir(), "gitea_input") if err != nil { return fmt.Errorf("%s create temp file when rendering %s failed: %w", p.Name(), p.Command, err) } diff --git a/modules/repository/temp.go b/modules/repository/temp.go index 04faa9db3d..9398b4c0cc 100644 --- a/modules/repository/temp.go +++ b/modules/repository/temp.go @@ -6,7 +6,6 @@ package repository import ( "fmt" "os" - "path" "path/filepath" "code.gitea.io/gitea/modules/log" @@ -14,30 +13,37 @@ import ( "code.gitea.io/gitea/modules/util" ) -// LocalCopyPath returns the local repository temporary copy path. -func LocalCopyPath() string { - if filepath.IsAbs(setting.Repository.Local.LocalCopyPath) { - return setting.Repository.Local.LocalCopyPath +// localCopyPath returns the local repository temporary copy path. +func localCopyPath() string { + return filepath.Join(setting.TempDir(), "local-repo") +} + +func CleanUpTemporaryPaths() { + if err := util.RemoveAll(localCopyPath()); err != nil { + log.Error("Unable to remove local repository temporary copy path: %s (%v)", localCopyPath(), err) } - return path.Join(setting.AppDataPath, setting.Repository.Local.LocalCopyPath) } // CreateTemporaryPath creates a temporary path -func CreateTemporaryPath(prefix string) (string, error) { - if err := os.MkdirAll(LocalCopyPath(), os.ModePerm); err != nil { - log.Error("Unable to create localcopypath directory: %s (%v)", LocalCopyPath(), err) - return "", fmt.Errorf("Failed to create localcopypath directory %s: %w", LocalCopyPath(), err) +func CreateTemporaryPath(prefix string) (string, func(), error) { + if err := os.MkdirAll(localCopyPath(), os.ModePerm); err != nil { + log.Error("Unable to create localcopypath directory: %s (%v)", localCopyPath(), err) + return "", func() {}, fmt.Errorf("failed to create localcopypath directory %s: %w", localCopyPath(), err) } - basePath, err := os.MkdirTemp(LocalCopyPath(), prefix+".git") + basePath, err := os.MkdirTemp(localCopyPath(), prefix+".git") if err != nil { log.Error("Unable to create temporary directory: %s-*.git (%v)", prefix, err) - return "", fmt.Errorf("Failed to create dir %s-*.git: %w", prefix, err) + return "", func() {}, fmt.Errorf("failed to create dir %s-*.git: %w", prefix, err) } - return basePath, nil + return basePath, func() { + if err := removeTemporaryPath(basePath); err != nil { + log.Error("Unable to remove temporary directory: %s (%v)", basePath, err) + } + }, nil } -// RemoveTemporaryPath removes the temporary path -func RemoveTemporaryPath(basePath string) error { +// removeTemporaryPath removes the temporary path +func removeTemporaryPath(basePath string) error { if _, err := os.Stat(basePath); !os.IsNotExist(err) { return util.RemoveAll(basePath) } diff --git a/modules/setting/global.go b/modules/setting/global.go index 55dfe485b2..1e119ffee1 100644 --- a/modules/setting/global.go +++ b/modules/setting/global.go @@ -3,6 +3,8 @@ package setting +import "os" + // Global settings var ( // RunUser is the OS user that Gitea is running as. ini:"RUN_USER" @@ -16,3 +18,8 @@ var ( // AppName is the Application name, used in the page title. ini: "APP_NAME" AppName string ) + +// TempDir returns the OS temp directory +func TempDir() string { + return os.TempDir() +} diff --git a/modules/setting/ssh.go b/modules/setting/ssh.go index ea387e521f..deac56c285 100644 --- a/modules/setting/ssh.go +++ b/modules/setting/ssh.go @@ -4,7 +4,6 @@ package setting import ( - "os" "path" "path/filepath" "strings" @@ -124,7 +123,7 @@ func loadSSHFrom(rootCfg ConfigProvider) { if len(serverMACs) > 0 { SSH.ServerMACs = serverMACs } - SSH.KeyTestPath = os.TempDir() + SSH.KeyTestPath = TempDir() if err = sec.MapTo(&SSH); err != nil { log.Fatal("Failed to map SSH settings: %v", err) } diff --git a/modules/storage/local.go b/modules/storage/local.go index 9bb532f1df..ab94a490bb 100644 --- a/modules/storage/local.go +++ b/modules/storage/local.go @@ -36,7 +36,7 @@ func NewLocalStorage(ctx context.Context, config *setting.Storage) (ObjectStorag } if config.TemporaryPath == "" { - config.TemporaryPath = filepath.Join(config.Path, "tmp") + config.TemporaryPath = filepath.Join(setting.TempDir(), filepath.Base(config.Path)) } if !filepath.IsAbs(config.TemporaryPath) { return nil, fmt.Errorf("LocalStorageConfig.TemporaryPath should be an absolute path, but not: %q", config.TemporaryPath) diff --git a/modules/storage/local_test.go b/modules/storage/local_test.go index e230323f67..747ec6f58a 100644 --- a/modules/storage/local_test.go +++ b/modules/storage/local_test.go @@ -4,7 +4,6 @@ package storage import ( - "os" "path/filepath" "testing" @@ -56,6 +55,6 @@ func TestBuildLocalPath(t *testing.T) { } func TestLocalStorageIterator(t *testing.T) { - dir := filepath.Join(os.TempDir(), "TestLocalStorageIteratorTestDir") + dir := filepath.Join(t.TempDir(), "TestLocalStorageIteratorTestDir") testStorageIterator(t, setting.LocalStorageType, &setting.Storage{Path: dir}) } diff --git a/modules/util/filebuffer/file_backed_buffer.go b/modules/util/filebuffer/file_backed_buffer.go index 739543e297..fdc8352f17 100644 --- a/modules/util/filebuffer/file_backed_buffer.go +++ b/modules/util/filebuffer/file_backed_buffer.go @@ -9,6 +9,8 @@ import ( "io" "math" "os" + + "code.gitea.io/gitea/modules/setting" ) var ( @@ -73,7 +75,7 @@ func (b *FileBackedBuffer) Write(p []byte) (int, error) { n, err = b.file.Write(p) } else { if b.size+int64(len(p)) > b.maxMemorySize { - b.file, err = os.CreateTemp("", "gitea-buffer-") + b.file, err = os.CreateTemp(setting.TempDir(), "gitea-buffer-") if err != nil { return 0, err } diff --git a/modules/util/legacy_test.go b/modules/util/legacy_test.go index e732094c29..91d60f9f1c 100644 --- a/modules/util/legacy_test.go +++ b/modules/util/legacy_test.go @@ -11,13 +11,15 @@ import ( "testing" "time" + "code.gitea.io/gitea/modules/setting" + "github.com/stretchr/testify/assert" ) func TestCopyFile(t *testing.T) { testContent := []byte("hello") - tmpDir := os.TempDir() + tmpDir := setting.TempDir() now := time.Now() srcFile := fmt.Sprintf("%s/copy-test-%d-src.txt", tmpDir, now.UnixMicro()) dstFile := fmt.Sprintf("%s/copy-test-%d-dst.txt", tmpDir, now.UnixMicro()) diff --git a/routers/web/repo/githttp.go b/routers/web/repo/githttp.go index ee1ec1fd0c..e040c025ab 100644 --- a/routers/web/repo/githttp.go +++ b/routers/web/repo/githttp.go @@ -303,7 +303,7 @@ var ( func dummyInfoRefs(ctx *context.Context) { infoRefsOnce.Do(func() { - tmpDir, err := os.MkdirTemp(os.TempDir(), "gitea-info-refs-cache") + tmpDir, err := os.MkdirTemp(setting.TempDir(), "gitea-info-refs-cache") if err != nil { log.Error("Failed to create temp dir for git-receive-pack cache: %v", err) return diff --git a/routers/web/repo/setting/lfs.go b/routers/web/repo/setting/lfs.go index fad6359668..e70503ed0c 100644 --- a/routers/web/repo/setting/lfs.go +++ b/routers/web/repo/setting/lfs.go @@ -109,17 +109,13 @@ func LFSLocks(ctx *context.Context) { } // Clone base repo. - tmpBasePath, err := repo_module.CreateTemporaryPath("locks") + tmpBasePath, cancel, err := repo_module.CreateTemporaryPath("locks") if err != nil { log.Error("Failed to create temporary path: %v", err) ctx.ServerError("LFSLocks", err) return } - defer func() { - if err := repo_module.RemoveTemporaryPath(tmpBasePath); err != nil { - log.Error("LFSLocks: RemoveTemporaryPath: %v", err) - } - }() + defer cancel() if err := git.Clone(ctx, ctx.Repo.Repository.RepoPath(), tmpBasePath, git.CloneRepoOptions{ Bare: true, diff --git a/services/pull/patch.go b/services/pull/patch.go index 0934a86c89..37eb6b70e2 100644 --- a/services/pull/patch.go +++ b/services/pull/patch.go @@ -346,7 +346,7 @@ func checkConflicts(ctx context.Context, pr *issues_model.PullRequest, gitRepo * } // 3b. Create a plain patch from head to base - tmpPatchFile, err := os.CreateTemp("", "patch") + tmpPatchFile, err := os.CreateTemp(setting.TempDir(), "patch") if err != nil { log.Error("Unable to create temporary patch file! Error: %v", err) return false, fmt.Errorf("unable to create temporary patch file! Error: %w", err) diff --git a/services/pull/temp_repo.go b/services/pull/temp_repo.go index e5753178b8..99f9d87ad0 100644 --- a/services/pull/temp_repo.go +++ b/services/pull/temp_repo.go @@ -73,11 +73,13 @@ func createTemporaryRepoForPR(ctx context.Context, pr *issues_model.PullRequest) } // Clone base repo. - tmpBasePath, err := repo_module.CreateTemporaryPath("pull") + tmpBasePath, cancel, err := repo_module.CreateTemporaryPath("pull") if err != nil { log.Error("CreateTemporaryPath[%-v]: %v", pr, err) return nil, nil, err } + defer cancel() + prCtx = &prContext{ Context: ctx, tmpBasePath: tmpBasePath, @@ -85,11 +87,6 @@ func createTemporaryRepoForPR(ctx context.Context, pr *issues_model.PullRequest) outbuf: &strings.Builder{}, errbuf: &strings.Builder{}, } - cancel = func() { - if err := repo_module.RemoveTemporaryPath(tmpBasePath); err != nil { - log.Error("Error whilst removing removing temporary repo for %-v: %v", pr, err) - } - } baseRepoPath := pr.BaseRepo.RepoPath() headRepoPath := pr.HeadRepo.RepoPath() diff --git a/services/repository/create.go b/services/repository/create.go index 282b2d3e58..1e7c2b2c82 100644 --- a/services/repository/create.go +++ b/services/repository/create.go @@ -134,6 +134,10 @@ func prepareRepoCommit(ctx context.Context, repo *repo_model.Repository, tmpDir, return nil } +func createRepoTempDir(repoName string) (string, error) { + return os.MkdirTemp(filepath.Join(setting.TempDir(), "repos"), "gitea-"+repoName) +} + // InitRepository initializes README and .gitignore if needed. func initRepository(ctx context.Context, repoPath string, u *user_model.User, repo *repo_model.Repository, opts CreateRepoOptions) (err error) { if err = repo_module.CheckInitRepository(ctx, repo.OwnerName, repo.Name, opts.ObjectFormatName); err != nil { @@ -142,9 +146,9 @@ func initRepository(ctx context.Context, repoPath string, u *user_model.User, re // Initialize repository according to user's choice. if opts.AutoInit { - tmpDir, err := os.MkdirTemp(os.TempDir(), "gitea-"+repo.Name) + tmpDir, err := createRepoTempDir(repo.Name) if err != nil { - return fmt.Errorf("Failed to create temp dir for repository %s: %w", repo.RepoPath(), err) + return fmt.Errorf("failed to create temp dir for repository %s: %w", repo.RepoPath(), err) } defer func() { if err := util.RemoveAll(tmpDir); err != nil { diff --git a/services/repository/files/temp_repo.go b/services/repository/files/temp_repo.go index d70b1e8d54..94f8da350c 100644 --- a/services/repository/files/temp_repo.go +++ b/services/repository/files/temp_repo.go @@ -30,23 +30,28 @@ type TemporaryUploadRepository struct { repo *repo_model.Repository gitRepo *git.Repository basePath string + cancel func() } // NewTemporaryUploadRepository creates a new temporary upload repository func NewTemporaryUploadRepository(ctx context.Context, repo *repo_model.Repository) (*TemporaryUploadRepository, error) { - basePath, err := repo_module.CreateTemporaryPath("upload") + basePath, cancel, err := repo_module.CreateTemporaryPath("upload") if err != nil { return nil, err } - t := &TemporaryUploadRepository{ctx: ctx, repo: repo, basePath: basePath} + t := &TemporaryUploadRepository{ + ctx: ctx, repo: repo, + basePath: basePath, + cancel: cancel, + } return t, nil } // Close the repository cleaning up all files func (t *TemporaryUploadRepository) Close() { defer t.gitRepo.Close() - if err := repo_module.RemoveTemporaryPath(t.basePath); err != nil { - log.Error("Failed to remove temporary path %s: %v", t.basePath, err) + if t.cancel != nil { + t.cancel() } } diff --git a/services/repository/generate.go b/services/repository/generate.go index 2b95bbcd4d..336db4e5a6 100644 --- a/services/repository/generate.go +++ b/services/repository/generate.go @@ -23,6 +23,7 @@ import ( "code.gitea.io/gitea/modules/gitrepo" "code.gitea.io/gitea/modules/log" repo_module "code.gitea.io/gitea/modules/repository" + "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/util" "github.com/gobwas/glob" @@ -253,7 +254,7 @@ func generateRepoCommit(ctx context.Context, repo, templateRepo, generateRepo *r } func generateGitContent(ctx context.Context, repo, templateRepo, generateRepo *repo_model.Repository) (err error) { - tmpDir, err := os.MkdirTemp(os.TempDir(), "gitea-"+repo.Name) + tmpDir, err := os.MkdirTemp(setting.TempDir(), "gitea-"+repo.Name) if err != nil { return fmt.Errorf("Failed to create temp dir for repository %s: %w", repo.RepoPath(), err) } diff --git a/services/repository/repository.go b/services/repository/repository.go index 59b4491132..a6982ca4fd 100644 --- a/services/repository/repository.go +++ b/services/repository/repository.go @@ -107,7 +107,10 @@ func Init(ctx context.Context) error { return err } system_model.RemoveAllWithNotice(ctx, "Clean up temporary repository uploads", setting.Repository.Upload.TempPath) - system_model.RemoveAllWithNotice(ctx, "Clean up temporary repositories", repo_module.LocalCopyPath()) + repo_module.CleanUpTemporaryPaths() + if err := system_model.CreateNotice(db.DefaultContext, system_model.NoticeRepository, "Clean up temporary repositories"); err != nil { + log.Error("CreateRepositoryNotice: %v", err) + } if err := initPushQueue(); err != nil { return err } diff --git a/services/wiki/wiki.go b/services/wiki/wiki.go index 7a0419aea7..439003999f 100644 --- a/services/wiki/wiki.go +++ b/services/wiki/wiki.go @@ -102,15 +102,11 @@ func updateWikiPage(ctx context.Context, doer *user_model.User, repo *repo_model hasDefaultBranch := git.IsBranchExist(ctx, repo.WikiPath(), repo.DefaultWikiBranch) - basePath, err := repo_module.CreateTemporaryPath("update-wiki") + basePath, cancel, err := repo_module.CreateTemporaryPath("update-wiki") if err != nil { return err } - defer func() { - if err := repo_module.RemoveTemporaryPath(basePath); err != nil { - log.Error("Merge: RemoveTemporaryPath: %s", err) - } - }() + defer cancel() cloneOpts := git.CloneRepoOptions{ Bare: true, @@ -264,15 +260,11 @@ func DeleteWikiPage(ctx context.Context, doer *user_model.User, repo *repo_model return fmt.Errorf("InitWiki: %w", err) } - basePath, err := repo_module.CreateTemporaryPath("update-wiki") + basePath, cancel, err := repo_module.CreateTemporaryPath("update-wiki") if err != nil { return err } - defer func() { - if err := repo_module.RemoveTemporaryPath(basePath); err != nil { - log.Error("Merge: RemoveTemporaryPath: %s", err) - } - }() + defer cancel() if err := git.Clone(ctx, repo.WikiPath(), basePath, git.CloneRepoOptions{ Bare: true, diff --git a/tests/integration/dump_restore_test.go b/tests/integration/dump_restore_test.go index 47bb6f76e9..f5a8d0730f 100644 --- a/tests/integration/dump_restore_test.go +++ b/tests/integration/dump_restore_test.go @@ -44,7 +44,7 @@ func TestDumpRestore(t *testing.T) { reponame := "repo1" - basePath, err := os.MkdirTemp("", reponame) + basePath, err := os.MkdirTemp(setting.TempDir(), reponame) assert.NoError(t, err) defer util.RemoveAll(basePath)