Address review comments

Signed-off-by: Tushar Goel <tushar.goel.dav@gmail.com>

Author: TG1999

src/_packagedcode/pypi.py

@@ -454,7 +454,7 @@ def parse_metadata(location, datasource_id, package_type):
         primary_language='Python',
         name=name,
         version=version, #TODO: https://github.com/nexB/scancode-toolkit/issues/3014
-        description=get_description(meta, str(location)),
+        description=get_description( metainfo= meta, location= str(location)),
         declared_license=get_declared_license(meta),
         keywords=get_keywords(meta),
         parties=get_parties(meta),
@@ -1934,20 +1934,19 @@ def get_requirement_from_section(section, sub_section):
     """
     content = section.get(sub_section) or ""
     for req in content.splitlines():
-        if req:
-            #pytest-mypy >= 0.9.1; \
-            req = req.replace("; \\", "")
-            # pip>=19.1 # For proper file:// URLs support.
-            if "#" in req:
-                req , _ = req.rsplit("#")
+        if not req:
+            continue
+        #pytest-mypy >= 0.9.1; \
+        req = req.replace("; \\", "")
+        # pip>=19.1 # For proper file:// URLs support.
+        if "#" in req:
+            req , _ = req.rsplit("#")
+        try:
+            Requirement(req)
+            yield req
+        except:
             #pure-eval; black; tox
             req_split_by_semi_colon = req.split(";")
             req_split_by_semi_colon = [req.strip() for req in req_split_by_semi_colon if req]
-            if len(req_split_by_semi_colon) >= 2 and not(req_split_by_semi_colon[1].startswith("python_version") # pip>=19.1 ;python_version > 3.7 
-            or req_split_by_semi_colon[1].startswith("sys_platform") # pip>=19.1 ;sys_platform = "Windows"
-            or req_split_by_semi_colon[1].startswith("platform_system") # pip>=19.1 ;platform_system = "Windows"
-            or req_split_by_semi_colon[1].startswith("platform_python_implementation")):#pytest-black>=0.3.7; platform_python_implementation != "PyPy"
-                for temp_req in req_split_by_semi_colon:
-                    yield temp_req
-            else:
+            for req in req_split_by_semi_colon:
                 yield req

src/python_inspector/__init__.py

@@ -6,3 +6,5 @@
 # See https://github.com/nexB/scancode-toolkit for support or download.
 # See https://aboutcode.org for more information about nexB OSS projects.
 #
+
+DEFAULT_PYTHON_VERSION = "3.8"

src/python_inspector/resolution.py

@@ -68,9 +68,16 @@ def get_python_version_from_env_tag(python_version: str):
     return python_version
 
 
-def get_sdist_file(repos, candidate, python_version):
+def fetch_and_extract_sdist(repos, candidate, python_version):
     """
-    Return the sdist file for a candidate.
+    Fetch and extract the source distribution (sdist) for the ``candidate`` Candidate
+    from the `repos` list of PyPiRepository
+    and a required ``python_version`` Python version.
+    Return the directory location string where the sdist has been extracted.
+    Return None if the sdist was not fetched either
+    because does not exist in any of the ``repos`` or it does not work with
+    the required ``python_version``.
+    Raise an Exception if extraction fails.
     """
     sdist = utils_pypi.download_sdist(
         name=candidate.name,
@@ -82,25 +89,23 @@ def get_sdist_file(repos, candidate, python_version):
     if not sdist:
         return
 
-    sdist_file = None
-
     if sdist.endswith(".tar.gz"):
         sdist_file = sdist.rstrip(".tar.gz")
         with tarfile.open(os.path.join(utils_pypi.CACHE_THIRDPARTY_DIR, sdist)) as file:
             file.extractall(
                 os.path.join(utils_pypi.CACHE_THIRDPARTY_DIR, "extracted_sdists", sdist_file)
             )
-    if sdist.endswith(".zip"):
+    elif sdist.endswith(".zip"):
         sdist_file = sdist.rstrip(".zip")
         with ZipFile(os.path.join(utils_pypi.CACHE_THIRDPARTY_DIR, sdist)) as zip:
             zip.extractall(
                 os.path.join(utils_pypi.CACHE_THIRDPARTY_DIR, "extracted_sdists", sdist_file)
             )
 
-    if not sdist_file:
+    else:
         raise Exception(f"Unable to extract sdist {sdist}")
 
-    return sdist_file
+    return os.path.join(utils_pypi.CACHE_THIRDPARTY_DIR, "extracted_sdists", sdist_file, sdist_file)
 
 
 class PythonInputProvider(AbstractProvider):
@@ -212,32 +217,21 @@ def get_requirements_for_package_from_pypi_simple(self, candidate):
                 if dep.scope == "install":
                     yield packaging.requirements.Requirement(str(dep.extracted_requirement))
 
-        sdist_file = get_sdist_file(
+        sdist_file = fetch_and_extract_sdist(
             repos=self.repos, candidate=candidate, python_version=python_version
         )
 
         if sdist_file:
             setup_py_path = os.path.join(
-                utils_pypi.CACHE_THIRDPARTY_DIR,
-                "extracted_sdists",
-                sdist_file,
                 sdist_file,
                 "setup.py",
             )
             setup_cfg_path = os.path.join(
-                utils_pypi.CACHE_THIRDPARTY_DIR,
-                "extracted_sdists",
-                sdist_file,
                 sdist_file,
                 "setup.cfg",
             )
-            pkg_info_path = os.path.join(
-                utils_pypi.CACHE_THIRDPARTY_DIR, "extracted_sdists", sdist_file
-            )
+            pkg_info_path = os.path.dirname(sdist_file)
             requirement_path = os.path.join(
-                utils_pypi.CACHE_THIRDPARTY_DIR,
-                "extracted_sdists",
-                sdist_file,
                 sdist_file,
                 "requirements.txt",
             )
@@ -254,30 +248,31 @@ def get_requirements_for_package_from_pypi_simple(self, candidate):
                     continue
 
                 deps = list(handler.parse(path))
-                assert len(deps) == 1, handler
-                if not deps:
-                    continue
+                assert len(deps) == 1
+
                 dependencies = deps[0].dependencies
                 for dep in dependencies:
                     if not dep.purl:
                         continue
 
+                    if dep.scope != "install":
+                        continue
+
                     dep_purl = PackageURL.from_string(dep.purl)
-                    if not (
-                        dep.scope == "install"
-                        and (
-                            not (dep.is_resolved)
-                            or (dep.is_resolved and dep_purl.name not in self.resolved_requirements)
-                        )
-                    ):
+
+                    if self.is_dep_resolved_and_in_resolved_requirements(dep, dep_purl):
                         continue
+
                     if dep.is_resolved:
-                        self.resolved_requirements = (*self.resolved_requirements, dep_purl)
+                        self.resolved_requirements.append(dep_purl)
                     # skip the requirement starting with -- like
                     # --editable, --requirement
                     if not dep.extracted_requirement.startswith("--"):
                         yield packaging.requirements.Requirement(str(dep.extracted_requirement))
 
+    def is_dep_resolved_and_in_resolved_requirements(self, dep, dep_purl):
+        return dep.is_resolved and dep_purl.name in self.resolved_requirements
+
     def get_requirements_for_package_from_pypi_json_api(self, purl):
         """
         Return requirements for a package from the PyPI.org JSON API
@@ -460,7 +455,7 @@ def get_resolved_dependencies(
     ]
     resolver = Resolver(
         provider=PythonInputProvider(
-            environment=environment, repos=repos, resolved_requirements=tuple(resolved_requirements)
+            environment=environment, repos=repos, resolved_requirements=resolved_requirements
         ),
         reporter=BaseReporter(),
     )

src/python_inspector/utils_pypi.py

@@ -30,6 +30,7 @@
 from packaging import version as packaging_version
 from packaging.specifiers import SpecifierSet
 
+from python_inspector import DEFAULT_PYTHON_VERSION
 from python_inspector import utils_pip_compatibility_tags
 
 """
@@ -201,7 +202,7 @@ def download_wheel(
     repos=tuple(),
     verbose=False,
     echo_func=None,
-    python_version="3.8",
+    python_version=DEFAULT_PYTHON_VERSION,
 ):
     """
     Download the wheels binary distribution(s) of package ``name`` and
@@ -258,10 +259,9 @@ def valid_distribution(distribution, python_version):
     """
     Return True if distribution is a valid distribution for the given Python version.
     """
-    return (
-        distribution.requires_python
-        and python_version in SpecifierSet(distribution.requires_python)
-    ) or not distribution.requires_python
+    if not distribution.python_requires:
+        return True
+    return python_version in SpecifierSet(distribution.python_requires)
 
 
 def download_sdist(
@@ -271,7 +271,7 @@ def download_sdist(
     repos=tuple(),
     verbose=False,
     echo_func=None,
-    python_version="3.8",
+    python_version=DEFAULT_PYTHON_VERSION,
 ):
     """
     Download the sdist source distribution of package ``name`` and ``version``
@@ -366,7 +366,7 @@ class Link(NamedTuple):
     """
 
     url: str
-    required_python: str
+    python_requires: str
 
 
 @attr.attributes
@@ -527,7 +527,7 @@ class Distribution(NameVer):
         default=None,
     )
 
-    requires_python = attr.ib(
+    python_requires = attr.ib(
         type=str,
         default="",
         metadata=dict(help="Python 'specifier' required by this distribution."),
@@ -613,18 +613,18 @@ def download(
         return self.filename
 
     @classmethod
-    def from_path_or_url_with_required_python(cls, path_or_url_with_required_python: Link):
+    def from_link(cls, link: Link):
         """
         Return a distribution built from the data found in the filename of a
         ``path_or_url`` string. Raise an exception if this is not a valid
         filename.
         """
-        requires_python = path_or_url_with_required_python.required_python
-        path_or_url = path_or_url_with_required_python.url
+        requires_python = link.python_requires
+        path_or_url = link.url
         filename = os.path.basename(path_or_url.strip("/"))
         dist = cls.from_filename(filename)
         dist.path_or_url = path_or_url
-        dist.requires_python = requires_python
+        dist.python_requires = requires_python
         return dist
 
     @classmethod
@@ -1256,9 +1256,7 @@ def dists_from_paths_or_urls_with_required_python(cls, links: List[Link]):
         installable: List[Link] = [link for link in links if link.url.endswith(EXTENSIONS)]
         for path_or_url_with_required_python in installable:
             try:
-                dist = Distribution.from_path_or_url_with_required_python(
-                    path_or_url_with_required_python
-                )
+                dist = Distribution.from_link(path_or_url_with_required_python)
                 dists.append(dist)
                 if TRACE_DEEP:
                     print(

tests/test_resolution.py

@@ -111,7 +111,7 @@ def test_get_resolved_dependencies_with_tilde_requirement_using_json_api():
     as_list = [p["package"] for p in results]
     assert as_list == [
         "pkg:pypi/click@8.1.3",
-        "pkg:pypi/flask@2.1.2",
+        "pkg:pypi/flask@2.1.3",
         "pkg:pypi/importlib-metadata@4.12.0",
         "pkg:pypi/itsdangerous@2.1.2",
         "pkg:pypi/jinja2@3.1.2",
@@ -143,7 +143,7 @@ def test_without_supported_wheels():
         "pkg:pypi/hyperlink@21.0.0",
         "pkg:pypi/idna@3.3",
         "pkg:pypi/pycparser@2.21",
-        "pkg:pypi/setuptools@63.1.0",
+        "pkg:pypi/setuptools@63.2.0",
         "pkg:pypi/txaio@22.2.1",
     ]
 

tests/test_utils.py

@@ -17,10 +17,8 @@
 from test_cli import check_json_results
 from tinynetrc import Netrc
 
-from _packagedcode.models import PackageData
 from _packagedcode.pypi import SetupCfgHandler
-from python_inspector.resolution import PythonInputProvider
-from python_inspector.resolution import get_sdist_file
+from python_inspector.resolution import fetch_and_extract_sdist
 from python_inspector.utils import get_netrc_auth
 from python_inspector.utils_pypi import PypiSimpleRepository
 
@@ -67,9 +65,9 @@ def test_parse_reqs():
 
 
 def test_get_sdist_file():
-    sdist_file = get_sdist_file(
+    sdist_file = fetch_and_extract_sdist(
         repos=tuple([PypiSimpleRepository()]),
         candidate=Candidate(name="psycopg2", version="2.7.5", extras=None),
         python_version="3.8",
     )
-    assert sdist_file == "psycopg2-2.7.5"
+    assert sdist_file == ".cache/thirdparty/extracted_sdists/psycopg2-2.7.5/psycopg2-2.7.5"