From 81d429bc0f13d05770fa5a824202e07f6f80c001 Mon Sep 17 00:00:00 2001
From: Axel Christ <axel.christ@inovex.de>
Date: Wed, 28 Oct 2020 16:52:52 +0100
Subject: [PATCH] Fix relative submodule resolution

With the current behavior, the config will always hold the resolved,
absolute URL, leavin the user of go-git no choice to determine whether
the original URL is relative or not.

This changes to employ relative URL resolution only when resolving
a submodule to a repository to keep the correct configuration
'unresolved' and intact.

Change relative resolution using `filepath.Dir` to `path.Join` while
parsing both the 'root' and the relative URL with `net/url.URL`.

Adapt test to verify the new behavior.

Re-fixes #184 (see comments).
---
 submodule.go     | 24 +++++++++++++++++++++++-
 worktree.go      | 22 ----------------------
 worktree_test.go | 23 +++++++++++++++++++----
 3 files changed, 42 insertions(+), 27 deletions(-)

diff --git a/submodule.go b/submodule.go
index dff26b0d8..b6bef46cc 100644
--- a/submodule.go
+++ b/submodule.go
@@ -5,6 +5,8 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"net/url"
+	"path"
 
 	"github.com/go-git/go-billy/v5"
 	"github.com/go-git/go-git/v5/config"
@@ -131,9 +133,29 @@ func (s *Submodule) Repository() (*Repository, error) {
 		return nil, err
 	}
 
+	moduleURL, err := url.Parse(s.c.URL)
+	if err != nil {
+		return nil, err
+	}
+
+	if !path.IsAbs(moduleURL.Path) {
+		remotes, err := s.w.r.Remotes()
+		if err != nil {
+			return nil, err
+		}
+
+		rootURL, err := url.Parse(remotes[0].c.URLs[0])
+		if err != nil {
+			return nil, err
+		}
+
+		rootURL.Path = path.Join(rootURL.Path, moduleURL.Path)
+		*moduleURL = *rootURL
+	}
+
 	_, err = r.CreateRemote(&config.RemoteConfig{
 		Name: DefaultRemoteName,
-		URLs: []string{s.c.URL},
+		URLs: []string{moduleURL.String()},
 	})
 
 	return r, err
diff --git a/worktree.go b/worktree.go
index c79e71583..1c89a0221 100644
--- a/worktree.go
+++ b/worktree.go
@@ -720,31 +720,9 @@ func (w *Worktree) readGitmodulesFile() (*config.Modules, error) {
 		return m, err
 	}
 
-	w.resolveRelativeSubmodulePaths(m)
-
 	return m, nil
 }
 
-
-// resolveRelativeSubmodulePaths is to replace any relative submodule URL (../foobar.git) in .gitmodules
-// to the accessible repository URL
-func (w *Worktree) resolveRelativeSubmodulePaths(m *config.Modules) {
-	origin, err := w.r.Remotes()
-	// remote is not associated with this worktree. we don't need to process any futher more
-	if err != nil {
-		return
-	}
-
-	parentURL := filepath.Dir(origin[0].c.URLs[0])
-
-	for i := range m.Submodules {
-		if strings.HasPrefix(m.Submodules[i].URL, "../") {
-			child := strings.Replace(m.Submodules[i].URL, "../", "", 1)
-			m.Submodules[i].URL = fmt.Sprintf("%s/%s", parentURL, child)
-		}
-	}
-}
-
 // Clean the worktree by removing untracked files.
 // An empty dir could be removed - this is what  `git clean -f -d .` does.
 func (w *Worktree) Clean(opts *CleanOptions) error {
diff --git a/worktree_test.go b/worktree_test.go
index 93a4a9a37..8a7586a6e 100644
--- a/worktree_test.go
+++ b/worktree_test.go
@@ -520,7 +520,6 @@ func (s *WorktreeSuite) TestCheckoutSubmoduleInitialized(c *C) {
 	c.Assert(status.IsClean(), Equals, true)
 }
 
-
 func (s *WorktreeSuite) TestCheckoutRelativePathSubmoduleInitialized(c *C) {
 	url := "https://github.com/git-fixtures/submodule.git"
 	r := s.NewRepository(fixtures.ByURL(url).One())
@@ -547,13 +546,29 @@ func (s *WorktreeSuite) TestCheckoutRelativePathSubmoduleInitialized(c *C) {
 	// test submodule path
 	modules, err := w.readGitmodulesFile()
 
-	c.Assert(modules.Submodules["basic"].URL, Equals, "git@github.com:git-fixtures/basic.git")
-	c.Assert(modules.Submodules["itself"].URL, Equals, "git@github.com:git-fixtures/submodule.git")
+	c.Assert(modules.Submodules["basic"].URL, Equals, "../basic.git")
+	c.Assert(modules.Submodules["itself"].URL, Equals, "../submodule.git")
+
+	basicSubmodule, err := w.Submodule("basic")
+	c.Assert(err, IsNil)
+	basicRepo, err := basicSubmodule.Repository()
+	c.Assert(err, IsNil)
+	basicRemotes, err := basicRepo.Remotes()
+	c.Assert(err, IsNil)
+	c.Assert(basicRemotes[0].Config().URLs[0], Equals, "https://github.com/git-fixtures/basic.git")
+
+	itselfSubmodule, err := w.Submodule("itself")
+	c.Assert(err, IsNil)
+	itselfRepo, err := itselfSubmodule.Repository()
+	c.Assert(err, IsNil)
+	itselfRemotes, err := itselfRepo.Remotes()
+	c.Assert(err, IsNil)
+	c.Assert(itselfRemotes[0].Config().URLs[0], Equals, "https://github.com/git-fixtures/submodule.git")
 
 	sub, err := w.Submodules()
 	c.Assert(err, IsNil)
 
-	err = sub.Update(&SubmoduleUpdateOptions{Init: true, RecurseSubmodules:DefaultSubmoduleRecursionDepth})
+	err = sub.Update(&SubmoduleUpdateOptions{Init: true, RecurseSubmodules: DefaultSubmoduleRecursionDepth})
 	c.Assert(err, IsNil)
 
 	status, err := w.Status()