Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

git: worktree, Support relative submodule URL. #184

Merged
merged 1 commit into from Oct 20, 2020
Merged

git: worktree, Support relative submodule URL. #184

merged 1 commit into from Oct 20, 2020

Conversation

mikyk10
Copy link
Contributor

@mikyk10 mikyk10 commented Oct 13, 2020

Original issue created by @tyru at src-d/go-git#737

This PR supports relative URLs in .gitmodules which refers to the superproject's remote origin.

@mikyk10
Copy link
Contributor Author

mikyk10 commented Oct 13, 2020

I'm not sure why only 1.14.x, macos-latest failed while others succeeded. Please let me know what you think about this PR.

@mikyk10 mikyk10 marked this pull request as ready for review October 13, 2020 02:29
@mcuadros
Copy link
Member

This is a feature supported by git?

@mikyk10
Copy link
Contributor Author

mikyk10 commented Oct 14, 2020

@mcuadros
Yes, the feature is supported by git. According to the official documentation.

<repository> is the URL of the new submodule’s origin repository. This may be either an absolute URL, or (if it begins with ./ or ../), the location relative to the superproject’s default remote repository (Please note that to specify a repository foo.git which is located right next to a superproject bar.git, you’ll have to use ../foo.git instead of ./foo.git - as one might expect when following the rules for relative URLs - because the evaluation of relative URLs in Git is identical to that of relative directories).

The default remote is the remote of the remote-tracking branch of the current branch. If no such remote-tracking branch exists or the HEAD is detached, "origin" is assumed to be the default remote. If the superproject doesn’t have a default remote configured the superproject is its own authoritative upstream and the current working directory is used instead.

@mcuadros mcuadros merged commit 15aedd2 into go-git:master Oct 20, 2020
@cbauernhofer
Copy link

This fix actually contains 2 bugs.

First parentURL := filepath.Dir(origin[0].c.URLs[0]) produces a wrong parentURL in case you use https. The resulting URL will have the format http:/xyz... - so there is one slash missing because filepath.Dir() is removing it.

The second issue is that multiple relative paths are not handled correctly - so ../../submodule.git will not work as well.

As this PR is already merged new issues should be created. Will this be done by you guys?

adracus pushed a commit to adracus/go-git that referenced this pull request Oct 28, 2020
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 go-git#184 (see comments).
@dikkini
Copy link

dikkini commented Dec 2, 2020

This fix actually contains 2 bugs.

First parentURL := filepath.Dir(origin[0].c.URLs[0]) produces a wrong parentURL in case you use https. The resulting URL will have the format http:/xyz... - so there is one slash missing because filepath.Dir() is removing it.

The second issue is that multiple relative paths are not handled correctly - so ../../submodule.git will not work as well.

As this PR is already merged new issues should be created. Will this be done by you guys?

need fix. any date?

@dikkini
Copy link

dikkini commented Jan 12, 2021

How i can get this fixes in my project?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants