From e271544f105e9d73f27d9f320053503c1600d167 Mon Sep 17 00:00:00 2001
From: Nicolas Simonds <nisimond@cisco.com>
Date: Sun, 31 Dec 2023 11:27:42 -0800
Subject: [PATCH] fix: Restore proper namespace separation of cached clones

Commit d306437a045ae878a85d249282dbc838ce2ada77 pulled out the
name normalization for cache directories and left it up to the
tooling to work it out; turns out the tooling is preposterously bad
at it, and shouldn't be trusted.

Restore the needed functionality, and keep cached copies in
properly-namespaced locations under the hood.

Fixes: Issue #100
---
 internal/repository/repository.go             | 7 +++++--
 internal/repository/repository_public_test.go | 6 ++++--
 test/integration/test_cli.bats                | 2 +-
 3 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/internal/repository/repository.go b/internal/repository/repository.go
index af5a418..0cd072c 100644
--- a/internal/repository/repository.go
+++ b/internal/repository/repository.go
@@ -33,6 +33,9 @@ import (
 	"github.com/retr0h/go-gilt/pkg/config"
 )
 
+// We'll use this to normalize Git URLs as "safe" filenames
+var replacer = strings.NewReplacer("/", "-", ":", "-")
+
 // New factory to create a new Repository instance.
 func New(
 	appFs afero.Fs,
@@ -84,12 +87,12 @@ func (r *Repository) Clone(
 	c config.Repository,
 	cloneDir string,
 ) (string, error) {
+	targetDir := filepath.Join(cloneDir, replacer.Replace(c.Git))
 	r.logger.Info(
 		"cloning",
 		slog.String("repository", c.Git),
-		slog.String("dstDir", cloneDir),
+		slog.String("dstDir", targetDir),
 	)
-	targetDir := filepath.Join(cloneDir, filepath.Base(c.Git))
 
 	if _, err := r.appFs.Stat(targetDir); os.IsNotExist(err) {
 		if err := r.gitManager.Clone(c.Git, targetDir); err != nil {
diff --git a/internal/repository/repository_public_test.go b/internal/repository/repository_public_test.go
index 0a2a0b2..bf8f0d7 100644
--- a/internal/repository/repository_public_test.go
+++ b/internal/repository/repository_public_test.go
@@ -50,6 +50,7 @@ type RepositoryPublicTestSuite struct {
 	cloneDir string
 	dstDir   string
 	gitURL   string
+	cacheDir string
 	gitSHA   string
 	gitTag   string
 	logger   *slog.Logger
@@ -74,6 +75,7 @@ func (suite *RepositoryPublicTestSuite) SetupTest() {
 	suite.cloneDir = "/cloneDir"
 	suite.dstDir = "/dstDir"
 	suite.gitURL = "https://example.com/user/repo.git"
+	suite.cacheDir = "https---example.com-user-repo.git"
 	suite.gitSHA = "abc123"
 	suite.gitTag = "v1.1"
 	suite.logger = slog.New(slog.NewTextHandler(os.Stdout, nil))
@@ -86,7 +88,7 @@ func (suite *RepositoryPublicTestSuite) TestCloneOk() {
 		Git:     suite.gitURL,
 		Version: suite.gitSHA,
 	}
-	targetDir := filepath.Join(suite.cloneDir, filepath.Base(c.Git))
+	targetDir := filepath.Join(suite.cloneDir, suite.cacheDir)
 
 	gomock.InOrder(
 		suite.mockGit.EXPECT().Clone(suite.gitURL, targetDir).Return(nil),
@@ -120,7 +122,7 @@ func (suite *RepositoryPublicTestSuite) TestCloneDoesNotCloneWhenCloneDirExists(
 		Git:     suite.gitURL,
 		Version: suite.gitSHA,
 	}
-	targetDir := filepath.Join(suite.cloneDir, filepath.Base(c.Git))
+	targetDir := filepath.Join(suite.cloneDir, suite.cacheDir)
 
 	_ = suite.appFs.MkdirAll(targetDir, 0o755)
 
diff --git a/test/integration/test_cli.bats b/test/integration/test_cli.bats
index f0c2027..1c2e792 100644
--- a/test/integration/test_cli.bats
+++ b/test/integration/test_cli.bats
@@ -29,7 +29,7 @@ GILT_PROGRAM="../../../main.go"
 GILT_DIR=~/.gilt/clone
 
 setup() {
-	GILT_CLONED_REPO=${GILT_DIR}/cache/ansible-etcd.git
+	GILT_CLONED_REPO=${GILT_DIR}/cache/https---github.com-retr0h-ansible-etcd.git
 
 	GILT_CLONED_REPO_1_DST_DIR=${GILT_TEST_BASE_TMP_DIR}/retr0h.ansible-etcd
 	GILT_CLONED_REPO_2_DST_DIR=${GILT_TEST_BASE_TMP_DIR}/retr0h.ansible-etcd-tag