Introduce path Clean/Join helper functions, partially backport&refactor (#23495) (#23607)

Backport #23495, partially backport&refactor

The `modules/options` files are just copied from 1.20 to 1.19

modules/options/base.go

@@ -9,9 +9,52 @@ import (
 	"os"
 	"path/filepath"
 
+	"code.gitea.io/gitea/modules/log"
+	"code.gitea.io/gitea/modules/setting"
 	"code.gitea.io/gitea/modules/util"
 )
 
+var directories = make(directorySet)
+
+// Locale reads the content of a specific locale from static/bindata or custom path.
+func Locale(name string) ([]byte, error) {
+	return fileFromOptionsDir("locale", name)
+}
+
+// Readme reads the content of a specific readme from static/bindata or custom path.
+func Readme(name string) ([]byte, error) {
+	return fileFromOptionsDir("readme", name)
+}
+
+// Gitignore reads the content of a gitignore locale from static/bindata or custom path.
+func Gitignore(name string) ([]byte, error) {
+	return fileFromOptionsDir("gitignore", name)
+}
+
+// License reads the content of a specific license from static/bindata or custom path.
+func License(name string) ([]byte, error) {
+	return fileFromOptionsDir("license", name)
+}
+
+// Labels reads the content of a specific labels from static/bindata or custom path.
+func Labels(name string) ([]byte, error) {
+	return fileFromOptionsDir("label", name)
+}
+
+// WalkLocales reads the content of a specific locale
+func WalkLocales(callback func(path, name string, d fs.DirEntry, err error) error) error {
+	if IsDynamic() {
+		if err := walkAssetDir(filepath.Join(setting.StaticRootPath, "options", "locale"), callback); err != nil && !os.IsNotExist(err) {
+			return fmt.Errorf("failed to walk locales. Error: %w", err)
+		}
+	}
+
+	if err := walkAssetDir(filepath.Join(setting.CustomPath, "options", "locale"), callback); err != nil && !os.IsNotExist(err) {
+		return fmt.Errorf("failed to walk locales. Error: %w", err)
+	}
+	return nil
+}
+
 func walkAssetDir(root string, callback func(path, name string, d fs.DirEntry, err error) error) error {
 	if err := filepath.WalkDir(root, func(path string, d fs.DirEntry, err error) error {
 		// name is the path relative to the root
@@ -37,3 +80,55 @@ func walkAssetDir(root string, callback func(path, name string, d fs.DirEntry, e
 	}
 	return nil
 }
+
+// mustLocalPathAbs coverts a path to absolute path
+// FIXME: the old behavior (StaticRootPath might not be absolute), not ideal, just keep the same as before
+func mustLocalPathAbs(s string) string {
+	abs, err := filepath.Abs(s)
+	if err != nil {
+		// This should never happen in a real system. If it happens, the user must have already been in trouble: the system is not able to resolve its own paths.
+		log.Fatal("Unable to get absolute path for %q: %v", s, err)
+	}
+	return abs
+}
+
+func joinLocalPaths(baseDirs []string, subDir string, elems ...string) (paths []string) {
+	abs := make([]string, len(elems)+2)
+	abs[1] = subDir
+	copy(abs[2:], elems)
+	for _, baseDir := range baseDirs {
+		abs[0] = mustLocalPathAbs(baseDir)
+		paths = append(paths, util.FilePathJoinAbs(abs...))
+	}
+	return paths
+}
+
+func listLocalDirIfExist(baseDirs []string, subDir string, elems ...string) (files []string, err error) {
+	for _, localPath := range joinLocalPaths(baseDirs, subDir, elems...) {
+		isDir, err := util.IsDir(localPath)
+		if err != nil {
+			return nil, fmt.Errorf("unable to check if path %q is a directory. %w", localPath, err)
+		} else if !isDir {
+			continue
+		}
+
+		dirFiles, err := util.StatDir(localPath, true)
+		if err != nil {
+			return nil, fmt.Errorf("unable to read directory %q. %w", localPath, err)
+		}
+		files = append(files, dirFiles...)
+	}
+	return files, nil
+}
+
+func readLocalFile(baseDirs []string, subDir string, elems ...string) ([]byte, error) {
+	for _, localPath := range joinLocalPaths(baseDirs, subDir, elems...) {
+		data, err := os.ReadFile(localPath)
+		if err == nil {
+			return data, nil
+		} else if !os.IsNotExist(err) {
+			log.Error("Unable to read file %q. Error: %v", localPath, err)
+		}
+	}
+	return nil, os.ErrNotExist
+}

modules/options/dynamic.go

@@ -6,120 +6,26 @@
 package options
 
 import (
-	"fmt"
-	"io/fs"
-	"os"
-	"path"
-	"path/filepath"
-
-	"code.gitea.io/gitea/modules/log"
 	"code.gitea.io/gitea/modules/setting"
-	"code.gitea.io/gitea/modules/util"
 )
 
-var directories = make(directorySet)
-
 // Dir returns all files from static or custom directory.
 func Dir(name string) ([]string, error) {
 	if directories.Filled(name) {
 		return directories.Get(name), nil
 	}
 
-	var result []string
-
-	customDir := path.Join(setting.CustomPath, "options", name)
-
-	isDir, err := util.IsDir(customDir)
+	result, err := listLocalDirIfExist([]string{setting.CustomPath, setting.StaticRootPath}, "options", name)
 	if err != nil {
-		return []string{}, fmt.Errorf("Unabe to check if custom directory %s is a directory. %w", customDir, err)
-	}
-	if isDir {
-		files, err := util.StatDir(customDir, true)
-		if err != nil {
-			return []string{}, fmt.Errorf("Failed to read custom directory. %w", err)
-		}
-
-		result = append(result, files...)
-	}
-
-	staticDir := path.Join(setting.StaticRootPath, "options", name)
-
-	isDir, err = util.IsDir(staticDir)
-	if err != nil {
-		return []string{}, fmt.Errorf("unable to check if static directory %s is a directory. %w", staticDir, err)
-	}
-	if isDir {
-		files, err := util.StatDir(staticDir, true)
-		if err != nil {
-			return []string{}, fmt.Errorf("Failed to read static directory. %w", err)
-		}
-
-		result = append(result, files...)
+		return nil, err
 	}
 
 	return directories.AddAndGet(name, result), nil
 }
 
-// Locale reads the content of a specific locale from static or custom path.
-func Locale(name string) ([]byte, error) {
-	return fileFromDir(path.Join("locale", name))
-}
-
-// WalkLocales reads the content of a specific locale from static or custom path.
-func WalkLocales(callback func(path, name string, d fs.DirEntry, err error) error) error {
-	if err := walkAssetDir(filepath.Join(setting.StaticRootPath, "options", "locale"), callback); err != nil && !os.IsNotExist(err) {
-		return fmt.Errorf("failed to walk locales. Error: %w", err)
-	}
-
-	if err := walkAssetDir(filepath.Join(setting.CustomPath, "options", "locale"), callback); err != nil && !os.IsNotExist(err) {
-		return fmt.Errorf("failed to walk locales. Error: %w", err)
-	}
-	return nil
-}
-
-// Readme reads the content of a specific readme from static or custom path.
-func Readme(name string) ([]byte, error) {
-	return fileFromDir(path.Join("readme", name))
-}
-
-// Gitignore reads the content of a specific gitignore from static or custom path.
-func Gitignore(name string) ([]byte, error) {
-	return fileFromDir(path.Join("gitignore", name))
-}
-
-// License reads the content of a specific license from static or custom path.
-func License(name string) ([]byte, error) {
-	return fileFromDir(path.Join("license", name))
-}
-
-// Labels reads the content of a specific labels from static or custom path.
-func Labels(name string) ([]byte, error) {
-	return fileFromDir(path.Join("label", name))
-}
-
-// fileFromDir is a helper to read files from static or custom path.
-func fileFromDir(name string) ([]byte, error) {
-	customPath := path.Join(setting.CustomPath, "options", name)
-
-	isFile, err := util.IsFile(customPath)
-	if err != nil {
-		log.Error("Unable to check if %s is a file. Error: %v", customPath, err)
-	}
-	if isFile {
-		return os.ReadFile(customPath)
-	}
-
-	staticPath := path.Join(setting.StaticRootPath, "options", name)
-
-	isFile, err = util.IsFile(staticPath)
-	if err != nil {
-		log.Error("Unable to check if %s is a file. Error: %v", staticPath, err)
-	}
-	if isFile {
-		return os.ReadFile(staticPath)
-	}
-
-	return []byte{}, fmt.Errorf("Asset file does not exist: %s", name)
+// fileFromOptionsDir is a helper to read files from custom or static path.
+func fileFromOptionsDir(elems ...string) ([]byte, error) {
+	return readLocalFile([]string{setting.CustomPath, setting.StaticRootPath}, "options", elems...)
 }
 
 // IsDynamic will return false when using embedded data (-tags bindata)

modules/options/static.go

@@ -8,38 +8,20 @@ package options
 import (
 	"fmt"
 	"io"
-	"io/fs"
-	"os"
-	"path"
-	"path/filepath"
 
-	"code.gitea.io/gitea/modules/log"
 	"code.gitea.io/gitea/modules/setting"
 	"code.gitea.io/gitea/modules/util"
 )
 
-var directories = make(directorySet)
-
-// Dir returns all files from bindata or custom directory.
+// Dir returns all files from custom directory or bindata.
 func Dir(name string) ([]string, error) {
 	if directories.Filled(name) {
 		return directories.Get(name), nil
 	}
 
-	var result []string
-
-	customDir := path.Join(setting.CustomPath, "options", name)
-	isDir, err := util.IsDir(customDir)
+	result, err := listLocalDirIfExist([]string{setting.CustomPath}, "options", name)
 	if err != nil {
-		return []string{}, fmt.Errorf("unable to check if custom directory %q is a directory. %w", customDir, err)
-	}
-	if isDir {
-		files, err := util.StatDir(customDir, true)
-		if err != nil {
-			return []string{}, fmt.Errorf("unable to read custom directory %q. %w", customDir, err)
-		}
-
-		result = append(result, files...)
+		return nil, err
 	}
 
 	files, err := AssetDir(name)
@@ -69,57 +51,18 @@ func AssetDir(dirName string) ([]string, error) {
 	return results, nil
 }
 
-// Locale reads the content of a specific locale from bindata or custom path.
-func Locale(name string) ([]byte, error) {
-	return fileFromDir(path.Join("locale", name))
-}
-
-// WalkLocales reads the content of a specific locale from static or custom path.
-func WalkLocales(callback func(path, name string, d fs.DirEntry, err error) error) error {
-	if err := walkAssetDir(filepath.Join(setting.CustomPath, "options", "locale"), callback); err != nil && !os.IsNotExist(err) {
-		return fmt.Errorf("failed to walk locales. Error: %w", err)
-	}
-	return nil
-}
-
-// Readme reads the content of a specific readme from bindata or custom path.
-func Readme(name string) ([]byte, error) {
-	return fileFromDir(path.Join("readme", name))
-}
-
-// Gitignore reads the content of a gitignore locale from bindata or custom path.
-func Gitignore(name string) ([]byte, error) {
-	return fileFromDir(path.Join("gitignore", name))
-}
-
-// License reads the content of a specific license from bindata or custom path.
-func License(name string) ([]byte, error) {
-	return fileFromDir(path.Join("license", name))
-}
-
-// Labels reads the content of a specific labels from static or custom path.
-func Labels(name string) ([]byte, error) {
-	return fileFromDir(path.Join("label", name))
-}
-
-// fileFromDir is a helper to read files from bindata or custom path.
-func fileFromDir(name string) ([]byte, error) {
-	customPath := path.Join(setting.CustomPath, "options", name)
-
-	isFile, err := util.IsFile(customPath)
-	if err != nil {
-		log.Error("Unable to check if %s is a file. Error: %v", customPath, err)
-	}
-	if isFile {
-		return os.ReadFile(customPath)
+// fileFromOptionsDir is a helper to read files from custom path or bindata.
+func fileFromOptionsDir(elems ...string) ([]byte, error) {
+	// only try custom dir, no static dir
+	if data, err := readLocalFile([]string{setting.CustomPath}, "options", elems...); err == nil {
+		return data, nil
 	}
 
-	f, err := Assets.Open(name)
+	f, err := Assets.Open(util.PathJoinRelX(elems...))
 	if err != nil {
 		return nil, err
 	}
 	defer f.Close()
-
 	return io.ReadAll(f)
 }