Accept git remotes with periods in hostname #7386
Conversation
|
PR is now waiting for a maintainer to run the acceptance tests. Note for the maintainer: To run the acceptance tests, please comment /run-acceptance-tests on the PR Further commands available:
|
sdk/go/common/util/gitutil/git.go
Outdated
| @@ -53,8 +53,8 @@ const ( | |||
| // be sure to update its usage elsewhere in the code as well. | |||
| // The nolint instruction prevents gometalinter from complaining about the length of the line. | |||
| var ( | |||
| cloudSourceControlSSHRegex = regexp.MustCompile(`git@(?P<host_name>[a-zA-Z]*\.com|[a-zA-Z]*\.org):(?P<owner_and_repo>.*)`) //nolint | |||
| azureSourceControlSSHRegex = regexp.MustCompile(`git@([a-zA-Z]+\.)?(?P<host_name>([a-zA-Z]+\.)*[a-zA-Z]*\.com):(v[0-9]{1}/)?(?P<owner_and_repo>.*)`) //nolint | |||
| cloudSourceControlSSHRegex = regexp.MustCompile(`git@(?P<host_name>[a-zA-Z.-]*\.com|[a-zA-Z.-]*\.org):(?P<owner_and_repo>[a-zA-Z_-]*/[a-zA-Z_-]*\.git)`) //nolint | |||
There was a problem hiding this comment.
Out of curiosity - is the part of this change at the end required? If so - is there any reference you have for the format of this "owner_and_repo" section that suggests the provided new regexps are correct here? For example, can these not contain numbers? Can they not contain other unicode? I fear this might actully prevent this regexp from working on some valid repos?
There was a problem hiding this comment.
I've updated the regex to allow non-alphanumeric characters in the owner_and_repo group. I'm using this as my reference: https://github.com/jonschlinkert/is-git-url/blob/master/index.js
Generally speaking, there needs to be a way to distinguish between Azure source control and non-Azure source control. Before, the regex was doing that by treating any URL with more than one period in the host name as Azure. With this change, it's treating any URL with with multiple slashes in the owner_and_repo group as Azure.
Could there be an easier way to check if a URL is for Azure source control? Would it be sufficient to check if it contains the substring azure.com?
|
PR is now waiting for a maintainer to run the acceptance tests. Note for the maintainer: To run the acceptance tests, please comment /run-acceptance-tests on the PR Further commands available:
|
Co-authored-by: Luke Hoban <lukehoban@gmail.com>
|
PR is now waiting for a maintainer to run the acceptance tests. Note for the maintainer: To run the acceptance tests, please comment /run-acceptance-tests on the PR Further commands available:
|
|
/run-acceptance-tests |
|
Please view the results of the PR Build + Acceptance Tests Run Here |
|
PR is now waiting for a maintainer to run the acceptance tests. Note for the maintainer: To run the acceptance tests, please comment /run-acceptance-tests on the PR Further commands available:
|
|
/run-acceptance-tests |
|
Please view the results of the PR Build + Acceptance Tests Run Here |
1 similar comment
|
Please view the results of the PR Build + Acceptance Tests Run Here |
|
PR is now waiting for a maintainer to run the acceptance tests. Note for the maintainer: To run the acceptance tests, please comment /run-acceptance-tests on the PR Further commands available:
|
|
/run-acceptance-tests |
|
Please view the results of the PR Build + Acceptance Tests Run Here |
Description
Fix
cloudSourceControlSSHRegexso that it will match git remotes with periods or hyphens in the hostname. However,azureSourceControlSSHRegexdoes match hostnames with a period. As a result,TryGetVCSInfowill treat these sorts of remotes as Azure source control remotes and drop the first group. This causes updates to have an incorrect"vcs.kind"field. For example, an update with a Git remote ofgithub.foo.acme.comwill have a"vcs.kind"field offoo.acme.com. This occurs if a user is using a self-hosted GH enterprise instance.Fixes # (issue) (No issue has been opened for this)
Checklist