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

Fix some mirror bugs #18649

Merged
merged 17 commits into from
Jun 11, 2022
Merged

Fix some mirror bugs #18649

merged 17 commits into from
Jun 11, 2022

Conversation

lunny
Copy link
Member

@lunny lunny commented Feb 7, 2022

This PR introduces a new git URL parse package to parse git URL.
It will fix mirror repository UI display problem.

Fix #18652

modules/git/url/url.go Outdated Show resolved Hide resolved
@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Feb 7, 2022
modules/git/url/url.go Outdated Show resolved Hide resolved
modules/git/url/url.go Outdated Show resolved Hide resolved
modules/git/url/url.go Show resolved Hide resolved
@singuliere singuliere added this to the 1.17.0 milestone Feb 7, 2022
modules/git/url/url.go Outdated Show resolved Hide resolved
modules/git/url/url.go Outdated Show resolved Hide resolved
@lunny lunny force-pushed the lunny/fix_mirror branch 2 times, most recently from 8391deb to e04309a Compare April 25, 2022 13:35
modules/templates/helper.go Outdated Show resolved Hide resolved
Copy link
Contributor

@wxiaoguang wxiaoguang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM (maybe some consts can be used for extraMark)

@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels May 16, 2022
modules/git/remote.go Outdated Show resolved Hide resolved
modules/git/remote.go Outdated Show resolved Hide resolved
return nil, err
}
return &GitURL{URL: u}, nil
} else if strings.Contains(remote, "@") && strings.Contains(remote, ":") {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The SCP target path parsing is found here: https://github.com/openssh/openssh-portable/blob/16ea8b85838dd7a4dbeba4e51ac4f43fd68b1e5b/misc.c#L767-L834

  1. If there is a '[' then we can split by the first ':' after ']'
    • note this ':' has to be immediately after the ] otherwise the whole parse fails and it's a path.
  2. Otherwise split by the first ':'
  3. We split by this ':' into <host_user> ':' <path>
    • if <path> is empty set to ".". If no ':' set all to <path>
  4. Split <host_user> by '@' into <user> '@' <host-to-clean>.
    • if no '@' set all to <host-to-clean>
  5. <host-to-clean> - strip off '[' and ']' if present to form the <host>

Copy link
Contributor

@zeripath zeripath left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Although the scp url parsing isn't exactly the same as that in ssh it appears almost equivalent.

@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Jun 11, 2022
@lunny
Copy link
Member Author

lunny commented Jun 11, 2022

make L-G-T-M work

@lunny lunny merged commit ce3dd04 into go-gitea:main Jun 11, 2022
@lunny lunny deleted the lunny/fix_mirror branch June 11, 2022 13:50
@lunny
Copy link
Member Author

lunny commented Jun 11, 2022

Removed backport because the changes too big to cherry pick.

zjjhot added a commit to zjjhot/gitea that referenced this pull request Jun 12, 2022
* giteaoffical/main:
  Fix signal loop in graceful manager (go-gitea#19943)
  Prettify number of issues (go-gitea#17760)
  Improve file header on mobile (go-gitea#19945)
  Unify repo settings & show better error (go-gitea#19828)
  [skip ci] Updated translations via Crowdin
  fixed comment typo (go-gitea#19944)
  Auto merge pull requests when all checks succeeded via WebUI (go-gitea#19648)
  Fix some mirror bugs (go-gitea#18649)
Comment on lines +977 to +979
if !m.IsMirror {
return a
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lunny I saw an issue here, the push mirrors URL are no longer displayed on the UI

image

I think we should return empty remoteAddress when the repo is a mirror not the opposite

	if m.IsMirror {
		return a
	}

AbdulrhmnGhanem pushed a commit to kitspace/gitea that referenced this pull request Aug 24, 2022
* Fix some mirror bugs

* Remove unnecessary code

* Fix lint

* rename stdard url

* Allow more charactors in git ssh protocol url

* improve the detection

* support ipv6 for git url parse

* Fix bug

* Fix template

* Fix bug

* fix template

* Fix tmpl

* Fix tmpl

* Fix parse ssh with interface

* Rename functions name

Co-authored-by: zeripath <art27@cantab.net>
@go-gitea go-gitea locked and limited conversation to collaborators May 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. type/bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Mirror information missed in repository setting page
7 participants