Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

#206: Preserve environment markers from requirements.in #459

Closed
wants to merge 8 commits into from
Closed

#206: Preserve environment markers from requirements.in #459

wants to merge 8 commits into from

Conversation

barrywhart
Copy link
Contributor

@barrywhart barrywhart commented Feb 28, 2017

  • piptools/scripts/compile.py
    • When calling OutputWriter.write(), pass along the markers from requirements.in
  • piptools/util.py
    • format_requirement() -- add support for optional markers
  • piptools/writer.py
    • Add marker parameter to OutputWriter.write(), pass it to piptools.util.format_requirement()
  • tests/test_writer.py
    • Test for the new behavior

@barrywhart
Copy link
Contributor Author

For some reason, I can't view the Travis build log. The tests are passing locally for me. Can someone tell me what the Travis build error was?

@adamchainz
Copy link
Contributor

From 2.7:

=================================== FAILURES ===================================
__________________ test_format_requirement_environment_marker __________________
from_line = <bound method type.from_line of <class 'pip.req.req_install.InstallRequirement'>>
writer = <piptools.writer.OutputWriter object at 0x7fcd08690850>
    def test_format_requirement_environment_marker(from_line, writer):
        "Primary packages should not get annotated."
        ireq = from_line("test ; python_version=='2.7' and platform_python_implementation == 'CPython'")
        reverse_dependencies = set()
    
>       assert (writer._format_requirement(ireq,
                                           reverse_dependencies,
                                           primary_packages=['test'],
                                           marker=ireq.markers) ==
                "test ; python_version=='2.7' and platform_python_implementation == 'CPython'")
E       assert 'test ; pytho... == "CPython"' == "test ; python... == 'CPython'"
E         - test ; python_version == "2.7" and platform_python_implementation == "CPython"
E         ?                      -  ^^   ^                                       ^       ^
E         + test ; python_version=='2.7' and platform_python_implementation == 'CPython'
E         ?                        ^   ^                                       ^       ^
tests/test_writer.py:65: AssertionError
===================== 1 failed, 52 passed in 1.02 seconds ======================

@adamchainz
Copy link
Contributor

3.5 errored and seemed to produce no logs, if you push a new commit it'll restart the tests and it might succeed

@barrywhart
Copy link
Contributor Author

Thanks, I'm seeing build results now. Fixed a test that was failing on Python 3.x and fixed a style checker complaint about lines that are too long.

@davidovich
Copy link
Contributor

@barrywhart can you squash your commits ?

I think we can add this to the next feature release (1.9).

result = writer._format_requirement(
ireq, reverse_dependencies, primary_packages=['test'],
marker=ireq.markers)
result = result.replace('\"', '\'')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't get why you're modifying the result before asserting on it, why not just assert on the original value of result?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was seeing different behavior in Python 2.7 vs 3.5. Python 2.7 preserved quotes in the original format, while Python 3.5 was converting to double quotes. I assume this is a difference in the underlying pip library which parses the lines (including environment markers) from requirements.in.

@barrywhart
Copy link
Contributor Author

Closing this PR. I had already done a "git pull" without "--rebase", which made it hard to squash commits. Here is a new PR with the commits squashed: #460.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants