Repository: jazzband/pip-tools (python)

Issue:
Nomalize 'To update, run' advice
Currently when you regenerate a requirements.txt, if you run with syntactically different, but semantically identical command line flags the 'To update, run' advice changes.

The 'To update, run' advice should be normalized by:

* expanding short to long,
* removing values that are already default,
* sorting the arguments
* removing arguments that don't change build behaviour like 'verbose'

Pull request:
Normalize «command to run» in pip-compile headers.
Normalizes `To update, run:` header in `pip-compile`. To normalize it:

- expands short options to long
- removes values that are already default
- sorts the arguments alphabetically
- sorts variadic arguments (`src files`) alphabetically and puts them to the tail of the command
- removes arguments that don't change build behaviour like `--verbose` or `--quiet`
- removes one-off options like `--rebuild` or `--upgrade`

Closes #788, #799.

Also fixed an error when `requirements.txt` has unicode chars.

**Changelog-friendly one-liner**: Normalize «command to run» in `pip-compile` headers.

##### Contributor checklist

- [X] Provided the tests for the changes.
- [x] Requested a review from another contributor.
- [X] Gave a clear one-line description in the PR (that the maintainers can add to CHANGELOG.md on release).
- [ ] Assign the PR to an existing or new milestone for the target version (following [Semantic Versioning](https://blog.versioneye.com/2014/01/16/semantic-versioning/)).


Patch:
diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
index bbd401a..ddb2404 100644
--- a/.pre-commit-config.yaml
+++ b/.pre-commit-config.yaml
@@ -1,5 +1,5 @@
 repos:
-  - repo: https://github.com/python/black
+  - repo: https://github.com/ambv/black
     rev: 19.3b0
     hooks:
     - id: black
diff --git a/piptools/__init__.py b/piptools/__init__.py
index e69de29..9f0c95a 100644
--- a/piptools/__init__.py
+++ b/piptools/__init__.py
@@ -0,0 +1,11 @@
+import locale
+
+from piptools.click import secho
+
+# Needed for locale.getpreferredencoding(False) to work
+# in pip._internal.utils.encoding.auto_decode
+try:
+    locale.setlocale(locale.LC_ALL, "")
+except locale.Error as e:  # pragma: no cover
+    # setlocale can apparently crash if locale are uninitialized
+    secho("Ignoring error when setting locale: {}".format(e), fg="red")
diff --git a/piptools/scripts/compile.py b/piptools/scripts/compile.py
index b9e12fb..896de7a 100755
--- a/piptools/scripts/compile.py
+++ b/piptools/scripts/compile.py
@@ -394,6 +394,7 @@ def cli(
     writer = OutputWriter(
         src_files,
         output_file,
+        click_ctx=ctx,
         dry_run=dry_run,
         emit_header=header,
         emit_index=index,
diff --git a/piptools/utils.py b/piptools/utils.py
index 085505d..ad01fe0 100644
--- a/piptools/utils.py
+++ b/piptools/utils.py
@@ -5,10 +5,21 @@ import sys
 from collections import OrderedDict
 from itertools import chain, groupby
 
+import six
+from six.moves import shlex_quote
+
 from ._compat import install_req_from_line
 from .click import style
 
 UNSAFE_PACKAGES = {"setuptools", "distribute", "pip"}
+COMPILE_EXCLUDE_OPTIONS = {
+    "--dry-run",
+    "--quiet",
+    "--rebuild",
+    "--upgrade",
+    "--upgrade-package",
+    "--verbose",
+}
 
 
 def key_from_ireq(ireq):
@@ -260,3 +271,93 @@ def get_hashes_from_ireq(ireq):
         for hash_ in hexdigests:
             result.append("{}:{}".format(algorithm, hash_))
     return result
+
+
+def force_text(s):
+    """
+    Return a string representing `s`.
+    """
+    if s is None:
+        return ""
+    if not isinstance(s, six.string_types):
+        return six.text_type(s)
+    return s
+
+
+def get_compile_command(click_ctx):
+    """
+    Returns a normalized compile command depending on cli context.
+
+    The command will be normalized by:
+        - expanding options short to long
+        - removing values that are already default
+        - sorting the arguments
+        - removing one-off arguments like '--upgrade'
+        - removing arguments that don't change build behaviour like '--verbose'
+    """
+    from piptools.scripts.compile import cli
+
+    # Map of the compile cli options (option name -> click.Option)
+    compile_options = {option.name: option for option in cli.params}
+
+    left_args = []
+    right_args = []
+
+    for option_name, value in click_ctx.params.items():
+        option = compile_options[option_name]
+
+        # Get the latest option name (usually it'll be a long name)
+        option_long_name = option.opts[-1]
+
+        # Collect variadic args separately, they will be added
+        # at the end of the command later
+        if option.nargs < 0:
+            right_args.extend([shlex_quote(force_text(val)) for val in value])
+            continue
+
+        # Exclude one-off options (--upgrade/--upgrade-package/--rebuild/...)
+        # or options that don't change compile behaviour (--verbose/--dry-run/...)
+        if option_long_name in COMPILE_EXCLUDE_OPTIONS:
+            continue
+
+        # Skip options without a value
+        if option.default is None and not value:
+            continue
+
+        # Skip options with a default value
+        if option.default == value:
+            continue
+
+        # Use a file name for file-like objects
+        if (
+            hasattr(value, "write")
+            and hasattr(value, "read")
+            and hasattr(value, "name")
+        ):
+            value = value.name
+
+        # Convert value to the list
+        if not isinstance(value, (tuple, list)):
+            value = [value]
+
+        for val in value:
+            # Flags don't have a value, thus add to args true or false option long name
+            if option.is_flag:
+                # If there are false-options, choose an option name depending on a value
+                if option.secondary_opts:
+                    # Get the latest false-option
+                    secondary_option_long_name = option.secondary_opts[-1]
+                    arg = option_long_name if val else secondary_option_long_name
+                # There are no false-options, use true-option
+                else:
+                    arg = option_long_name
+                left_args.append(shlex_quote(arg))
+            # Append to args the option with a value
+            else:
+                left_args.append(
+                    "{option}={value}".format(
+                        option=option_long_name, value=shlex_quote(force_text(val))
+                    )
+                )
+
+    return " ".join(["pip-compile"] + sorted(left_args) + sorted(right_args))
diff --git a/piptools/writer.py b/piptools/writer.py
index 91e7e59..18a715b 100644
--- a/piptools/writer.py
+++ b/piptools/writer.py
@@ -1,10 +1,18 @@
+from __future__ import unicode_literals
+
 import os
-import sys
 from itertools import chain
 
-from .click import get_os_args, unstyle
+from .click import unstyle
 from .logging import log
-from .utils import UNSAFE_PACKAGES, comment, dedup, format_requirement, key_from_req
+from .utils import (
+    UNSAFE_PACKAGES,
+    comment,
+    dedup,
+    format_requirement,
+    get_compile_command,
+    key_from_req,
+)
 
 
 class OutputWriter(object):
@@ -12,6 +20,7 @@ class OutputWriter(object):
         self,
         src_files,
         dst_file,
+        click_ctx,
         dry_run,
         emit_header,
         emit_index,
@@ -27,6 +36,7 @@ class OutputWriter(object):
     ):
         self.src_files = src_files
         self.dst_file = dst_file
+        self.click_ctx = click_ctx
         self.dry_run = dry_run
         self.emit_header = emit_header
         self.emit_index = emit_index
@@ -49,13 +59,10 @@ class OutputWriter(object):
             yield comment("# This file is autogenerated by pip-compile")
             yield comment("# To update, run:")
             yield comment("#")
-            custom_cmd = os.environ.get("CUSTOM_COMPILE_COMMAND")
-            if custom_cmd:
-                yield comment("#    {}".format(custom_cmd))
-            else:
-                prog = os.path.basename(sys.argv[0])
-                args = " ".join(get_os_args())
-                yield comment("#    {prog} {args}".format(prog=prog, args=args))
+            compile_command = os.environ.get(
+                "CUSTOM_COMPILE_COMMAND"
+            ) or get_compile_command(self.click_ctx)
+            yield comment("#    {}".format(compile_command))
             yield comment("#")
 
     def write_index_options(self):


Test patch:
diff --git a/tests/conftest.py b/tests/conftest.py
index 8c81307..e4d11bf 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -1,7 +1,6 @@
 import json
 from contextlib import contextmanager
 from functools import partial
-from tempfile import NamedTemporaryFile
 
 from click.testing import CliRunner
 from pip._vendor.packaging.version import Version
@@ -139,6 +138,6 @@ def runner():
 
 
 @fixture
-def tmp_file():
-    with NamedTemporaryFile("wt") as fp:
-        yield fp
+def tmpdir_cwd(tmpdir):
+    with tmpdir.as_cwd():
+        yield tmpdir
diff --git a/tests/test_utils.py b/tests/test_utils.py
index 584e836..3bd1281 100644
--- a/tests/test_utils.py
+++ b/tests/test_utils.py
@@ -1,13 +1,22 @@
+# coding: utf-8
+from __future__ import unicode_literals
+
+import os
+
 import six
 from pytest import mark, raises
+from six.moves import shlex_quote
 
+from piptools.scripts.compile import cli as compile_cli
 from piptools.utils import (
     as_tuple,
     dedup,
     flat_map,
+    force_text,
     format_requirement,
     format_specifier,
     fs_str,
+    get_compile_command,
     get_hashes_from_ireq,
     is_pinned_requirement,
     name_from_req,
@@ -164,12 +173,128 @@ def test_name_from_req_with_project_name(from_line):
 
 
 def test_fs_str():
-    assert fs_str(u"some path component/Something") == "some path component/Something"
+    assert fs_str("some path component/Something") == "some path component/Something"
     assert isinstance(fs_str("whatever"), str)
-    assert isinstance(fs_str(u"whatever"), str)
 
 
 @mark.skipif(six.PY2, reason="Not supported in py2")
 def test_fs_str_with_bytes():
     with raises(AssertionError):
         fs_str(b"whatever")
+
+
+@mark.parametrize(
+    "value, expected_text", [(None, ""), (42, "42"), ("foo", "foo"), ("bãr", "bãr")]
+)
+def test_force_text(value, expected_text):
+    assert force_text(value) == expected_text
+
+
+@mark.parametrize(
+    "cli_args, expected_command",
+    [
+        # Check empty args
+        ([], "pip-compile"),
+        # Check all options which will be excluded from command
+        (["-v"], "pip-compile"),
+        (["--verbose"], "pip-compile"),
+        (["-n"], "pip-compile"),
+        (["--dry-run"], "pip-compile"),
+        (["-q"], "pip-compile"),
+        (["--quiet"], "pip-compile"),
+        (["-r"], "pip-compile"),
+        (["--rebuild"], "pip-compile"),
+        (["-U"], "pip-compile"),
+        (["--upgrade"], "pip-compile"),
+        (["-P", "django"], "pip-compile"),
+        (["--upgrade-package", "django"], "pip-compile"),
+        # Check options
+        (["--max-rounds", "42"], "pip-compile --max-rounds=42"),
+        (["--index-url", "https://foo"], "pip-compile --index-url=https://foo"),
+        # Check that short options will be expanded to long options
+        (["-p"], "pip-compile --pre"),
+        (["-f", "links"], "pip-compile --find-links=links"),
+        (["-i", "https://foo"], "pip-compile --index-url=https://foo"),
+        # Check positive flags
+        (["--generate-hashes"], "pip-compile --generate-hashes"),
+        (["--pre"], "pip-compile --pre"),
+        (["--allow-unsafe"], "pip-compile --allow-unsafe"),
+        # Check negative flags
+        (["--no-index"], "pip-compile --no-index"),
+        (["--no-emit-trusted-host"], "pip-compile --no-emit-trusted-host"),
+        (["--no-annotate"], "pip-compile --no-annotate"),
+        # Check that default values will be removed from the command
+        (["--emit-trusted-host"], "pip-compile"),
+        (["--annotate"], "pip-compile"),
+        (["--index"], "pip-compile"),
+        (["--max-rounds=10"], "pip-compile"),
+        (["--no-build-isolation"], "pip-compile"),
+        # Check options with multiple values
+        (
+            ["--find-links", "links1", "--find-links", "links2"],
+            "pip-compile --find-links=links1 --find-links=links2",
+        ),
+        # Check that option values will be quoted
+        (["-f", "foo;bar"], "pip-compile --find-links='foo;bar'"),
+        (["-f", "συνδέσεις"], "pip-compile --find-links='συνδέσεις'"),
+        (["-o", "my file.txt"], "pip-compile --output-file='my file.txt'"),
+        (["-o", "απαιτήσεις.txt"], "pip-compile --output-file='απαιτήσεις.txt'"),
+    ],
+)
+def test_get_compile_command(tmpdir_cwd, cli_args, expected_command):
+    """
+    Test general scenarios for the get_compile_command function.
+    """
+    with compile_cli.make_context("pip-compile", cli_args) as ctx:
+        assert get_compile_command(ctx) == expected_command
+
+
+@mark.parametrize(
+    "filename", ["requirements.in", "my requirements.in", "απαιτήσεις.txt"]
+)
+def test_get_compile_command_with_files(tmpdir_cwd, filename):
+    """
+    Test that get_compile_command returns a command with correct
+    and sanitized file names.
+    """
+    os.mkdir("sub")
+
+    path = os.path.join("sub", filename)
+    with open(path, "w"):
+        pass
+
+    args = [path, "--output-file", "requirements.txt"]
+    with compile_cli.make_context("pip-compile", args) as ctx:
+        assert get_compile_command(
+            ctx
+        ) == "pip-compile --output-file=requirements.txt {src_file}".format(
+            src_file=shlex_quote(path)
+        )
+
+
+def test_get_compile_command_sort_args(tmpdir_cwd):
+    """
+    Test that get_compile_command correctly sorts arguments.
+
+    The order is "pip-compile {sorted options} {sorted src files}".
+    """
+    with open("setup.py", "w"), open("requirements.in", "w"):
+        pass
+
+    args = [
+        "--no-index",
+        "--no-emit-trusted-host",
+        "--no-annotate",
+        "setup.py",
+        "--find-links",
+        "foo",
+        "--find-links",
+        "bar",
+        "requirements.in",
+    ]
+    with compile_cli.make_context("pip-compile", args) as ctx:
+        assert get_compile_command(ctx) == (
+            "pip-compile --find-links=bar --find-links=foo "
+            "--no-annotate --no-emit-trusted-host --no-index "
+            "requirements.in setup.py"
+        )
diff --git a/tests/test_writer.py b/tests/test_writer.py
index 92596a5..b255c11 100644
--- a/tests/test_writer.py
+++ b/tests/test_writer.py
@@ -1,28 +1,43 @@
 from pytest import fixture, mark, raises
 
 from piptools._compat import FormatControl
+from piptools.scripts.compile import cli
 from piptools.utils import comment
 from piptools.writer import OutputWriter
 
 
 @fixture
-def writer(tmp_file):
-    return OutputWriter(
-        src_files=["src_file", "src_file2"],
-        dst_file=tmp_file,
-        dry_run=True,
-        emit_header=True,
-        emit_index=True,
-        emit_trusted_host=True,
-        annotate=True,
-        generate_hashes=False,
-        default_index_url=None,
-        index_urls=[],
-        trusted_hosts=[],
-        format_control=FormatControl(set(), set()),
-        allow_unsafe=False,
-        find_links=[],
-    )
+def writer(tmpdir_cwd):
+    with open("src_file", "w"), open("src_file2", "w"):
+        pass
+
+    cli_args = [
+        "--dry-run",
+        "--output-file",
+        "requirements.txt",
+        "src_file",
+        "src_file2",
+    ]
+
+    with cli.make_context("pip-compile", cli_args) as ctx:
+        writer = OutputWriter(
+            src_files=["src_file", "src_file2"],
+            dst_file=ctx.params["output_file"],
+            click_ctx=ctx,
+            dry_run=True,
+            emit_header=True,
+            emit_index=True,
+            emit_trusted_host=True,
+            annotate=True,
+            generate_hashes=False,
+            default_index_url=None,
+            index_urls=[],
+            trusted_hosts=[],
+            format_control=FormatControl(set(), set()),
+            allow_unsafe=False,
+            find_links=[],
+        )
+        yield writer
 
 
 def test_format_requirement_annotation_editable(from_editable, writer):
@@ -120,11 +135,7 @@ def test_iter_lines__unsafe_dependencies(writer, from_line, allow_unsafe):
     assert "test==1.2" in str_lines
 
 
-def test_write_header(writer, monkeypatch):
-    monkeypatch.setattr(
-        "sys.argv",
-        ["pip-compile", "--output-file", "dst_file", "src_file", "src_file2"],
-    )
+def test_write_header(writer):
     expected = map(
         comment,
         [
@@ -132,7 +143,9 @@ def test_write_header(writer, monkeypatch):
             "# This file is autogenerated by pip-compile",
             "# To update, run:",
             "#",
-            "#    pip-compile --output-file dst_file src_file src_file2",
+            "#    pip-compile --output-file={} src_file src_file2".format(
+                writer.click_ctx.params["output_file"].name
+            ),
             "#",
         ],
     )