Merge pull request #144 from fviernau/main

Fix resolving requirements with percent encoded characters

Author: pombredanne

src/python_inspector/api.py

@@ -58,9 +58,16 @@ class Resolution(NamedTuple):
     packages: List[PackageData]
     files: List[Dict]
 
-    def to_dict(self):
+    def to_dict(self, generic_paths=False):
+        files = self.files
+        if generic_paths:
+            # clean file paths
+            for file in files:
+                path = file["path"]
+                file["path"] = utils.remove_test_data_dir_variable_prefix(path=path)
+
         return {
-            "files": self.files,
+            "files": files,
             "packages": [package for package in self.packages],
             "resolution": self.resolution,
         }
@@ -82,6 +89,7 @@ def resolve_dependencies(
     analyze_setup_py_insecurely=False,
     prefer_source=False,
     printer=print,
+    generic_paths=False,
 ):
     """
     Resolve the dependencies for the package requirements listed in one or
@@ -141,6 +149,7 @@ def resolve_dependencies(
     if PYPI_SIMPLE_URL not in index_urls:
         index_urls = tuple([PYPI_SIMPLE_URL]) + tuple(index_urls)
 
+    # requirements
     for req_file in requirement_files:
         deps = dependencies.get_dependencies_from_requirements(requirements_file=req_file)
         for extra_data in dependencies.get_extra_data_from_requirements(requirements_file=req_file):
@@ -149,6 +158,9 @@ def resolve_dependencies(
         package_data = [
             pkg_data.to_dict() for pkg_data in PipRequirementsFileHandler.parse(location=req_file)
         ]
+        if generic_paths:
+            req_file = utils.remove_test_data_dir_variable_prefix(path=req_file)
+
         files.append(
             dict(
                 type="file",
@@ -157,10 +169,12 @@ def resolve_dependencies(
             )
         )
 
+    # specs
     for specifier in specifiers:
         dep = dependencies.get_dependency(specifier=specifier)
         direct_dependencies.append(dep)
 
+    # setup.py
     if setup_py_file:
         package_data = list(PythonSetupPyHandler.parse(location=setup_py_file))
         assert len(package_data) == 1
@@ -203,6 +217,8 @@ def resolve_dependencies(
 
         package_data.dependencies = setup_py_file_deps
         file_package_data = [package_data.to_dict()]
+        if generic_paths:
+            setup_py_file = utils.remove_test_data_dir_variable_prefix(path=setup_py_file)
         files.append(
             dict(
                 type="file",
@@ -294,6 +310,9 @@ def resolve_dependencies(
     )
 
 
+resolver_api = resolve_dependencies
+
+
 def resolve(
     direct_dependencies,
     environment,

src/python_inspector/resolve_cli.py

@@ -14,7 +14,6 @@
 import click
 
 from python_inspector import utils_pypi
-from python_inspector.api import resolve_dependencies as resolver_api
 from python_inspector.cli_utils import FileOptionType
 from python_inspector.utils import write_output_in_file
 
@@ -52,9 +51,9 @@ def print_version(ctx, param, value):
     "setup_py_file",
     type=click.Path(exists=True, readable=True, path_type=str, dir_okay=False),
     metavar="SETUP-PY-FILE",
+    multiple=False,
     required=False,
-    help="Path to setuptools setup.py file listing dependencies and metadata. "
-    "This option can be used multiple times.",
+    help="Path to setuptools setup.py file listing dependencies and metadata.",
 )
 @click.option(
     "--spec",
@@ -74,7 +73,8 @@ def print_version(ctx, param, value):
     metavar="PYVER",
     show_default=True,
     required=True,
-    help="Python version to use for dependency resolution.",
+    help="Python version to use for dependency resolution. One of "
+    + ", ".join(utils_pypi.PYTHON_DOT_VERSIONS_BY_VER.values()),
 )
 @click.option(
     "-o",
@@ -84,7 +84,7 @@ def print_version(ctx, param, value):
     metavar="OS",
     show_default=True,
     required=True,
-    help="OS to use for dependency resolution.",
+    help="OS to use for dependency resolution. One of " + ", ".join(utils_pypi.PLATFORMS_BY_OS),
 )
 @click.option(
     "--index-url",
@@ -123,21 +123,23 @@ def print_version(ctx, param, value):
     metavar="NETRC-FILE",
     hidden=True,
     required=False,
-    help="Netrc file to use for authentication. ",
+    help="Netrc file to use for authentication.",
 )
 @click.option(
     "--max-rounds",
     "max_rounds",
     hidden=True,
     type=int,
     default=200000,
-    help="Increase the max rounds whenever the resolution is too deep",
+    help="Increase the maximum number of resolution rounds. "
+    "Use in the rare cases where the resolution graph is very deep.",
 )
 @click.option(
     "--use-cached-index",
     is_flag=True,
     hidden=True,
-    help="Use cached on-disk PyPI simple package indexes and do not refetch if present.",
+    help="Use cached on-disk PyPI simple package indexes "
+    "and do not refetch package index if cache is present.",
 )
 @click.option(
     "--use-pypi-json-api",
@@ -148,20 +150,19 @@ def print_version(ctx, param, value):
 @click.option(
     "--analyze-setup-py-insecurely",
     is_flag=True,
-    help="Enable collection of requirements in setup.py that compute these"
-    " dynamically. This is an insecure operation as it can run arbitrary code.",
+    help="Enable collection of requirements in setup.py that compute these "
+    "dynamically. This is an insecure operation as it can run arbitrary code.",
 )
 @click.option(
     "--prefer-source",
     is_flag=True,
-    help="Prefer source distributions over binary distributions"
-    " if no source distribution is available then binary distributions are used",
+    help="Prefer source distributions over binary distributions if no source "
+    "distribution is available then binary distributions are used",
 )
 @click.option(
     "--verbose",
     is_flag=True,
-    hidden=True,
-    help="Enable debug output.",
+    help="Enable verbose debug output.",
 )
 @click.option(
     "-V",
@@ -173,6 +174,13 @@ def print_version(ctx, param, value):
     help="Show the version and exit.",
 )
 @click.help_option("-h", "--help")
+@click.option(
+    "--generic-paths",
+    is_flag=True,
+    hidden=True,
+    help="Use generic or truncated paths in the JSON output header and files sections. "
+    "Used only for testing to avoid absolute paths and paths changing at each run.",
+)
 def resolve_dependencies(
     ctx,
     requirement_files,
@@ -190,6 +198,7 @@ def resolve_dependencies(
     analyze_setup_py_insecurely=False,
     prefer_source=False,
     verbose=TRACE,
+    generic_paths=False,
 ):
     """
     Resolve the dependencies for the package requirements listed in one or
@@ -212,6 +221,8 @@ def resolve_dependencies(
 
         python-inspector --spec "flask==2.1.2" --json -
     """
+    from python_inspector.api import resolve_dependencies as resolver_api
+
     if not (json_output or pdt_output):
         click.secho("No output file specified. Use --json or --json-pdt.", err=True)
         ctx.exit(1)
@@ -220,12 +231,7 @@ def resolve_dependencies(
         click.secho("Only one of --json or --json-pdt can be used.", err=True)
         ctx.exit(1)
 
-    options = [f"--requirement {rf}" for rf in requirement_files]
-    options += [f"--specifier {sp}" for sp in specifiers]
-    options += [f"--index-url {iu}" for iu in index_urls]
-    options += [f"--python-version {python_version}"]
-    options += [f"--operating-system {operating_system}"]
-    options += ["--json <file>"]
+    options = get_pretty_options(ctx, generic_paths=generic_paths)
 
     notice = (
         "Dependency tree generated with python-inspector.\n"
@@ -260,23 +266,133 @@ def resolve_dependencies(
             analyze_setup_py_insecurely=analyze_setup_py_insecurely,
             printer=click.secho,
             prefer_source=prefer_source,
+            generic_paths=generic_paths,
         )
+
+        files = resolution_result.files or []
         output = dict(
             headers=headers,
-            files=resolution_result.files,
+            files=files,
             packages=resolution_result.packages,
             resolved_dependencies_graph=resolution_result.resolution,
         )
         write_output_in_file(
             output=output,
             location=json_output or pdt_output,
         )
-    except Exception as exc:
+    except Exception:
         import traceback
 
         click.secho(traceback.format_exc(), err=True)
         ctx.exit(1)
 
 
+def get_pretty_options(ctx, generic_paths=False):
+    """
+    Return a sorted list of formatted strings for the selected CLI options of
+    the `ctx` Click.context, putting arguments first then options:
+
+        ["~/some/path", "--license", ...]
+
+    Skip options that are hidden or flags that are not set.
+    If ``generic_paths`` is True, click.File and click.Path parameters are made
+    "generic" replacing their value with a placeholder. This is used mostly for
+    testing.
+    """
+
+    args = []
+    options = []
+
+    param_values = ctx.params
+    for param in ctx.command.params:
+        name = param.name
+        value = param_values.get(name)
+
+        if param.is_eager:
+            continue
+
+        if getattr(param, "hidden", False):
+            continue
+
+        if value == param.default:
+            continue
+
+        if value in (None, False):
+            continue
+
+        if value in (tuple(), []):
+            # option with multiple values, the value is a emoty tuple
+            continue
+
+        # opts is a list of CLI options as in "--verbose": the last opt is
+        # the CLI option long form by convention
+        cli_opt = param.opts[-1]
+
+        if not isinstance(value, (tuple, list)):
+            value = [value]
+
+        for val in value:
+            val = get_pretty_value(param_type=param.type, value=val, generic_paths=generic_paths)
+
+            if isinstance(param, click.Argument):
+                args.append(val)
+            else:
+                # an option
+                if val is True:
+                    # mere flag... do not add the "true" value
+                    options.append(f"{cli_opt}")
+                else:
+                    options.append(f"{cli_opt} {val}")
+
+    return sorted(args) + sorted(options)
+
+
+def get_pretty_value(param_type, value, generic_paths=False):
+    """
+    Return pretty formatted string extracted from a parameter ``value``.
+    Make paths generic (by using a placeholder or truncating the path) if
+    ``generic_paths`` is True.
+    """
+    if isinstance(param_type, (click.Path, click.File)):
+        return get_pretty_path(param_type, value, generic_paths)
+
+    elif not (value is None or isinstance(value, (str, bytes, tuple, list, dict, bool))):
+        # coerce to string for non-basic types
+        return repr(value)
+
+    else:
+        return value
+
+
+def get_pretty_path(param_type, value, generic_paths=False):
+    """
+    Return a pretty path value for a Path or File option. Truncate the path or
+    use a placeholder as needed if ``generic_paths`` is True. Used for testing.
+    """
+    from python_inspector.utils import remove_test_data_dir_variable_prefix
+
+    if value == "-":
+        return value
+
+    if isinstance(param_type, click.Path):
+        if generic_paths:
+            return remove_test_data_dir_variable_prefix(path=value)
+        return value
+
+    elif isinstance(param_type, click.File):
+        # the value cannot be displayed as-is as this may be an opened file-
+        # like object
+        vname = getattr(value, "name", None)
+        if not vname:
+            return "<file>"
+        else:
+            value = vname
+
+        if generic_paths:
+            return remove_test_data_dir_variable_prefix(path=value, placeholder="<file>")
+
+    return value
+
+
 if __name__ == "__main__":
     resolve_dependencies()

src/python_inspector/utils.py

@@ -11,7 +11,6 @@
 
 import json
 import os
-import tempfile
 from typing import Dict
 from typing import List
 from typing import NamedTuple
@@ -73,3 +72,15 @@ def get_response(url: str) -> Dict:
     resp = requests.get(url)
     if resp.status_code == 200:
         return resp.json()
+
+
+def remove_test_data_dir_variable_prefix(path, placeholder="<file>"):
+    """
+    Return a clean path, removing variable test path prefix or using a ``placeholder``.
+    Used for testing to ensure that results are stable across runs.
+    """
+    if "tests/data/" in path:
+        _junk, test_dir, cleaned = path.partition("tests/data/")
+        return f"{test_dir}{cleaned}"
+    else:
+        return placeholder