Repository navigation
fix(forge): parse userless scp remotes like git does and match hosts case-insensitively - #1135
Conversation
…case-insensitively
There was a problem hiding this comment.
The scp branch matches git's url_is_local_not_ssh (no ://, no / before the first :, not a drive letter), and taking the host from rsplit('@') on the pre-colon authority is what fixes group/app@v2.git. Lowercasing in valid_host reaches every consumer that matters: the SaaS suffix match, the probe cache key, api_base in build_forge, web_base_url. I could not run the suite (no cargo in this environment), but I traced each new case by hand and they hold, including file:///... falling out of the :// branch with an empty authority.
Nit: extract_repo_slug was not brought along. It still requires an @ for the scp form and still matches hosts case-sensitively, so codeberg.org:owner/repo.git and Git.Corp.com:team/app.git now resolve a host but no slug, and forge_target still fails with UnknownForge. Only github.com and gitlab.com escape that, via the hardcoded prefix loop at the top of the function. Reusing the same rule for the slug's path split, instead of the url.contains('@') && url.contains(':') test, would make the two functions agree on what an scp URL is. Not blocking: those remotes were equally broken before this PR.
Nit: is_dos_drive applies on every platform, whereas git's has_dos_drive_prefix is compiled to 0 off Windows, so real git on Linux reads C:/repos/app.git as ssh to host c. The divergence is the safer direction and I would keep it, but the PR body says it follows git's rule, so it is worth a comment on the function.
|
On |
Closes #1133. Follow-up to the review on #1132.
extract_hostonly understood the scp form (user@host:path) when an@was present. #1132 routed forge detection through it, sogithub.com:owner/repo.git(valid git syntax, no user) stopped resolving. It now follows git's own rule (url_is_local_not_sshinconnect.c): without://, a URL is scp-style when the part before the first:contains no/and is not a Windows drive letter. The host is that part with anyuser@removed.That also fixes
git@gitlab.com:group/app@v2.git, where the oldrsplit('@')took the host from after the last@.valid_hostnow lowercases, sohttps://GitHub.com/o/r.gitdetects GitHub. Every consumer of the host (forge detection, probe cache key, API base URL, messages) gets the canonical form.Tests
github.com:owner/repo.githas hostgithub.comand detects GitHub@in the scp path does not move the hostC:\repos\app.git,C:/repos/app.git,/srv/git/app.git,../app.git,./dir:with-colon/app.git,file:///...The existing
extract_hostanddetect_forge_from_urlcases pass unchanged. Full suite: 2476 passed.The other point from that review,
GITHUB_TOKENgoing to any remote not recognised as GitLab, changes who receives which credential, so it is filed separately as #1134 rather than folded in here.