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 cocoapods support to package.py #119

Merged
merged 13 commits into from
Sep 19, 2024
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
1 change: 1 addition & 0 deletions requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ text-unidecode==1.3
toml==0.10.2
typecode==30.0.0
typecode-libmagic==5.39.210531
univers==30.11.0
urllib3==1.26.9
urlpy==0.5
wcwidth==0.2.5
Expand Down
2 changes: 1 addition & 1 deletion setup.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ install_requires =
requests
python-dateutil
python-dotenv
univers == 30.11.0
Copy link
Member

Choose a reason for hiding this comment

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

Please never pin the version here. Only express what is your minimal required version. The pin goes in the requirements file.

Suggested change
univers == 30.11.0
univers >= 30.11.0

Copy link
Member Author

Choose a reason for hiding this comment

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

Glad you caught that -- thanks and fixed.



[options.packages.find]
Expand All @@ -80,4 +81,3 @@ docs =
sphinx-rtd-theme>=1.0.0
sphinx-reredirects >= 0.1.2
doc8>=0.11.2

127 changes: 127 additions & 0 deletions src/fetchcode/package.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
# specific language governing permissions and limitations under the License.

import dataclasses
import logging
import os
import re
import time
from typing import List
Expand All @@ -32,11 +34,19 @@
from fetchcode.package_util import GitHubSource
from fetchcode.package_util import MiniupnpPackagesGitHubSource
from fetchcode.package_util import OpenSSLGitHubSource
from fetchcode.package_util import construct_cocoapods_package
from fetchcode.package_util import get_cocoapod_tags
from fetchcode.package_util import get_cocoapods_org_url_status
from fetchcode.package_util import get_pod_data_with_soup
from fetchcode.packagedcode_models import Package
from fetchcode.utils import get_hashed_path
from fetchcode.utils import get_response

router = Router()

LOG_FILE_LOCATION = os.path.join(os.path.expanduser("~"), "purlcli.log")
logger = logging.getLogger(__name__)


def info(url):
"""
Expand Down Expand Up @@ -362,6 +372,123 @@ def get_gnu_data_from_purl(purl):
)


@router.route("pkg:cocoapods/.*")
def get_cocoapods_data_from_purl(purl):
Copy link
Member

Choose a reason for hiding this comment

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

@johnmhoran after refactoring get_cocoapods_data_from_purl into multiple functions, please put those functions in package_util.py and only keep the top-level get_cocoapods_data_from_purl function in package.py file

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks @keshav-space -- I was wondering about that, given how the other existing, relatively short @router.route() functions in package.py have related functions in both package_util.py and utils.py. I've already added a handful of utilities to utils.py for cocoapods support (siblings of existing utilities, but these do not throw exceptions because that stops the purlcli metadata command, which we don't want to do) and will do as you suggest with the now 4 additional functions for cocoapods created by my almost-finished refactoring. And then I have 3 or 4 mock tests to create.

Copy link
Member Author

Choose a reason for hiding this comment

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

@keshav-space Moving these related functions to package_util.py raises one question: in order to facilitate the collection and sharing of cocoapods data from a number of different sources, I've created a dictionary at the top of package.py which all functions can access. When I move some functions to package_util.py, will continued access be as simple as importing that dictionary from package.py into package_util.py? That's my plan atm.

Copy link
Member Author

Choose a reason for hiding this comment

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

@keshav-space I am having trouble importing and accessing in package_util.py the logger I've defined and use widely in my package.py code. I'll dig into this soon, but meanwhile, do you have any guidance on how to share a logging function -- this prints to screen and to the "errors"/"warnings" keys in the JSON output. I now import in package_util.py with from fetchcode.package import logger but get this error running metadata:

(venv) Wed May 01, 2024 08:33 AM  /home/jmh/dev/nexb/purldb jmh (365-update-cocoapods-pypi-support)
$ python -m purldb_toolkit.purlcli metadata --purl pkg:cocoapods/BoringSSL@10.0.6 --output -
Traceback (most recent call last):
  File "/usr/lib/python3.10/runpy.py", line 196, in _run_module_as_main
    return _run_code(code, main_globals, None,
  File "/usr/lib/python3.10/runpy.py", line 86, in _run_code
    exec(code, run_globals)
  File "/home/jmh/dev/nexb/purldb/purldb-toolkit/src/purldb_toolkit/purlcli.py", line 19, in <module>
    from fetchcode.package import info
  File "/home/jmh/dev/nexb/fetchcode/src/fetchcode/package.py", line 32, in <module>
    from fetchcode.package_util import GITHUB_SOURCE_BY_PACKAGE
  File "/home/jmh/dev/nexb/fetchcode/src/fetchcode/package_util.py", line 25, in <module>
    from fetchcode.package import logger
ImportError: cannot import name 'logger' from partially initialized module 'fetchcode.package' (most likely due to a circular import) (/home/jmh/dev/nexb/fetchcode/src/fetchcode/package.py)

(venv) Wed May 01, 2024 08:48 AM  /home/jmh/dev/nexb/purldb jmh (365-update-cocoapods-pypi-support)
$

Copy link
Member

Choose a reason for hiding this comment

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

@johnmhoran please don't share the same logger across different files. Define a new logger for package_util.py and avoid any circular dependencies i.e. don't import anything from package.py in package_util.py. The error above is due to a circular dependency.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks @keshav-space . I've defined the logger in each of package.py and package_util.py (configured in get_cocoapods_data_from_purl()), and have defined the pod_summary dictionary in package_util.py and import it into package.py (pod_summary is shared among functions in both files), and everything seems to still work as desired. 👍

"""
Generate `Package` object from the `purl` string of cocoapods type
"""
logging.basicConfig(
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 logging in the library, but I am sure we do not want logging to be enabled and configured here at all.

Copy link
Member Author

Choose a reason for hiding this comment

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

Confirming that my added logging has been removed.

filename=LOG_FILE_LOCATION,
level=logging.WARN,
format="%(levelname)s - %(message)s",
filemode="w",
)

purl = PackageURL.from_string(purl)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Put this in try/except block, given input may not be a valid PURL

Copy link
Member Author

@johnmhoran johnmhoran Apr 29, 2024

Choose a reason for hiding this comment

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

Thank you @TG1999 . I'm in the midst of refactoring but will add this to the updated code. One note: there are nearly a dozen other uses of that same syntax by other supported PURL types in package.py and none uses a try/except (but perhaps should?).

Copy link
Member Author

Choose a reason for hiding this comment

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

@TG1999 On second thought, purldb-toolkit's purlcli.py already handles invalid PURL inputs by checking the validate endpoint (including for the metadata command, which is the command that calls the fetchcode package.py info() function) and prints a warning to the output JSON warnings list, so I don't think a try/except is needed in the package.py cocoapods code. E.g.,

(venv) Mon Apr 29, 2024 12:25 PM  /home/jmh/dev/nexb/purldb jmh (365-update-cocoapods-pypi-support)
$ python -m purldb_toolkit.purlcli metadata --purl pkg:cocoapods/# --output -
{
    "headers": [
        {
            "tool_name": "purlcli",
            "tool_version": "0.2.0",
            "options": {
                "command": "metadata",
                "--purl": [
                    "pkg:cocoapods/#"
                ],
                "--file": null,
                "--output": "<stdout>"
            },
            "purls": [
                "pkg:cocoapods/#"
            ],
            "errors": [],
            "warnings": [
                "'pkg:cocoapods/#' not valid"
            ]
        }
    ],
    "packages": []
}
(venv) Mon Apr 29, 2024 12:29 PM  /home/jmh/dev/nexb/purldb jmh (365-update-cocoapods-pypi-support)
$

name = purl.name
version = purl.version
cocoapods_org_url = f"https://cocoapods.org/pods/{name}"
repository_homepage_url = 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.

why track two variables with the exact same value?

Copy link
Member Author

Choose a reason for hiding this comment

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

Deleted.


purl_to_cocoapods_org_url_status = get_cocoapods_org_url_status(purl, name, cocoapods_org_url)
cocoa_org_url_status = purl_to_cocoapods_org_url_status["return_message"]

status_values = [
Copy link
Member

Choose a reason for hiding this comment

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

Do we have all these statuses in other package implementations?
I am not sure we need to track all these. Having errors and exceptions bubble up is fine here, especially in a low level library. If we need to do more refined handling of errors when we fetch, this should not be something special to cocoapods IMHO.

Copy link
Member Author

Choose a reason for hiding this comment

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

These have been removed along with all related logging, messaging etc.

"cocoapods_org_redirects_to_github",
"cocoapods_org_url_redirects",
"failed_to_fetch_github_redirect",
"github_redirect_error",
"github_redirect_not_found",
]

cocoa_org_url_status_code = None
if cocoa_org_url_status == "cocoapods_org_url_not_found":
cocoa_org_url_status_code = 404
elif cocoa_org_url_status == "cocoapods_org_url_temporarily_unavailable":
cocoa_org_url_status_code = 503
elif any(cocoa_org_url_status == status for status in status_values):
keshav-space marked this conversation as resolved.
Show resolved Hide resolved
cocoa_org_url_status_code = 302

if (
cocoa_org_url_status == "cocoapods_org_url_not_found"
or cocoa_org_url_status == "cocoapods_org_url_redirects"
or cocoa_org_url_status == "cocoapods_org_url_temporarily_unavailable"
or cocoa_org_url_status == "failed_to_fetch_github_redirect"
or cocoa_org_url_status == "github_redirect_error"
or cocoa_org_url_status == "github_redirect_not_found"
):
keshav-space marked this conversation as resolved.
Show resolved Hide resolved
return

purl_to_pod_data_with_soup = {}
if cocoa_org_url_status_code != 302 and cocoa_org_url_status_code != 503:
keshav-space marked this conversation as resolved.
Show resolved Hide resolved
purl_to_pod_data_with_soup = get_pod_data_with_soup(purl, name, cocoapods_org_url)

cocoapods_org_pod_name = None
if purl_to_pod_data_with_soup.get('cocoapods_org_pod_name') is not None:
keshav-space marked this conversation as resolved.
Show resolved Hide resolved
cocoapods_org_pod_name = purl_to_pod_data_with_soup["cocoapods_org_pod_name"]
elif purl_to_cocoapods_org_url_status.get('cocoapods_org_pod_name') is not None:
keshav-space marked this conversation as resolved.
Show resolved Hide resolved
cocoapods_org_pod_name = purl_to_cocoapods_org_url_status["cocoapods_org_pod_name"]

cocoapods_org_version = None
cocoapods_org_gh_repo_owner = None
cocoapods_org_gh_repo_name = None

if purl_to_pod_data_with_soup.get("cocoapods_org_version") is not None:
keshav-space marked this conversation as resolved.
Show resolved Hide resolved
cocoapods_org_version = purl_to_pod_data_with_soup["cocoapods_org_version"]
elif purl_to_cocoapods_org_url_status.get("cocoapods_org_version") is not None:
keshav-space marked this conversation as resolved.
Show resolved Hide resolved
cocoapods_org_version = purl_to_cocoapods_org_url_status[
"cocoapods_org_version"
]

if purl_to_pod_data_with_soup.get("cocoapods_org_gh_repo_owner") is not None:
keshav-space marked this conversation as resolved.
Show resolved Hide resolved
cocoapods_org_gh_repo_owner = purl_to_pod_data_with_soup[
"cocoapods_org_gh_repo_owner"
]
elif (
purl_to_cocoapods_org_url_status.get("cocoapods_org_gh_repo_owner") is not None
):
keshav-space marked this conversation as resolved.
Show resolved Hide resolved
cocoapods_org_gh_repo_owner = purl_to_cocoapods_org_url_status[
"cocoapods_org_gh_repo_owner"
]

if purl_to_pod_data_with_soup.get("cocoapods_org_gh_repo_name") is not None:
keshav-space marked this conversation as resolved.
Show resolved Hide resolved
cocoapods_org_gh_repo_name = purl_to_pod_data_with_soup[
"cocoapods_org_gh_repo_name"
]
elif purl_to_cocoapods_org_url_status.get("cocoapods_org_gh_repo_name") is not None:
keshav-space marked this conversation as resolved.
Show resolved Hide resolved
cocoapods_org_gh_repo_name = purl_to_cocoapods_org_url_status[
"cocoapods_org_gh_repo_name"
]

api = "https://cdn.cocoapods.org"
hashed_path = get_hashed_path(cocoapods_org_pod_name)
hashed_path_underscore = hashed_path.replace("/", "_")
file_prefix = "all_pods_versions_"
spec = f"{api}/{file_prefix}{hashed_path_underscore}.txt"
data_list = get_cocoapod_tags(spec, cocoapods_org_pod_name)
if not version:
version = cocoapods_org_version
for tag in data_list:
if purl.version and tag != purl.version:
continue

tag_pkg = construct_cocoapods_package(
purl,
name,
hashed_path,
repository_homepage_url,
cocoapods_org_gh_repo_owner,
cocoapods_org_gh_repo_name,
tag,
cocoapods_org_pod_name
)

yield tag_pkg

if purl.version:
break


@dataclasses.dataclass
class DirectoryListedSource:
source_url: str = dataclasses.field(
Expand Down
Loading
Loading