Repository: go-git/go-git

Issue:
Gitmodules relative url no longer supported
Hello,

I have an application that used `go-git` to clone some Gitlab projects.

When I was testing upgrading the lib version from `v5.12.0` to `v5.13.1` I noticed that there are new changes to the logic on how submodules are being fetched.

My current config in on of the projects is as follows:

```toml
# .gitmodules
[submodule ".somedir"]
	path = ".somedir"
	url = "../../somegroup/someproject.git"
```

This configuration is no longer supported after [this change](https://github.com/go-git/go-git/pull/1141/files#diff-5a5b32fa6e206860c0f22cd41ba67393395ae481257b70b85c3ab9dc4402b4deR299) was done to the logic of how submodules `url` is handled. It has replaced the relative url path with a absolute filesystem path, which results in the error `repository not found`.

Any particular reason why this behaviour has been changed in the newer version?

Any help on this topic would be appreciated!

Thank you,
AN

cc: @pjbgf as you might be able to help here with some context 😄 

Hints from issue comments:
@ferrastas: @hiddeco maybe you can add some context on why this was done, the original PR and commit don't contain any information, though you approved it. 
Thanks!@pjbgf: This is an open-source project maintained by people on their own free time, so please refrain from pinging contributors unless you know them or they explicitly asked you to.

Sharing a code snippet (and ideally a public repository) to help reproduce the problem may be the fastest way to attain support by the community. I suspect that depending on the [Filesystem](https://github.com/go-git/go-billy/blob/18f878617b0e1dba74366c215a20cfb83f27cd78/fs.go#L52) being used you may get different results, hence why the code snippet is important.@aanogueira: As I've identified the specific issue, and found no reason for this change, decided to tag you, as I though that you might have some additional context here. Was not my intention to cause any disturbance.

I've created the following repos to replicate the issue:

* https://gitlab.com/aanogueira/go-git-issue-main - contains the go code (using the latest `go-git` version - `v5.13.2`)
* https://gitlab.com/aanogueira/go-git-issue-sub - contains the sub module

When running locally I get the previous mentioned error:

```shell
error cloning Git repository: repository not found
```

Rolling back to the previous older version (`v5.12.0`) works as expected.
@pjbgf: @aanogueira that's much appreciated, thanks for sharing the repositories above.

For context, the commit you shared is the fix for [this issue](https://github.com/go-git/go-git/security/advisories/GHSA-v725-9546-7q7m), which means we can't just revert it.

I am quite busy with the work for v6 on the little time I have for go-git, but I will try to take a look at this - if no one else does before me.@pjbgf: The fix to re-introduce relative URL support is likely better handled in [Worktree.Submodules()](https://github.com/go-git/go-git/blob/89fc507cd90333305d99d8e7b1279c6c51c9a52c/worktree.go#L878).

The reason being that the [code changed](https://github.com/go-git/go-git/pull/1141/files#diff-5a5b32fa6e206860c0f22cd41ba67393395ae481257b70b85c3ab9dc4402b4deR299) is in `parseFile`, which is specific to file transport, and it should never be reached for HTTPS/SSH relative endpoint.

A potential solution would be to amend the in-memory representation of the git modules URL so that it takes into account the `origin` remote when assessing a relative path. That way, when it is time for a [NewEndpoint](https://github.com/pjbgf/go-git/blob/9550dfab10f009362b4921bc7f45996d9ff6ce5e/plumbing/transport/common.go#L201) to be created, it goes to the correct transport instead.@pjbgf: For folks that can't use an absolute path for the submodules, a possible workaround is to change the submodule URLs before updating them:

```golang
	w, err := r.Worktree()
	CheckIfError(err)

	subs, err := w.Submodules()
	CheckIfError(err)

	for _, sub := range subs {
		config := sub.Config()

		if strings.HasPrefix(config.URL, "..") {
			new, err := newUrl(url, config.URL)
			CheckIfError(err)

			config.URL = new
		}

		Info("submodule URL %s", config.URL)
		err = sub.Init()
		CheckIfError(err)

		err = sub.Update(&git.SubmoduleUpdateOptions{RecurseSubmodules: git.DefaultSubmoduleRecursionDepth})
		CheckIfError(err)
	}
...
func newUrl(baseURL, relPath string) (string, error) {
	u, err := url.Parse(baseURL)
	if err != nil {
		return "", err
	}

	u.Path = path.Join(u.Path, relPath)
	return u.String(), nil
}
```

This won't save the changes to the URL to disk, therefore it won't have any impact if used interchangeably with the `git` cli.@cappyzawa: I'd like to work on this issue!

Based on @pjbgf's suggestion to "amend the in-memory representation of the git modules URL so that it takes into account the `origin` remote when assessing a relative path," I plan to:

- Resolve relative submodule URLs against the origin remote URL in the `Worktree.Submodules()` method
- Convert relative URLs (e.g., `../other-repo.git`) to absolute URLs using the origin remote
- Support both HTTPS and SSH protocols  
- Maintain backward compatibility for absolute URLs
- Avoid the file:// protocol that requires git binary dependency

Does this approach align with what you had in mind?@pjbgf: @cappyzawa thanks for that, I've assigned the issue to you. Your plan SGTM.

> Avoid the file:// protocol that requires git binary dependency

At some point we would need to support this protocol, however for the time being feel free to ignore it, as this is likely going to require some additional API changes.

Please work based-off the `v6-transport` branch and target it when you create your PR.@pjbgf: Closing this as per #1581.

Solution patch:
diff --git a/worktree.go b/worktree.go
index 1f42c698..0eb16ddd 100644
--- a/worktree.go
+++ b/worktree.go
@@ -5,7 +5,9 @@ import (
 	"errors"
 	"fmt"
 	"io"
+	"net/url"
 	"os"
+	"path"
 	"path/filepath"
 	"runtime"
 	"strings"
@@ -14,6 +16,7 @@ import (
 	"github.com/go-git/go-billy/v5"
 	"github.com/go-git/go-billy/v5/util"
 	"github.com/go-git/go-git/v6/config"
+	giturl "github.com/go-git/go-git/v6/internal/url"
 	"github.com/go-git/go-git/v6/plumbing"
 	"github.com/go-git/go-git/v6/plumbing/filemode"
 	"github.com/go-git/go-git/v6/plumbing/format/gitignore"
@@ -897,6 +900,31 @@ func (w *Worktree) Submodule(name string) (*Submodule, error) {
 	return nil, ErrSubmoduleNotFound
 }
 
+// resolveModuleURL resolves relative URLs based on the originURL.
+// The moduleURL is returned verbatim when originURL is empty or moduleURL is an absolute URL.
+func resolveModuleURL(originURL, moduleURL string) (string, error) {
+	if originURL == "" {
+		return moduleURL, nil
+	}
+	if !strings.HasPrefix(moduleURL, "../") && !strings.HasPrefix(moduleURL, "./") {
+		return moduleURL, nil
+	}
+	if !giturl.MatchesScheme(originURL) && giturl.MatchesScpLike(originURL) {
+		user, host, portStr, p := giturl.FindScpLikeComponents(originURL)
+		p = path.Join(p, moduleURL)
+		if portStr != "" {
+			portStr += ":"
+		}
+		return fmt.Sprintf("%s@%s:%s%s", user, host, portStr, p), nil
+	}
+	base, err := url.Parse(originURL)
+	if err != nil {
+		return "", err
+	}
+	base.Path = path.Join(base.Path, moduleURL)
+	return base.String(), nil
+}
+
 // Submodules returns all the available submodules
 func (w *Worktree) Submodules() (Submodules, error) {
 	l := make(Submodules, 0)
@@ -910,8 +938,22 @@ func (w *Worktree) Submodules() (Submodules, error) {
 		return nil, err
 	}
 
+	var originURL string
+	if origin, err := w.r.Remote(DefaultRemoteName); err == nil {
+		if origin.c != nil && len(origin.c.URLs) > 0 {
+			originURL = origin.c.URLs[0]
+		}
+	}
+
 	for _, s := range m.Submodules {
-		l = append(l, w.newSubmodule(s, c.Submodules[s.Name]))
+		sub := w.newSubmodule(s, c.Submodules[s.Name])
+		cfg := sub.Config()
+		resolvedURL, err := resolveModuleURL(originURL, cfg.URL)
+		if err != nil {
+			return nil, fmt.Errorf("failed to resolve submodule URL %q: %w", s.URL, err)
+		}
+		cfg.URL = resolvedURL
+		l = append(l, sub)
 	}
 
 	return l, nil



Test patch:
diff --git a/worktree_test.go b/worktree_test.go
index 1a3b5cd9..d359c00b 100644
--- a/worktree_test.go
+++ b/worktree_test.go
@@ -1585,6 +1585,177 @@ func (s *WorktreeSuite) TestSubmodules() {
 	s.Len(l, 2)
 }
 
+func (s *WorktreeSuite) TestSubmodules_URLResolution() {
+	tests := []struct {
+		name       string
+		originURL  string
+		gitmodules string
+		expected   map[string]string
+	}{
+		{
+			name:      "HTTPS origin with relative URL",
+			originURL: "https://github.com/user/parent-repo.git",
+			gitmodules: `[submodule "relative"]
+	path = relative
+	url = ../relative-repo.git`,
+			expected: map[string]string{
+				"relative": "https://github.com/user/relative-repo.git",
+			},
+		},
+		{
+			name:      "SSH origin with relative URL",
+			originURL: "git@github.com:user/parent-repo.git",
+			gitmodules: `[submodule "relative"]
+	path = relative
+	url = ../relative-repo.git`,
+			expected: map[string]string{
+				"relative": "git@github.com:user/relative-repo.git",
+			},
+		},
+		{
+			name:      "HTTPS origin with mixed relative and absolute URLs",
+			originURL: "https://github.com/user/parent-repo.git",
+			gitmodules: `[submodule "relative"]
+	path = relative
+	url = ../relative-repo.git
+[submodule "absolute"]
+	path = absolute
+	url = https://github.com/other/absolute-repo.git`,
+			expected: map[string]string{
+				"relative": "https://github.com/user/relative-repo.git",
+				"absolute": "https://github.com/other/absolute-repo.git",
+			},
+		},
+		{
+			name:      "file protocol with relative URL",
+			originURL: "file:///home/user/parent-repo.git",
+			gitmodules: `[submodule "relative"]
+	path = relative
+	url = ../relative-repo.git`,
+			expected: map[string]string{
+				"relative": "file:///home/user/relative-repo.git",
+			},
+		},
+	}
+
+	for _, tc := range tests {
+		s.Run(tc.name, func() {
+			fs := memfs.New()
+			r, err := Init(memory.NewStorage(), WithWorkTree(fs))
+			s.NoError(err)
+
+			_, err = r.CreateRemote(&config.RemoteConfig{
+				Name: "origin",
+				URLs: []string{tc.originURL},
+			})
+			s.NoError(err)
+
+			w, err := r.Worktree()
+			s.NoError(err)
+
+			err = util.WriteFile(fs, ".gitmodules", []byte(tc.gitmodules), 0644)
+			s.NoError(err)
+
+			submodules, err := w.Submodules()
+			s.NoError(err)
+			s.Len(submodules, len(tc.expected))
+
+			submoduleMap := make(map[string]*Submodule)
+			for _, sub := range submodules {
+				submoduleMap[sub.Config().Name] = sub
+			}
+
+			for name, expectedURL := range tc.expected {
+				sub := submoduleMap[name]
+				s.NotNil(sub, "submodule %s should exist", name)
+				s.Equal(expectedURL, sub.Config().URL)
+			}
+		})
+	}
+}
+
+func TestResolveModuleURL(t *testing.T) {
+	tests := []struct {
+		originURL string
+		moduleURL string
+		want      string
+		wantErr   error
+	}{
+		{
+			originURL: "https://github/user/repo1",
+			moduleURL: "../repo2",
+			want:      "https://github/user/repo2",
+		},
+		{
+			originURL: "https://github/user/repo1",
+			moduleURL: "https://github/user/repo3",
+			want:      "https://github/user/repo3",
+		},
+		{
+			originURL: "ssh://git@github.com:22/user/repo1.git",
+			moduleURL: "../repo2.git",
+			want:      "ssh://git@github.com:22/user/repo2.git",
+		},
+		{
+			originURL: "ssh://git@github.com:22/user/repo1.git",
+			moduleURL: "ssh://git@github.com:22/user/repo3.git",
+			want:      "ssh://git@github.com:22/user/repo3.git",
+		},
+		{
+			originURL: "git@github.com:2222:user/repo1.git",
+			moduleURL: "../repo2.git",
+			want:      "git@github.com:2222:user/repo2.git",
+		},
+		{
+			originURL: "git@github.com:user/repo1.git",
+			moduleURL: "../repo2.git",
+			want:      "git@github.com:user/repo2.git",
+		},
+		{
+			originURL: "git@github.com:user/repo1.git",
+			moduleURL: "git@github.com:user/repo3.git",
+			want:      "git@github.com:user/repo3.git",
+		},
+		{
+			originURL: "file://git/user/repo1.git",
+			moduleURL: "../repo2.git",
+			want:      "file://git/user/repo2.git",
+		},
+		{
+			originURL: "file://git/user/repo1.git",
+			moduleURL: "file://git/user/repo3.git",
+			want:      "file://git/user/repo3.git",
+		},
+		{
+			originURL: "/git/user/repo1.git",
+			moduleURL: "../repo2.git",
+			want:      "/git/user/repo2.git",
+		},
+		{
+			originURL: "/git/user/repo1.git",
+			moduleURL: "/git/user/repo3.git",
+			want:      "/git/user/repo3.git",
+		},
+		{
+			originURL: "",
+			moduleURL: "../repo2/.git",
+			want:      "../repo2/.git",
+		},
+	}
+	for _, tc := range tests {
+		t.Run(tc.want, func(t *testing.T) {
+			got, err := resolveModuleURL(tc.originURL, tc.moduleURL)
+			if tc.wantErr == nil {
+				assert.NoError(t, err)
+				assert.Equal(t, tc.want, got)
+			} else {
+				assert.Empty(t, got)
+				assert.ErrorIs(t, err, tc.wantErr)
+			}
+		})
+	}
+}
+
 func (s *WorktreeSuite) TestAddUntracked() {
 	fs := memfs.New()
 	w := &Worktree{