-
Notifications
You must be signed in to change notification settings - Fork 104
support virtual packages on generic git hosts (Gitea) #587
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
base: main
Are you sure you want to change the base?
Changes from all commits
4d5135f
cc79114
3dedd94
8cfcd22
13dbf73
c386e89
ab09b7a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -11,6 +11,7 @@ | |
| is_artifactory_path, | ||
| is_azure_devops_hostname, | ||
| is_github_hostname, | ||
| is_gitlab_hostname, | ||
| is_supported_git_host, | ||
| parse_artifactory_path, | ||
| unsupported_host_error, | ||
|
|
@@ -580,10 +581,18 @@ def _detect_virtual_package(cls, dependency_str: str): | |
| for seg in path_segments | ||
| ) | ||
| has_collection = "collections" in path_segments | ||
| # GitLab supports nested groups (group/subgroup/repo), so the full | ||
| # path is the repo -- no shorthand subdirectory splitting. | ||
| # Use https://gitlab.com/group/subgroup/repo.git for GitLab nested | ||
| # groups; shorthand subdirectory syntax is not supported for GitLab. | ||
| # All other generic hosts (Gitea, Bitbucket, self-hosted, etc.) use | ||
| # the owner/repo convention, so extra segments are a virtual subdir. | ||
|
Comment on lines
+584
to
+589
|
||
| if has_virtual_ext or has_collection: | ||
| min_base_segments = 2 | ||
| else: | ||
| elif is_gitlab_hostname(validated_host): | ||
| min_base_segments = len(path_segments) | ||
| else: | ||
| min_base_segments = 2 | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Blocking: This sets But your tests ( This is the root cause of the 4 CI failures. Either:
The Copilot bot suggested the first option -- I agree that's safer, since Gitea also supports nested orgs/groups.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, you are correct. but when I go with this logic and try to use the below structure of gitea repo to install, I am facing the issue and installation process is not completing. So, I have added this logic to work with the below structure of gitea repo to mention in apm.yml file. apm install gitea.host.com/group/repo/skills/create-pull-request#Skill_Feature |
||
| else: | ||
| min_base_segments = 2 | ||
|
|
||
|
|
@@ -734,7 +743,7 @@ def _parse_standard_url( | |
| user_repo = "/".join(parts[1:]) | ||
| else: | ||
| user_repo = "/".join(parts[1:3]) | ||
| elif len(parts) >= 2 and "." not in parts[0]: | ||
| elif len(parts) >= 2 and ("." not in parts[0] or validated_host is not None): | ||
| if not host: | ||
| host = default_host() | ||
| if is_azure_devops_hostname(host) and len(parts) >= 3: | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1656,5 +1656,147 @@ def test_try_raw_download_returns_content_on_200(self): | |
| assert result == b'hello world' | ||
|
|
||
|
|
||
| # --------------------------------------------------------------------------- | ||
| # Generic host (Gitea / GitLab) download tests | ||
| # --------------------------------------------------------------------------- | ||
|
|
||
| def _make_resp(status_code: int, content: bytes = b"") -> Mock: | ||
| """Build a minimal mock requests.Response.""" | ||
| resp = Mock() | ||
| resp.status_code = status_code | ||
| resp.content = content | ||
| if status_code >= 400: | ||
| resp.raise_for_status = Mock( | ||
| side_effect=requests_lib.exceptions.HTTPError(response=resp) | ||
| ) | ||
| else: | ||
| resp.raise_for_status = Mock() | ||
| return resp | ||
|
|
||
|
|
||
| class TestGiteaRawUrlDownload: | ||
| """Gitea raw URL path: /{owner}/{repo}/raw/{ref}/{file}.""" | ||
|
|
||
| def setup_method(self): | ||
| with patch.dict(os.environ, {}, clear=True), _CRED_FILL_PATCH: | ||
| self.downloader = GitHubPackageDownloader() | ||
|
|
||
| def test_raw_url_succeeds_on_first_attempt(self): | ||
| """Raw URL returns 200 -- content returned without calling the API.""" | ||
| dep_ref = DependencyReference.parse("gitea.myorg.com/owner/repo") | ||
| expected = b"# README content" | ||
| raw_ok = _make_resp(200, expected) | ||
|
|
||
| with patch.object(self.downloader, "_resilient_get", return_value=raw_ok) as mock_get: | ||
| result = self.downloader.download_raw_file(dep_ref, "README.md", "main") | ||
|
|
||
| assert result == expected | ||
| first_url = mock_get.call_args_list[0][0][0] | ||
| assert first_url == "https://gitea.myorg.com/owner/repo/raw/main/README.md" | ||
| assert mock_get.call_count == 1 | ||
|
|
||
| def test_raw_url_with_token_adds_auth_header(self): | ||
| """Token is forwarded as Authorization header in the raw URL request. | ||
|
|
||
| Token resolution is lazy, so the env patch must stay active for the | ||
| duration of the download call. | ||
| """ | ||
| dep_ref = DependencyReference.parse("gitea.myorg.com/owner/repo") | ||
| raw_ok = _make_resp(200, b"data") | ||
|
|
||
| with patch.dict(os.environ, {"GITHUB_APM_PAT": "gta-tok"}, clear=True): | ||
| with _CRED_FILL_PATCH: | ||
| downloader = GitHubPackageDownloader() | ||
| with patch.object(downloader, "_resilient_get", return_value=raw_ok) as mock_get: | ||
| downloader.download_raw_file(dep_ref, "README.md", "main") | ||
|
|
||
| raw_headers = mock_get.call_args_list[0][1].get("headers", {}) | ||
| assert "Authorization" in raw_headers | ||
|
|
||
| def test_falls_back_to_api_v1_when_raw_returns_non_200(self): | ||
| """When the raw URL returns 404, the API v1 path is tried next.""" | ||
| dep_ref = DependencyReference.parse("gitea.myorg.com/owner/repo") | ||
| expected = b"file via API" | ||
|
|
||
| with patch.object( | ||
| self.downloader, "_resilient_get", | ||
| side_effect=[_make_resp(404), _make_resp(200, expected)] | ||
| ) as mock_get: | ||
| result = self.downloader.download_raw_file(dep_ref, "README.md", "main") | ||
|
|
||
| assert result == expected | ||
| urls = [c[0][0] for c in mock_get.call_args_list] | ||
| assert urls[0] == "https://gitea.myorg.com/owner/repo/raw/main/README.md" | ||
| assert "/api/v1/" in urls[1] | ||
|
|
||
|
|
||
| class TestGitLabApiVersionNegotiation: | ||
| """API version negotiation: v1 -> v3 -> v4 for generic hosts.""" | ||
|
|
||
| def setup_method(self): | ||
| with patch.dict(os.environ, {}, clear=True), _CRED_FILL_PATCH: | ||
| self.downloader = GitHubPackageDownloader() | ||
|
|
||
| def test_gitlab_v4_reached_after_v1_and_v3_return_404(self): | ||
| """GitLab uses /api/v4/ -- negotiation must try v1, v3, then v4.""" | ||
| dep_ref = DependencyReference.parse("gitlab.myorg.com/owner/repo") | ||
| expected = b"gitlab file content" | ||
|
|
||
| side_effects = [ | ||
| _make_resp(404), # raw URL | ||
| _make_resp(404), # v1 | ||
| _make_resp(404), # v3 | ||
| _make_resp(200, expected), # v4 | ||
| ] | ||
| with patch.object(self.downloader, "_resilient_get", side_effect=side_effects) as mock_get: | ||
| result = self.downloader.download_raw_file(dep_ref, "skill.md", "main") | ||
|
|
||
| assert result == expected | ||
| urls = [c[0][0] for c in mock_get.call_args_list] | ||
| assert "/api/v1/" in urls[1] | ||
| assert "/api/v3/" in urls[2] | ||
| assert "/api/v4/" in urls[3] | ||
|
|
||
|
Comment on lines
+1733
to
+1759
|
||
| def test_gitea_v1_succeeds_without_trying_v3_or_v4(self): | ||
| """When v1 returns 200, v3 and v4 must never be called.""" | ||
| dep_ref = DependencyReference.parse("gitea.example.com/owner/repo") | ||
| expected = b"gitea content" | ||
|
|
||
| with patch.object( | ||
| self.downloader, "_resilient_get", | ||
| side_effect=[_make_resp(404), _make_resp(200, expected)] | ||
| ) as mock_get: | ||
| result = self.downloader.download_raw_file(dep_ref, "file.md", "main") | ||
|
|
||
| assert result == expected | ||
| urls = [c[0][0] for c in mock_get.call_args_list] | ||
| assert all("/api/v3/" not in u and "/api/v4/" not in u for u in urls) | ||
|
|
||
| def test_all_api_versions_404_raises_runtime_error(self): | ||
| """When every API version returns 404 for both refs, a clear error is raised.""" | ||
| dep_ref = DependencyReference.parse("git.example.com/owner/repo") | ||
| # raw + v1 + v3 + v4 for 'main', then v1 + v3 + v4 for 'master' fallback | ||
| side_effects = [_make_resp(404)] * 8 | ||
|
|
||
| with patch.object(self.downloader, "_resilient_get", side_effect=side_effects): | ||
| with pytest.raises(RuntimeError, match="File not found"): | ||
| self.downloader.download_raw_file(dep_ref, "missing.md", "main") | ||
|
|
||
| def test_github_com_uses_api_github_com_not_api_v4(self): | ||
| """github.com must still use api.github.com, never /api/v4/.""" | ||
| dep_ref = DependencyReference.parse("owner/repo") | ||
| expected = b"github content" | ||
| api_ok = _make_resp(200, expected) | ||
|
|
||
| with patch.object(self.downloader, "_try_raw_download", return_value=None): | ||
| with patch.object(self.downloader, "_resilient_get", return_value=api_ok) as mock_get: | ||
| result = self.downloader.download_raw_file(dep_ref, "README.md", "main") | ||
|
|
||
| assert result == expected | ||
| url_called = mock_get.call_args_list[0][0][0] | ||
| assert url_called.startswith("https://api.github.com/") | ||
| assert "/api/v4/" not in url_called | ||
|
|
||
|
|
||
| if __name__ == '__main__': | ||
| pytest.main([__file__]) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -682,3 +682,62 @@ def test_https_nested_group_with_virtual_ext_rejected(self): | |
| """HTTPS URLs can't embed virtual paths even with nested groups.""" | ||
| with pytest.raises(ValueError, match="virtual file extension"): | ||
| DependencyReference.parse("https://gitlab.com/group/subgroup/file.prompt.md") | ||
|
|
||
|
|
||
| class TestGiteaVirtualPackageDetection: | ||
| """Gitea-specific virtual package detection -- supplements TestFQDNVirtualPaths | ||
| and TestNestedGroupSupport with Gitea host fixtures and regression guards | ||
| for the len(path_segments) > 2 over-trigger.""" | ||
|
|
||
| # --- Must NOT be virtual (nested-group repo, no virtual indicators) --- | ||
|
|
||
| def test_three_segment_gitea_path_is_not_virtual(self): | ||
| """group/subgroup/repo on Gitea is a nested-group repo, not virtual.""" | ||
| dep = DependencyReference.parse("gitea.myorg.com/group/subgroup/repo") | ||
| assert dep.host == "gitea.myorg.com" | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Suggestion: Once the dep = DependencyReference.parse_from_dict({
"git": "gitea.myorg.com/group/subgroup/repo",
"path": "prompts/review.prompt.md",
})
assert dep.is_virtual is TrueThis would confirm both paths work: shorthand = repo, dict form = virtual.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| assert dep.repo_url == "group/subgroup/repo" | ||
| assert dep.is_virtual is False | ||
|
|
||
| def test_two_segment_gitea_path_is_not_virtual(self): | ||
| """Simple owner/repo on a Gitea host is never virtual.""" | ||
| dep = DependencyReference.parse("gitea.myorg.com/owner/repo") | ||
| assert dep.host == "gitea.myorg.com" | ||
| assert dep.repo_url == "owner/repo" | ||
| assert dep.is_virtual is False | ||
|
|
||
| def test_four_segment_generic_path_without_indicators_is_not_virtual(self): | ||
| """Deep nested groups without file extensions or /collections/ are not virtual.""" | ||
| dep = DependencyReference.parse("git.company.internal/team/skills/brand-guidelines") | ||
| assert dep.is_virtual is False | ||
| assert dep.repo_url == "team/skills/brand-guidelines" | ||
|
|
||
|
Comment on lines
+687
to
+713
|
||
| # --- Must be virtual (explicit virtual indicators) --- | ||
|
|
||
| def test_gitea_virtual_file_extension(self): | ||
| """Path with virtual file extension on Gitea is detected as virtual.""" | ||
| dep = DependencyReference.parse("gitea.myorg.com/owner/repo/file.prompt.md") | ||
| assert dep.host == "gitea.myorg.com" | ||
| assert dep.repo_url == "owner/repo" | ||
| assert dep.virtual_path == "file.prompt.md" | ||
| assert dep.is_virtual is True | ||
| assert dep.is_virtual_file() is True | ||
|
|
||
| def test_gitea_collections_path_is_virtual(self): | ||
| """Path with /collections/ on Gitea is detected as a virtual collection.""" | ||
| dep = DependencyReference.parse("gitea.myorg.com/owner/repo/collections/security") | ||
| assert dep.host == "gitea.myorg.com" | ||
| assert dep.repo_url == "owner/repo" | ||
| assert dep.virtual_path == "collections/security" | ||
| assert dep.is_virtual is True | ||
| assert dep.is_virtual_collection() is True | ||
|
|
||
| def test_dict_format_virtual_on_gitea(self): | ||
| """Dict format with path= on Gitea host yields a virtual package.""" | ||
| dep = DependencyReference.parse_from_dict({ | ||
| "git": "gitea.myorg.com/owner/repo", | ||
| "path": "prompts/review.prompt.md", | ||
| }) | ||
| assert dep.host == "gitea.myorg.com" | ||
| assert dep.repo_url == "owner/repo" | ||
| assert dep.virtual_path == "prompts/review.prompt.md" | ||
| assert dep.is_virtual is True | ||
Uh oh!
There was an error while loading. Please reload this page.