Add repo URL support and testing for cocoapods #153

johnmhoran · 2024-04-08T22:49:08Z

Reference: #143

@TG1999 This replaces the recently-closed PR 151. I look forward to your comments and questions.

Reference: package-url#143 Signed-off-by: John M. Horan <[email protected]>

tests/contrib/test_purl2url.py

TG1999 · 2024-04-19T15:47:41Z

src/packageurl/contrib/purl2url.py

+    purl_data = PackageURL.from_string(purl)
+    name = purl_data.name
+
+    if name:


Suggested change

if name:

if not name:

return

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

src/packageurl/contrib/purl2url.py

Reference: package-url#143 Signed-off-by: John M. Horan <[email protected]>

johnmhoran · 2024-05-30T00:58:29Z

@TG1999 Thank you for your comments. I've just pushed my responsive updates.

I added "pkg:cocoapods/": None, to test_purl2url_get_repo_url() and added "pkg:cocoapods/": [], to test_purl2url_get_inferred_urls() as you requested to test cocoapods PURL inputs with no name.
Also made a few changes to your suggested try/except approach -- I added try/except blocks to both get_download_url() and build_cocoapods_repo_url(), since get_inferred_urls() calls both get_repo_url and get_download_url and cocoapods only has repo_url support in purl2url.py (download_url support is now in fetchcode/package.py).

Note: these changes do not handle exceptions raised by other supported PURLs with no name, e.g., "pkg:cargo/abc": "https://crates.io/crates/abc", in test_purl2url_get_repo_url() raises an exception ValueError: purl is missing the required name component: 'pkg:cargo/'.

Question: since this output is consumed by the urls command in purldb-toolkit/purlcli.py, I don't want to raise an exception that will halt the underlying process of the urls command, e.g., a user might have submitted a list of 100 PURLs to query and does not want that to be interrupted. In purlcli.py and my fetchcode/package.py work I've used a "soft" fail approach that records an exception message in a shared .log file. I assume that's not an option here. As a placeholder I am printing the exception message to the console, but we do not want that in the PURLCLI tools. Do you have any suggestions?

TG1999

LGTM!

johnmhoran · 2024-05-30T17:23:38Z

@TG1999 You asked me to remove the two print statements I added to purl2url.py, one in get_download_url and the other in build_cocoapods_repo_url. Is this what we want for the cocoapods tests you asked me to add? E.g.,

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 = None
    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)

Or are we perhaps contemplating something like this:

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 = None
    try:
        purl_data = PackageURL.from_string(purl)
    except ValueError:
        return
    return purl_data.qualifiers.get("download_url", None)

pombredanne

Thanks. Do not swallow exception silently. Best is to let them propagate as-is when the purl is not valid.

pombredanne · 2024-06-05T15:26:04Z

src/packageurl/contrib/purl2url.py

@@ -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


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.

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.

pombredanne · 2024-06-05T15:26:43Z

src/packageurl/contrib/purl2url.py

+    """
+    purl_data = None
+    name = None
+    try:


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

@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).

pombredanne · 2024-06-05T15:30:56Z

src/packageurl/contrib/purl2url.py

@@ -24,6 +24,7 @@
 # Visit https://github.com/package-url/packageurl-python for support and
 # download.

+


Remove this

Thanks. I've deleted this:

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

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

I meant to remove the empty line.

pombredanne · 2024-06-05T15:33:42Z

src/packageurl/contrib/purl2url.py

+
+    if not name:
+        return
+    return f"https://cocoapods.org/pods/{name}"


Use the simpler shortcut:

Suggested change

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

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

Updated as shown in the comment just above ^.

…url#143 Reference: package-url#143 Signed-off-by: John M. Horan <[email protected]>

johnmhoran · 2024-06-05T17:56:35Z

@pombredanne @TG1999 I've just committed and pushed my changes in response to @pombredanne's recent comments. Please take a look when you can.

Reference: package-url#143 Signed-off-by: John M. Horan <[email protected]>

johnmhoran · 2024-06-07T16:51:23Z

@pombredanne I've restored the support/download URL comments we discussed.

pombredanne

Thanks. Looking good. Merging.

pombredanne · 2024-06-07T22:07:58Z

tests/contrib/test_purl2url.py

@@ -134,7 +136,10 @@ def test_purl2url_get_inferred_urls():
            "https://gitlab.com/tg1999/firebase",
            "https://gitlab.com/tg1999/firebase/-/archive/1a122122/firebase-1a122122.tar.gz",
        ],
-        "pkg:pypi/[email protected]": ["https://pypi.org/project/sortedcontainers/2.4.0/"],


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

@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.

Reference: package-url#143 Signed-off-by: John M. Horan <[email protected]>

johnmhoran · 2024-07-19T20:33:33Z

@pombredanne Just confirming that this PR is ready for your re-review when you have the time.

JonoYang · 2024-08-12T21:18:22Z

@johnmhoran This looks good, merging.

Add repo URL support and testing for cocoapods package-url#143

4bd76d5

Reference: package-url#143 Signed-off-by: John M. Horan <[email protected]>

TG1999 reviewed Apr 19, 2024

View reviewed changes

tests/contrib/test_purl2url.py Show resolved Hide resolved

TG1999 reviewed Apr 19, 2024

View reviewed changes

src/packageurl/contrib/purl2url.py Show resolved Hide resolved

Add code and tests to handle cocoapods PURL with no name package-url#143

aee327c

Reference: package-url#143 Signed-off-by: John M. Horan <[email protected]>

TG1999 previously approved these changes May 30, 2024

View reviewed changes

pombredanne requested changes Jun 5, 2024

View reviewed changes

pombredanne reviewed Jun 5, 2024

View reviewed changes

Remove cocoapods try/except blocks and update code and tests package-…

486be44

…url#143 Reference: package-url#143 Signed-off-by: John M. Horan <[email protected]>

johnmhoran dismissed TG1999’s stale review via 486be44 June 5, 2024 17:54

johnmhoran requested review from TG1999 and pombredanne June 5, 2024 17:55

Restore comments re support/download URL package-url#143

f7a88ba

Reference: package-url#143 Signed-off-by: John M. Horan <[email protected]>

pombredanne previously approved these changes Jun 7, 2024

View reviewed changes

pombredanne reviewed Jun 7, 2024

View reviewed changes

Restore prior formatting package-url#143

7ebe681

Reference: package-url#143 Signed-off-by: John M. Horan <[email protected]>

johnmhoran dismissed pombredanne’s stale review via 7ebe681 June 8, 2024 01:06

johnmhoran mentioned this pull request Jul 19, 2024

Update purlcli documentation, data structure and logging, add cocoapods support aboutcode-org/purldb#436

Open

johnmhoran requested a review from pombredanne July 19, 2024 20:32

JonoYang merged commit 04a706a into package-url:main Aug 12, 2024
5 of 19 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add repo URL support and testing for cocoapods #153

Add repo URL support and testing for cocoapods #153

johnmhoran commented Apr 8, 2024

TG1999 Apr 19, 2024

johnmhoran commented May 30, 2024

TG1999 left a comment

johnmhoran commented May 30, 2024

pombredanne left a comment

pombredanne Jun 5, 2024

johnmhoran Jun 5, 2024

pombredanne Jun 5, 2024

johnmhoran Jun 5, 2024

pombredanne Jun 5, 2024

johnmhoran Jun 5, 2024

johnmhoran Jun 5, 2024

pombredanne Jun 7, 2024

pombredanne Jun 5, 2024

johnmhoran Jun 5, 2024

johnmhoran commented Jun 5, 2024

johnmhoran commented Jun 7, 2024

pombredanne left a comment

pombredanne Jun 7, 2024

johnmhoran Jun 8, 2024

johnmhoran commented Jul 19, 2024

JonoYang commented Aug 12, 2024

		@@ -24,6 +24,7 @@
		# Visit https://github.com/package-url/packageurl-python for support and
		# download.

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

Add repo URL support and testing for cocoapods #153

Add repo URL support and testing for cocoapods #153

Conversation

johnmhoran commented Apr 8, 2024

Choose a reason for hiding this comment

johnmhoran commented May 30, 2024

TG1999 left a comment

Choose a reason for hiding this comment

johnmhoran commented May 30, 2024

pombredanne left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

johnmhoran commented Jun 5, 2024

johnmhoran commented Jun 7, 2024

pombredanne left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

johnmhoran commented Jul 19, 2024

JonoYang commented Aug 12, 2024