Fix allow unsafe false pinning bug v2

[dschaller]

Fix a bug introduced in #441 that pinned unsafe dependencies in generated requirements files when --allow-unsafe was False.

I will add the changes to the CHANGELOG if we agree this is the correct approach.

Contributor checklist

• Provided the tests for the changes
• Added the changes to CHANGELOG.md
• Requested (or received) a review from another contributor

[dschaller]

Ran a few things to test these changes:
Setup:

mktmpenv
pip install pip-tools
mkdir pkg && cd pkg
cat <<EOF > setup.py
from setuptools import setup
setup(install_requires=['setuptools'])
EOF
cd ..
echo "-e pkg" | pip-compile -n -o r.txt -

Run:

 > echo "-e pkg" | pip-compile -n -o r.txt - --allow-unsafe
#
# This file is autogenerated by pip-compile
# To update, run:
#
#    pip-compile --output-file r.txt -
#
-e file:///Users/dschaller/Desktop/testing/pkg
appdirs==1.4.3            # via setuptools
packaging==16.8           # via setuptools
pyparsing==2.2.0          # via packaging
six==1.10.0               # via packaging, setuptools

# The following packages are considered to be unsafe in a requirements file:
setuptools==35.0.2

 > echo "-e pkg" | pip-compile -n -o r.txt -
#
# This file is autogenerated by pip-compile
# To update, run:
#
#    pip-compile --output-file r.txt -
#
-e file:///Users/dschaller/Desktop/testing/pkg

Setup:

requests==2.0.0
setuptools==35.0.2
pip==8.0.2

Run:

> pip-compile --allow-unsafe
#
# This file is autogenerated by pip-compile
# To update, run:
#
#    pip-compile --output-file requirements.txt requirements.in
#
appdirs==1.4.3            # via setuptools
packaging==16.8           # via setuptools
pyparsing==2.2.0          # via packaging
requests==2.0.0
six==1.10.0               # via packaging, setuptools

# The following packages are considered to be unsafe in a requirements file:
pip==8.0.2
setuptools==35.0.2

 > pip-compile
#
# This file is autogenerated by pip-compile
# To update, run:
#
#    pip-compile --output-file requirements.txt requirements.in
#
requests==2.0.0

# The following packages are considered to be unsafe in a requirements file:
# pip
# setuptools

[dschaller]

@davidovich @vphilippon @jdufresne please take a look at this approach. It fixes the bug and maintains existing functionality.

[davidovich]

@dschaller I really appreciate the style of this new attempt, but I am still very not convinced that pip-tools should go down this route (even if a past PR made it in, that does not mean that was a wanted direction).

Aside from a better implementation (IMHO), the main question resides from the last PR, why do you absolutely need to put a known unsafe in your requirements.in and not want it in the compiled requirements? Why just not write it in the first place ?

[dschaller]

@davidovich there were several comments on the last pull request that outlined why this is a valid use case. The entire point of the flag is to pin these dependencies as noted in the help output. It does not say that this is only for transitive dependencies.

Also, if this functionality is no longer supported you should have released a major version change following semantic guidelines since this is a backward incompatible change in behavior.

[ryan-lane]

@davidovich as far as I can tell the behavior that @dschaller is putting back in was the original intended behavior. A change came in that broke the intended behavior and changed it to match what you'd prefer. A change in behavior, even if it is better in your view, is not great as it's a breaking change for every user who was relying on the old behavior.

We should make sure the current behavior matches the old behavior, then we should look into how we can make everyone happy. Right now there's a regression and the right thing to do is fix the regression.

[jdufresne]

there were several comments on the last pull request that outlined why this is a valid use case. The entire point of the flag is to pin these dependencies as noted in the help output. It does not say that this is only for transitive dependencies.

IMO, the use cases were stated but never adequately explained. They mostly boiled down to "we do this" without explaining the why. I think if someone that uses this feature explains why this is critical to their development workflow, it would be easier to understand how best to review these PRs in order to help everyone.

[dschaller]

Having unsafe dependencies in your requirements source allows for users to create generated requirements files that can be used in different environments where pining these packages is either wanted or not.
e.g.

> echo requirements.in
requests==2.0.0
setuptools==35.0.2
pip==8.0.2
 > pip-compile requirements.in -o a-requirements.txt
#
# This file is autogenerated by pip-compile
# To update, run:
#
#    pip-compile --output-file a-requirements.txt requirements.in
#
requests==2.0.0

# The following packages are considered to be unsafe in a requirements file:
# pip
# setuptools
> pip-compile requirements.in -o b-requirements.txt --allow-unsafe
#
# This file is autogenerated by pip-compile
# To update, run:
#
#    pip-compile --output-file b-requirements.txt requirements.in
#
appdirs==1.4.3            # via setuptools
packaging==16.8           # via setuptools
pyparsing==2.2.0          # via packaging
requests==2.0.0
six==1.10.0               # via packaging, setuptools

# The following packages are considered to be unsafe in a requirements file:
pip==8.0.2
setuptools==35.0.2
Environment A:
> pip install a-requirements.txt
Environment B:
> pip install b-requirements.txt

This allows for a single source of truth on what requirements are needed for a project while also not forcing people to consume what are considered unsafe packages when it doesn't matter if these are pinned.

If you look at the comments on the original PR there are at least two other people who had use-cases that required this flag as well.

[vphilippon]

LGTM.

This was an unexpected change, breaks the previous behaviour, and we have a report of it as a bug by a user, who sumbmited a PR to fix it. This should go through IMO.

We can discuss what changes we want in a seperate PR, and bring a 2.0 if we want to do so (after releasing at least one version with this fix, of course).