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

Add repo URL support and testing for cocoapods #153

Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 26 additions & 1 deletion src/packageurl/contrib/purl2url.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
# Visit https://github.com/package-url/packageurl-python for support and
# download.


Copy link
Member

Choose a reason for hiding this comment

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

Remove this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks. I've deleted this:

# Visit https://github.com/package-url/packageurl-python for support and
# download.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That comment also appears at the top of /packageurl-python/tests/contrib/test_purl2url.py -- I've deleted it there as well.

Copy link
Member

Choose a reason for hiding this comment

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

I meant to remove the empty line.

from packageurl import PackageURL
from packageurl.contrib.route import NoRouteAvailable
from packageurl.contrib.route import Router
Expand Down Expand Up @@ -75,7 +76,12 @@ def get_download_url(purl):
return download_url

# Fallback on the `download_url` qualifier when available.
purl_data = PackageURL.from_string(purl)
purl_data = None
Copy link
Member

Choose a reason for hiding this comment

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

I am not sure we want to swallow an exception here. If the purl is invalid, let the error bubble up and do not catch the exception.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks @pombredanne -- I have updated get_download_url() to the following (actually, this reverts the code to its state before I added a try/except):

def get_download_url(purl):
    """
    Return a download URL inferred from the `purl` string.
    """
    download_url = _get_url_from_router(download_router, purl)
    if download_url:
        return download_url

    # Fallback on the `download_url` qualifier when available.
    purl_data = PackageURL.from_string(purl)
    return purl_data.qualifiers.get("download_url", None)

I also removed "pkg:cocoapods/": [], from test_purl2url_get_inferred_urls() because otherwise the test fails w/o the try/except I've removed.

try:
purl_data = PackageURL.from_string(purl)
except Exception as e:
print(f"An error occurred in get_download_url(): {e}")
return
return purl_data.qualifiers.get("download_url", None)


Expand Down Expand Up @@ -304,6 +310,25 @@ def build_golang_repo_url(purl):
return f"https://pkg.go.dev/{namespace}/{name}"


@repo_router.route("pkg:cocoapods/.*")
def build_cocoapods_repo_url(purl):
"""
Return a CocoaPods repo URL from the `purl` string.
"""
purl_data = None
name = None
try:
Copy link
Member

Choose a reason for hiding this comment

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

Same as above: if the purl is invalid, let the error bubble up and do not catch the exception.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@pombredanne I'm not sure I understand your comments but hoping this is what you suggested. I've removed the try/except as well as this if condition:

    if not name:
        return

and the updated build_cocoapods_repo_url() looks like this:

@repo_router.route("pkg:cocoapods/.*")
def build_cocoapods_repo_url(purl):
    """
    Return a CocoaPods repo URL from the `purl` string.
    """
    purl_data = PackageURL.from_string(purl)
    name = purl_data.name
    return name and f"https://cocoapods.org/pods/{name}"

I also removed "pkg:cocoapods/": None, from test_purl2url_get_repo_url() because, having removed the try/except, that test fails on "pkg:cocoapods/": None, (which I'd added only to test the try/except).

purl_data = PackageURL.from_string(purl)
name = purl_data.name
except Exception as e:
print(f"An error occurred in build_cocoapods_repo_url(): {e}")
return

if not name:
return
return f"https://cocoapods.org/pods/{name}"
Copy link
Member

Choose a reason for hiding this comment

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

Use the simpler shortcut:

Suggested change
return f"https://cocoapods.org/pods/{name}"
return name and f"https://cocoapods.org/pods/{name}"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated as shown in the comment just above ^.



# Download URLs:


Expand Down
9 changes: 8 additions & 1 deletion tests/contrib/test_purl2url.py
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,9 @@ def test_purl2url_get_repo_url():
"pkg:golang/xorm.io/xorm": "https://pkg.go.dev/xorm.io/xorm",
"pkg:golang/xorm.io/xorm@v0.8.2": "https://pkg.go.dev/xorm.io/xorm@v0.8.2",
"pkg:golang/gopkg.in/ldap.v3@v3.1.0": "https://pkg.go.dev/gopkg.in/ldap.v3@v3.1.0",
"pkg:cocoapods/AFNetworking@4.0.1": "https://cocoapods.org/pods/AFNetworking",
"pkg:cocoapods/MapsIndoors@3.24.0": "https://cocoapods.org/pods/MapsIndoors",
"pkg:cocoapods/": None,
}

for purl, url in purls_url.items():
Expand Down Expand Up @@ -134,7 +137,11 @@ def test_purl2url_get_inferred_urls():
"https://gitlab.com/tg1999/firebase",
"https://gitlab.com/tg1999/firebase/-/archive/1a122122/firebase-1a122122.tar.gz",
],
"pkg:pypi/sortedcontainers@2.4.0": ["https://pypi.org/project/sortedcontainers/2.4.0/"],
Copy link
Member

Choose a reason for hiding this comment

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

Can you restore the formatting as it was? The black tests are failing

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@pombredanne I've turned off Black (it had been set in 2 VSCode locations and turned off in just one) and restored the original format for that test entry you identified. Committed and pushed just now.

"pkg:pypi/sortedcontainers@2.4.0": [
"https://pypi.org/project/sortedcontainers/2.4.0/"
],
"pkg:cocoapods/AFNetworking@4.0.1": ["https://cocoapods.org/pods/AFNetworking"],
pombredanne marked this conversation as resolved.
Show resolved Hide resolved
"pkg:cocoapods/": [],
"pkg:composer/psr/log@1.1.3": ["https://packagist.org/packages/psr/log#1.1.3"],
"pkg:rubygems/package-name": ["https://rubygems.org/gems/package-name"],
"pkg:bitbucket/birkenfeld": [],
Expand Down
Loading