Cleanup repo cache and fix logging

cli/merge.go

@@ -6,10 +6,10 @@ import (
 	"path/filepath"
 
 	"github.com/bitrise-io/bitrise/configmerge"
+	"github.com/bitrise-io/bitrise/log"
 	"github.com/bitrise-io/bitrise/models"
 	"github.com/bitrise-io/go-utils/fileutil"
 	"github.com/bitrise-io/go-utils/pathutil"
-	logV2 "github.com/bitrise-io/go-utils/v2/log"
 	"github.com/urfave/cli"
 )
 
@@ -35,37 +35,47 @@ func mergeConfig(c *cli.Context) error {
 	}
 	outputDir := c.String("output")
 
-	logger := logV2.NewLogger()
+	opts := log.GetGlobalLoggerOpts()
+	logger := log.NewLogger(opts)
+
 	repoCache := configmerge.NewRepoCache()
-	configReader := configmerge.NewConfigReader(repoCache, logger)
+	configReader, err := configmerge.NewConfigReader(repoCache, logger)
+	if err != nil {
+		return fmt.Errorf("failed to create config module reader: %w", err)
+	}
+	defer func() {
+		if err := configReader.CleanupRepoDirs(); err != nil {
+			log.Warnf("Failed to clean up config module repositories dir: %s", err)
+		}
+	}()
 	merger := configmerge.NewMerger(configReader, logger)
 	mergedConfigContent, configFileTree, err := merger.MergeConfig(configPth)
 	if err != nil {
-		return fmt.Errorf("failed to merge config: %s", err)
+		return fmt.Errorf("failed to merge config: %w", err)
 	}
 
 	if outputDir == "" {
 		if err := printOutputFiles(mergedConfigContent, *configFileTree, logger); err != nil {
-			return fmt.Errorf("failed to print output files: %s", err)
+			return fmt.Errorf("failed to print output files: %w", err)
 		}
 	} else {
 		if err := writeOutputFiles(mergedConfigContent, *configFileTree, outputDir); err != nil {
-			return fmt.Errorf("failed to write output files: %s", err)
+			return fmt.Errorf("failed to write output files: %w", err)
 		}
 	}
 
 	return nil
 }
 
-func printOutputFiles(mergedConfigContent string, configFileTree models.ConfigFileTreeModel, logger logV2.Logger) error {
+func printOutputFiles(mergedConfigContent string, configFileTree models.ConfigFileTreeModel, logger log.Logger) error {
 	logger.Printf("config tree:")
 	configTreeBytes, err := json.MarshalIndent(configFileTree, "", "\t")
 	if err != nil {
 		return fmt.Errorf("failed to parse config tree: %s", err)
 	}
 	logger.Printf(string(configTreeBytes))
 
-	logger.Println()
+	logger.Print()
 	logger.Printf("merged config:")
 	logger.Printf(mergedConfigContent)
 

cli/run_util.go

@@ -935,7 +935,15 @@ func CreateBitriseConfigFromCLIParams(bitriseConfigBase64Data, bitriseConfigPath
 		} else if isModularConfig {
 			logger := logV2.NewLogger()
 			repoCache := configmerge.NewRepoCache()
-			configReader := configmerge.NewConfigReader(repoCache, logger)
+			configReader, err := configmerge.NewConfigReader(repoCache, logger)
+			if err != nil {
+				return models.BitriseDataModel{}, []string{}, fmt.Errorf("failed to create config module reader: %w", err)
+			}
+			defer func() {
+				if err := configReader.CleanupRepoDirs(); err != nil {
+					log.Warnf("Failed to clean up config module repositories dir: %s", err)
+				}
+			}()
 			merger := configmerge.NewMerger(configReader, logger)
 			mergedConfigContent, _, err := merger.MergeConfig(bitriseConfigPath)
 			if err != nil {

configmerge/configmerge.go

@@ -8,7 +8,6 @@ import (
 
 	"github.com/bitrise-io/bitrise/models"
 	"github.com/bitrise-io/go-utils/sliceutil"
-	logV2 "github.com/bitrise-io/go-utils/v2/log"
 	"gopkg.in/yaml.v2"
 )
 
@@ -40,16 +39,21 @@ func IsModularConfig(mainConfigPth string) (bool, error) {
 
 type ConfigReader interface {
 	Read(ref ConfigReference) ([]byte, error)
+	CleanupRepoDirs() error
+}
+
+type Logger interface {
+	Warnf(format string, args ...interface{})
 }
 
 type Merger struct {
 	configReader ConfigReader
-	logger       logV2.Logger
+	logger       Logger
 
 	filesCount int
 }
 
-func NewMerger(configReader ConfigReader, logger logV2.Logger) Merger {
+func NewMerger(configReader ConfigReader, logger Logger) Merger {
 	return Merger{
 		configReader: configReader,
 		logger:       logger,

configmerge/file_reader.go

@@ -5,47 +5,33 @@ import (
 	"os"
 	"path/filepath"
 
-	logV2 "github.com/bitrise-io/go-utils/v2/log"
 	pathutilV2 "github.com/bitrise-io/go-utils/v2/pathutil"
-	"github.com/go-git/go-billy/v5/memfs"
 	"github.com/go-git/go-git/v5"
 	"github.com/go-git/go-git/v5/plumbing"
-	"github.com/go-git/go-git/v5/storage/memory"
 )
 
-type repoCache struct {
-	cache map[string]string
-}
-
 type RepoCache interface {
 	GetRepo(ref ConfigReference) string
 	SetRepo(dir string, ref ConfigReference)
 }
 
-func NewRepoCache() RepoCache {
-	return repoCache{
-		cache: map[string]string{},
-	}
-}
-
-func (c repoCache) GetRepo(ref ConfigReference) string {
-	return c.cache[ref.RepoKey()]
-}
-
-func (c repoCache) SetRepo(dir string, ref ConfigReference) {
-	c.cache[ref.RepoKey()] = dir
-}
-
 type fileReader struct {
 	repoCache RepoCache
-	logger    logV2.Logger
+	tmpDir    string
+	logger    Logger
 }
 
-func NewConfigReader(repoCache RepoCache, logger logV2.Logger) ConfigReader {
+func NewConfigReader(repoCache RepoCache, logger Logger) (ConfigReader, error) {
+	tmpDir, err := pathutilV2.NewPathProvider().CreateTempDir("config-merge")
+	if err != nil {
+		return nil, err
+	}
+
 	return fileReader{
 		repoCache: repoCache,
+		tmpDir:    tmpDir,
 		logger:    logger,
-	}
+	}, nil
 }
 
 func (f fileReader) Read(ref ConfigReference) ([]byte, error) {
@@ -59,7 +45,7 @@ func (f fileReader) Read(ref ConfigReference) ([]byte, error) {
 		return f.readFileFromFileSystem(pth)
 	}
 
-	repoDir, err := f.cloneGitRepository(ref.Repository, ref.Branch, ref.Commit, ref.Tag)
+	repoDir, err := f.cloneGitRepository(ref)
 	if err != nil {
 		return nil, err
 	}
@@ -69,6 +55,10 @@ func (f fileReader) Read(ref ConfigReference) ([]byte, error) {
 	return f.readFileFromFileSystem(pth)
 }
 
+func (f fileReader) CleanupRepoDirs() error {
+	return os.RemoveAll(f.tmpDir)
+}
+
 func isLocalReference(reference ConfigReference) bool {
 	return reference.Repository == ""
 }
@@ -86,27 +76,24 @@ func (f fileReader) readFileFromFileSystem(name string) ([]byte, error) {
 	return io.ReadAll(file)
 }
 
-func (f fileReader) cloneGitRepository(repository string, branch string, commit string, tag string) (string, error) {
+func (f fileReader) cloneGitRepository(ref ConfigReference) (string, error) {
 	opts := git.CloneOptions{
-		URL: repository,
+		URL: ref.Repository,
 	}
-	if branch != "" {
-		opts.ReferenceName = plumbing.NewBranchReferenceName(branch)
+	if ref.Branch != "" {
+		opts.ReferenceName = plumbing.NewBranchReferenceName(ref.Branch)
 	}
 
-	tmpDir, err := pathutilV2.NewPathProvider().CreateTempDir("config-merge")
-	if err != nil {
-		return "", err
-	}
+	repoDir := filepath.Join(f.tmpDir, ref.RepoKey())
 
-	repo, cloneErr := git.PlainClone(tmpDir, false, &opts)
+	repo, cloneErr := git.PlainClone(repoDir, false, &opts)
 	if cloneErr != nil {
-		if !isHttpFormatRepoURL(repository) {
+		if !isHttpFormatRepoURL(ref.Repository) {
 			return "", cloneErr
 		}
 
 		// Try repo url with ssh syntax
-		repoURL, err := parseGitRepoURL(repository)
+		repoURL, err := parseGitRepoURL(ref.Repository)
 		if err != nil {
 			return "", err
 		}
@@ -115,7 +102,7 @@ func (f fileReader) cloneGitRepository(repository string, branch string, commit
 		}
 
 		opts.URL = generateSCPStyleSSHFormatRepoURL(repoURL)
-		repo, err = git.PlainClone(tmpDir, false, &opts)
+		repo, err = git.PlainClone(repoDir, false, &opts)
 		if err != nil {
 			// Return the original error
 			return "", cloneErr
@@ -127,8 +114,8 @@ func (f fileReader) cloneGitRepository(repository string, branch string, commit
 		return "", err
 	}
 
-	if commit != "" {
-		h, err := repo.ResolveRevision(plumbing.Revision(commit))
+	if ref.Commit != "" {
+		h, err := repo.ResolveRevision(plumbing.Revision(ref.Commit))
 		if err != nil {
 			return "", err
 		}
@@ -138,86 +125,13 @@ func (f fileReader) cloneGitRepository(repository string, branch string, commit
 		}); err != nil {
 			return "", err
 		}
-	} else if tag != "" {
+	} else if ref.Tag != "" {
 		if err := tree.Checkout(&git.CheckoutOptions{
-			Branch: plumbing.NewTagReferenceName(tag),
+			Branch: plumbing.NewTagReferenceName(ref.Tag),
 		}); err != nil {
 			return "", err
 		}
 	}
 
-	return tmpDir, nil
-}
-
-func (f fileReader) readFileFromGitRepository(repository string, branch string, commit string, tag string, path string) ([]byte, error) {
-	opts := git.CloneOptions{
-		URL: repository,
-	}
-	if branch != "" {
-		opts.ReferenceName = plumbing.NewBranchReferenceName(branch)
-	}
-
-	repo, cloneErr := git.Clone(memory.NewStorage(), memfs.New(), &opts)
-	if cloneErr != nil {
-		if !isHttpFormatRepoURL(repository) {
-			return nil, cloneErr
-		}
-
-		// Try repo url with ssh syntax
-		repoURL, err := parseGitRepoURL(repository)
-		if err != nil {
-			return nil, err
-		}
-		if repoURL.User == "" {
-			repoURL.User = "git"
-		}
-
-		opts := git.CloneOptions{
-			URL: generateSCPStyleSSHFormatRepoURL(repoURL),
-		}
-		if branch != "" {
-			opts.ReferenceName = plumbing.NewBranchReferenceName(branch)
-		}
-
-		repo, err = git.Clone(memory.NewStorage(), memfs.New(), &opts)
-		if err != nil {
-			// Return the original error
-			return nil, cloneErr
-		}
-	}
-
-	tree, err := repo.Worktree()
-	if err != nil {
-		return nil, err
-	}
-
-	if commit != "" {
-		h, err := repo.ResolveRevision(plumbing.Revision(commit))
-		if err != nil {
-			return nil, err
-		}
-
-		if err := tree.Checkout(&git.CheckoutOptions{
-			Hash: *h,
-		}); err != nil {
-			return nil, err
-		}
-	} else if tag != "" {
-		if err := tree.Checkout(&git.CheckoutOptions{
-			Branch: plumbing.NewTagReferenceName(tag),
-		}); err != nil {
-			return nil, err
-		}
-	}
-
-	file, err := tree.Filesystem.Open(path)
-	if err != nil {
-		return nil, err
-	}
-	defer func() {
-		if err := file.Close(); err != nil {
-			f.logger.Warnf("Failed to close file: %s", err)
-		}
-	}()
-	return io.ReadAll(file)
+	return repoDir, nil
 }

configmerge/repo_cache.go

@@ -0,0 +1,19 @@
+package configmerge
+
+type repoCache struct {
+	cache map[string]string
+}
+
+func NewRepoCache() RepoCache {
+	return repoCache{
+		cache: map[string]string{},
+	}
+}
+
+func (c repoCache) GetRepo(ref ConfigReference) string {
+	return c.cache[ref.RepoKey()]
+}
+
+func (c repoCache) SetRepo(dir string, ref ConfigReference) {
+	c.cache[ref.RepoKey()] = dir
+}