Repository: jazzband/pip-tools
Pull request #814 (2019-05-12 23:20:57)

Warn when --generate-hashes output is uninstallable
`pip-compile --generate-hashes` can generate a `requirements.txt` file that will be rejected by `pip install`, in at least two ways:

(1) When unsafe packages are left out (#806):

```
$ pip-compile --generate-hashes
#
# This file is autogenerated by pip-compile
# To update, run:
#
#    pip-compile --generate-hashes
#
markdown==3.1 \
    --hash=sha256:fc4a6f69a656b8d858d7503bda633f4dd63c2d70cf80abdc6eafa64c4ae8c250 \
    --hash=sha256:fe463ff51e679377e3624984c829022e2cfb3be5518726b06f608a07a3aad680
$ pip install -r requirements.txt
Collecting markdown==3.1 (from -r requirements.txt (line 7))
  Using cached https://files.pythonhosted.org/packages/f5/e4/d8c18f2555add57ff21bf25af36d827145896a07607486cc79a2aea641af/Markdown-3.1-py2.py3-none-any.whl
Collecting setuptools>=36 (from markdown==3.1->-r requirements.txt (line 7))
ERROR: In --require-hashes mode, all requirements must have their versions pinned with ==. These do not:
    setuptools>=36 from https://files.pythonhosted.org/packages/ec/51/f45cea425fd5cb0b0380f5b0f048ebc1da5b417e48d304838c02d6288a1e/setuptools-41.0.1-py2.py3-none-any.whl#sha256=c7769ce668c7a333d84e17fe8b524b1c45e7ee9f7908ad0a73e1eda7e6a5aebf (from markdown==3.1->-r requirements.txt (line 7))
```

(2) When unhashable URL requirements are included:

```
$ pip-compile --generate-hashes
#
# This file is autogenerated by pip-compile
# To update, run:
#
#    pip-compile --generate-hashes
#
-e git+https://github.com/jazzband/pip-tools@7d86c8d3ecd1faa6be11c7ddc6b29a30ffd1dae3#egg=pip-tools
click==7.0 \
    --hash=sha256:2335065e6395b9e67ca716de5f7526736bfa6ceead690adf616d925bdc622b13 \
    --hash=sha256:5b94b49521f6456670fdb30cd82a4eca9412788a93fa6dd6df72c94d5a8ff2d7
first==2.0.2 \
    --hash=sha256:8d8e46e115ea8ac652c76123c0865e3ff18372aef6f03c22809ceefcea9dec86 \
    --hash=sha256:ff285b08c55f8c97ce4ea7012743af2495c9f1291785f163722bd36f6af6d3bf
six==1.12.0 \
    --hash=sha256:3350809f0555b11f552448330d0b52d5f24c91a322ea4a15ef22629740f3761c \
    --hash=sha256:d16a0141ec1a18405cd4ce8b4613101da75da0e9a7aec5bdd4fa804d0e0eba73
$ pip install -r requirements.txt
Obtaining pip-tools from git+https://github.com/jazzband/pip-tools@7d86c8d3ecd1faa6be11c7ddc6b29a30ffd1dae3#egg=pip-tools (from -r requirements.txt (line 7))
ERROR: The editable requirement pip-tools from git+https://github.com/jazzband/pip-tools@7d86c8d3ecd1faa6be11c7ddc6b29a30ffd1dae3#egg=pip-tools (from -r requirements.txt (line 7)) cannot be installed when requiring hashes, because there is no single file to hash.
```

This PR prints warning messages in the generated requirements.txt file for both of those situations, and also prints a warning message to stderr at the end:

```
$ pip-compile --generate-hashes
#
# This file is autogenerated by pip-compile
# To update, run:
#
#    pip-compile --generate-hashes
#
# WARNING: pip install will require the following package to be hashed.
# Consider using a hashable URL like https://github.com/jazzband/pip-tools/archive/SOMECOMMIT.zip
-e git+https://github.com/jazzband/pip-tools@7d86c8d3ecd1faa6be11c7ddc6b29a30ffd1dae3#egg=pip-tools
click==7.0 \
    --hash=sha256:2335065e6395b9e67ca716de5f7526736bfa6ceead690adf616d925bdc622b13 \
    --hash=sha256:5b94b49521f6456670fdb30cd82a4eca9412788a93fa6dd6df72c94d5a8ff2d7
first==2.0.2 \
    --hash=sha256:8d8e46e115ea8ac652c76123c0865e3ff18372aef6f03c22809ceefcea9dec86 \
    --hash=sha256:ff285b08c55f8c97ce4ea7012743af2495c9f1291785f163722bd36f6af6d3bf
markdown==3.1 \
    --hash=sha256:fc4a6f69a656b8d858d7503bda633f4dd63c2d70cf80abdc6eafa64c4ae8c250 \
    --hash=sha256:fe463ff51e679377e3624984c829022e2cfb3be5518726b06f608a07a3aad680
six==1.12.0 \
    --hash=sha256:3350809f0555b11f552448330d0b52d5f24c91a322ea4a15ef22629740f3761c \
    --hash=sha256:d16a0141ec1a18405cd4ce8b4613101da75da0e9a7aec5bdd4fa804d0e0eba73

# WARNING: The following packages were not pinned, but pip requires them to be
# pinned when the requirements file includes hashes. Consider using the --allow-unsafe flag.
# setuptools==41.0.1        # via markdown
[to stderr:]
The generated requirements file may be rejected by pip install. See # WARNING lines for details.
```

Note that the warning message text is static; this doesn't attempt to suggest a fixed URL for Github or similar VCS URLs, though that would be possible as an enhancement.

Here's an image of the output with color:

![image](https://user-images.githubusercontent.com/376272/57588920-f77ee000-74e9-11e9-902b-4972a285e236.png)

Tests are included for the two warning situations.

**Changelog-friendly one-liner**: Warn when pip-compile --generate-hashes output is uninstallable

##### 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/)).


codecov[bot]: # [Codecov](https://codecov.io/gh/jazzband/pip-tools/pull/814?src=pr&el=h1) Report
> Merging [#814](https://codecov.io/gh/jazzband/pip-tools/pull/814?src=pr&el=desc) into [master](https://codecov.io/gh/jazzband/pip-tools/commit/804d780f498fa6a0585cba24b3fd29c8c9e0e586?src=pr&el=desc) will **increase** coverage by `0.01%`.
> The diff coverage is `100%`.

[![Impacted file tree graph](https://codecov.io/gh/jazzband/pip-tools/pull/814/graphs/tree.svg?width=650&token=CELIrP9w8G&height=150&src=pr)](https://codecov.io/gh/jazzband/pip-tools/pull/814?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master     #814      +/-   ##
==========================================
+ Coverage   98.83%   98.84%   +0.01%     
==========================================
  Files          36       36              
  Lines        2153     2172      +19     
  Branches      276      279       +3     
==========================================
+ Hits         2128     2147      +19     
  Misses         15       15              
  Partials       10       10
```


| [Impacted Files](https://codecov.io/gh/jazzband/pip-tools/pull/814?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [tests/test\_writer.py](https://codecov.io/gh/jazzband/pip-tools/pull/814/diff?src=pr&el=tree#diff-dGVzdHMvdGVzdF93cml0ZXIucHk=) | `100% <100%> (ø)` | :arrow_up: |
| [piptools/writer.py](https://codecov.io/gh/jazzband/pip-tools/pull/814/diff?src=pr&el=tree#diff-cGlwdG9vbHMvd3JpdGVyLnB5) | `100% <100%> (ø)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/jazzband/pip-tools/pull/814?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/jazzband/pip-tools/pull/814?src=pr&el=footer). Last update [804d780...4b79946](https://codecov.io/gh/jazzband/pip-tools/pull/814?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

codecov[bot]: # [Codecov](https://codecov.io/gh/jazzband/pip-tools/pull/814?src=pr&el=h1) Report
> Merging [#814](https://codecov.io/gh/jazzband/pip-tools/pull/814?src=pr&el=desc) into [master](https://codecov.io/gh/jazzband/pip-tools/commit/804d780f498fa6a0585cba24b3fd29c8c9e0e586?src=pr&el=desc) will **increase** coverage by `0.01%`.
> The diff coverage is `100%`.

[![Impacted file tree graph](https://codecov.io/gh/jazzband/pip-tools/pull/814/graphs/tree.svg?width=650&token=CELIrP9w8G&height=150&src=pr)](https://codecov.io/gh/jazzband/pip-tools/pull/814?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master     #814      +/-   ##
==========================================
+ Coverage   98.83%   98.84%   +0.01%     
==========================================
  Files          36       36              
  Lines        2153     2172      +19     
  Branches      276      279       +3     
==========================================
+ Hits         2128     2147      +19     
  Misses         15       15              
  Partials       10       10
```


| [Impacted Files](https://codecov.io/gh/jazzband/pip-tools/pull/814?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [tests/test\_writer.py](https://codecov.io/gh/jazzband/pip-tools/pull/814/diff?src=pr&el=tree#diff-dGVzdHMvdGVzdF93cml0ZXIucHk=) | `100% <100%> (ø)` | :arrow_up: |
| [piptools/writer.py](https://codecov.io/gh/jazzband/pip-tools/pull/814/diff?src=pr&el=tree#diff-cGlwdG9vbHMvd3JpdGVyLnB5) | `100% <100%> (ø)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/jazzband/pip-tools/pull/814?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/jazzband/pip-tools/pull/814?src=pr&el=footer). Last update [804d780...4b79946](https://codecov.io/gh/jazzband/pip-tools/pull/814?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

codecov[bot]: # [Codecov](https://codecov.io/gh/jazzband/pip-tools/pull/814?src=pr&el=h1) Report
> Merging [#814](https://codecov.io/gh/jazzband/pip-tools/pull/814?src=pr&el=desc) into [master](https://codecov.io/gh/jazzband/pip-tools/commit/804d780f498fa6a0585cba24b3fd29c8c9e0e586?src=pr&el=desc) will **increase** coverage by `0.01%`.
> The diff coverage is `100%`.

[![Impacted file tree graph](https://codecov.io/gh/jazzband/pip-tools/pull/814/graphs/tree.svg?width=650&token=CELIrP9w8G&height=150&src=pr)](https://codecov.io/gh/jazzband/pip-tools/pull/814?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master     #814      +/-   ##
==========================================
+ Coverage   98.83%   98.84%   +0.01%     
==========================================
  Files          36       36              
  Lines        2153     2172      +19     
  Branches      276      279       +3     
==========================================
+ Hits         2128     2147      +19     
  Misses         15       15              
  Partials       10       10
```


| [Impacted Files](https://codecov.io/gh/jazzband/pip-tools/pull/814?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [tests/test\_writer.py](https://codecov.io/gh/jazzband/pip-tools/pull/814/diff?src=pr&el=tree#diff-dGVzdHMvdGVzdF93cml0ZXIucHk=) | `100% <100%> (ø)` | :arrow_up: |
| [piptools/writer.py](https://codecov.io/gh/jazzband/pip-tools/pull/814/diff?src=pr&el=tree#diff-cGlwdG9vbHMvd3JpdGVyLnB5) | `100% <100%> (ø)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/jazzband/pip-tools/pull/814?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/jazzband/pip-tools/pull/814?src=pr&el=footer). Last update [804d780...4b79946](https://codecov.io/gh/jazzband/pip-tools/pull/814?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
index c3b550f..bbd401a 100644
--- a/.pre-commit-config.yaml
+++ b/.pre-commit-config.yaml
@@ -14,9 +14,3 @@ repos:
     hooks:
     - id: flake8
       language_version: python3.7
-  - repo: https://github.com/PyCQA/bandit
-    rev: 1.6.0
-    hooks:
-    - id: bandit
-      language_version: python3.7
-      exclude: ^(tests|\.tox|\.eggs|\.venv|\.git)
diff --git a/README.rst b/README.rst
index 761d132..945b6b4 100644
--- a/README.rst
+++ b/README.rst
@@ -208,7 +208,7 @@ install/upgrade/uninstall everything necessary to match the
       Successfully uninstalled flake8-2.4.1
     Collecting click==4.1
       Downloading click-4.1-py2.py3-none-any.whl (62kB)
-        100% |................................| 65kB 1.8MB/s
+        100% |████████████████████████████████| 65kB 1.8MB/s
       Found existing installation: click 4.0
         Uninstalling click-4.0:
           Successfully uninstalled click-4.0
diff --git a/piptools/writer.py b/piptools/writer.py
index 9235551..a20e11b 100644
--- a/piptools/writer.py
+++ b/piptools/writer.py
@@ -14,6 +14,27 @@ from .utils import (
     key_from_ireq,
 )
 
+MESSAGE_UNHASHED_PACKAGE = comment(
+    "# WARNING: pip install will require the following package to be hashed."
+    "\n# Consider using a hashable URL like "
+    "https://github.com/jazzband/pip-tools/archive/SOMECOMMIT.zip"
+)
+
+MESSAGE_UNSAFE_PACKAGES_UNPINNED = comment(
+    "# WARNING: The following packages were not pinned, but pip requires them to be"
+    "\n# pinned when the requirements file includes hashes. "
+    "Consider using the --allow-unsafe flag."
+)
+
+MESSAGE_UNSAFE_PACKAGES = comment(
+    "# The following packages are considered to be unsafe in a requirements file:"
+)
+
+MESSAGE_UNINSTALLABLE = (
+    "The generated requirements file may be rejected by pip install. "
+    "See # WARNING lines for details."
+)
+
 
 class OutputWriter(object):
     def __init__(
@@ -104,12 +125,24 @@ class OutputWriter(object):
     def _iter_lines(
         self,
         results,
-        unsafe_requirements,
-        reverse_dependencies,
-        primary_packages,
-        markers,
-        hashes,
+        unsafe_requirements=None,
+        reverse_dependencies=None,
+        primary_packages=None,
+        markers=None,
+        hashes=None,
     ):
+        # default values
+        unsafe_requirements = unsafe_requirements or []
+        reverse_dependencies = reverse_dependencies or {}
+        primary_packages = primary_packages or []
+        markers = markers or {}
+        hashes = hashes or {}
+
+        # Check for unhashed or unpinned packages if at least one package does have
+        # hashes, which will trigger pip install's --require-hashes mode.
+        warn_uninstallable = False
+        has_hashes = hashes and any(hash for hash in hashes.values())
+
         for line in self.write_header():
             yield line
         for line in self.write_flags():
@@ -125,6 +158,9 @@ class OutputWriter(object):
         packages = sorted(packages, key=self._sort_key)
 
         for ireq in packages:
+            if has_hashes and not hashes.get(ireq):
+                yield MESSAGE_UNHASHED_PACKAGE
+                warn_uninstallable = True
             line = self._format_requirement(
                 ireq,
                 reverse_dependencies,
@@ -137,10 +173,11 @@ class OutputWriter(object):
         if unsafe_requirements:
             unsafe_requirements = sorted(unsafe_requirements, key=self._sort_key)
             yield ""
-            yield comment(
-                "# The following packages are considered "
-                "to be unsafe in a requirements file:"
-            )
+            if has_hashes and not self.allow_unsafe:
+                yield MESSAGE_UNSAFE_PACKAGES_UNPINNED
+                warn_uninstallable = True
+            else:
+                yield MESSAGE_UNSAFE_PACKAGES
 
             for ireq in unsafe_requirements:
                 req = self._format_requirement(
@@ -155,6 +192,9 @@ class OutputWriter(object):
                 else:
                     yield req
 
+        if warn_uninstallable:
+            log.warning(MESSAGE_UNINSTALLABLE)
+
     def write(
         self,
         results,
diff --git a/tox.ini b/tox.ini
index b843a23..d1270fc 100644
--- a/tox.ini
+++ b/tox.ini
@@ -35,9 +35,17 @@ commands = pytest {posargs}
 
 [testenv:checkqa]
 basepython = python3
-skip_install = True
-deps = pre-commit
-commands = pre-commit run --all-files --show-diff-on-failure
+ignore_errors = true
+deps =
+    flake8==3.7.7
+    black==19.3b0
+    isort==4.3.16
+    bandit==1.5.1
+commands =
+    black --check --diff .
+    isort --recursive --check-only --diff .
+    flake8 piptools tests
+    bandit -r .
 
 [testenv:readme]
 deps = twine


Test patch:
diff --git a/tests/test_writer.py b/tests/test_writer.py
index b255c11..40aac81 100644
--- a/tests/test_writer.py
+++ b/tests/test_writer.py
@@ -3,7 +3,12 @@ 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
+from piptools.writer import (
+    MESSAGE_UNHASHED_PACKAGE,
+    MESSAGE_UNSAFE_PACKAGES,
+    MESSAGE_UNSAFE_PACKAGES_UNPINNED,
+    OutputWriter,
+)
 
 
 @fixture
@@ -113,26 +118,52 @@ def test_format_requirement_environment_marker(from_line, writer):
 @mark.parametrize(("allow_unsafe",), [(True,), (False,)])
 def test_iter_lines__unsafe_dependencies(writer, from_line, allow_unsafe):
     writer.allow_unsafe = allow_unsafe
+    output = "\n".join(
+        writer._iter_lines([from_line("test==1.2")], [from_line("setuptools")])
+    )
+    assert (
+        "\n".join(
+            [
+                "test==1.2",
+                "",
+                MESSAGE_UNSAFE_PACKAGES,
+                "setuptools" if allow_unsafe else comment("# setuptools"),
+            ]
+        )
+        in output
+    )
 
-    ireq = [from_line("test==1.2")]
-    unsafe_req = [from_line("setuptools")]
-    reverse_dependencies = {"test": ["xyz"]}
 
-    str_lines = list(
-        writer._iter_lines(ireq, unsafe_req, reverse_dependencies, ["test"], {}, None)
+def test_iter_lines__unsafe_with_hashes(writer, from_line):
+    writer.allow_unsafe = False
+    ireqs = [from_line("test==1.2")]
+    unsafe_ireqs = [from_line("setuptools")]
+    hashes = {ireqs[0]: {"FAKEHASH"}, unsafe_ireqs[0]: set()}
+    output = "\n".join(writer._iter_lines(ireqs, unsafe_ireqs, hashes=hashes))
+    assert (
+        "\n".join(
+            [
+                "test==1.2 \\",
+                "    --hash=FAKEHASH",
+                "",
+                MESSAGE_UNSAFE_PACKAGES_UNPINNED,
+                comment("# setuptools"),
+            ]
+        )
+        in output
     )
+
+
+def test_iter_lines__hash_missing(writer, from_line):
+    ireqs = [from_line("test==1.2"), from_line("file:///example/#egg=example")]
+    hashes = {ireqs[0]: {"FAKEHASH"}, ireqs[1]: set()}
+    output = "\n".join(writer._iter_lines(ireqs, hashes=hashes))
     assert (
-        comment(
-            "# The following packages are considered "
-            "to be unsafe in a requirements file:"
+        "\n".join(
+            [MESSAGE_UNHASHED_PACKAGE, "file:///example/#egg=example", "test==1.2"]
         )
-        in str_lines
+        in output
     )
-    if allow_unsafe:
-        assert "setuptools" in str_lines
-    else:
-        assert comment("# setuptools") in str_lines
-    assert "test==1.2" in str_lines
 
 
 def test_write_header(writer):