PR#101: Fix autodetection of repo name with SSH remote - centos/centpkg

centos / centpkg

#101 Fix autodetection of repo name with SSH remote

Merged 5 months ago by tdawson. Opened 5 months ago by sgallagh.

centos/ sgallagh/centpkg ssh_urls into develop

Remove overzealous Catch block

Stephen Gallagher • 5 months ago

54ee6b

Fix autodetection of repo name with SSH remote

Stephen Gallagher • 5 months ago

ad46d3

src/centpkg/utils.py

file modified

+11 -7

		`@@ -197,9 +197,18 @@`
		`distgit_section = '{0}.distgit'.format(cli_name)`
		`distgit_api_base_url = config_get_safely(dist_git_config, distgit_section, "apibaseurl")`

		`- # Make sure the fork comes from the same Gitlab instance`
		`parsed_repo_url = urlparse(repo_url)`
		`- parsed_base_url = urlparse(distgit_api_base_url)`
		`+ if not parsed_repo_url.scheme:`
		`+ # Some git checkouts are in the form of git@gitlab.com/...`
		`+ # If it's missing the scheme, it will treat the entire URL as the path`
		`+ # so we'll fake up the scheme for this situation`
		`+ # https://www.git-scm.com/book/en/v2/Git-Basics-Getting-a-Git-Repository`
		`+ # implies that no scheme is equivalent to git+ssh://`
		`+ # When making that conversion, we also have to replace the leading ':'`
		`+ # with a slash.`
		`+ faked_url = "git+ssh://{0}".format(repo_url.replace(":", "/", 1))`
		`+ parsed_repo_url = urlparse(faked_url)`
		`+`

		`try:`
		`distgit_token = config_get_safely(dist_git_config, distgit_section, 'token')`
		`@@ -226,11 +235,6 @@`
		`# they haven't supplied a token or it is expired.`
		`raise rpkgError("Insufficient Gitlab API permissions. Missing token?")`

		`- except Exception as e:`
		`- # For any other exception, just fall back to using the last segment`
		`- # of the URL path.`
		`- canonical_repo_name = parsed_repo_url.path.split('/')[-1]`
		`-`
		`# Chop off a trailing .git if any`
		`return canonical_repo_name.rsplit('.git', 1)[0]`

sgallagh commented 5 months ago

There are two valid forms of SSH protocol git remotes, but we were only
expecting the [git+]ssh:// form. The urllib.parse.urlparse() routine was
thus failing to determine the scheme and was treating the entire URL as
the path to pass to the Gitlab API. As a result, it was throwing a 404
exception that we were catching and ignoring.

This patch checks for a missing scheme component and if it finds one, it
transfers the remote into the other URL form for parsing purposes.

Signed-off-by: Stephen Gallagher sgallagh@redhat.com

tdawson commented 5 months ago

LGTM
Thank you for this.

1 new commit added

Remove overzealous Catch block

5 months ago

sgallagh commented 5 months ago

I updated this MR to stop catching all exceptions as well.

tdawson commented 5 months ago

That sounds like a good thing. Your fix should cause the exceptions to go way down. And if there is another failure, we want it to fail instead of silently put things in odd places.