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

Parse environment markers from setup.py install_requires #1042

Closed
wants to merge 2 commits into from

Conversation

iamhatesz
Copy link

@iamhatesz iamhatesz commented Jan 24, 2020

Changelog-friendly one-liner: Added scanning of extras_require in setup.py for additional dependencies which might come from using environment markers in install_requires.

Contributor checklist
  • Provided the tests for the changes.
  • 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).

@codecov
Copy link

codecov bot commented Jan 24, 2020

Codecov Report

Merging #1042 into master will decrease coverage by 0.23%.
The diff coverage is 73.68%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1042      +/-   ##
==========================================
- Coverage    99.5%   99.26%   -0.24%     
==========================================
  Files          34       34              
  Lines        2408     2438      +30     
  Branches      306      312       +6     
==========================================
+ Hits         2396     2420      +24     
- Misses          6       10       +4     
- Partials        6        8       +2
Impacted Files Coverage Δ
piptools/scripts/compile.py 99.31% <100%> (-0.69%) ⬇️
tests/test_cli_compile.py 98.78% <61.53%> (-1.22%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 30e90be...0f7d96c. Read the comment docs.

Copy link
Member

@atugushev atugushev left a comment

Choose a reason for hiding this comment

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

Hello @alex

Thanks for your contribution! There's a related #908 issue that describes how to handle setup.py properly. Also, you might find useful previous closed PR #877.

@iamhatesz
Copy link
Author

Hi @atugushev

I am not sure if I understand #908 correctly. You propose the support of extras_require by passing --extras flag to pip-compile explicitly. This is fine when we have manually defined extras like dev, but I am afraid this could be a problem, when these extras are dynamically generated e.g. by specifying environment markers. In this case, I don't know what value should I use when calling pip-compile - maybe :"linux" in sys_platform, maybe :platform_system == "Linux", too many cases.

@atugushev
Copy link
Member

atugushev commented Jan 26, 2020

@iamhatesz

I am not sure if I understand #908 correctly. You propose the support of extras_require by passing --extras flag to pip-compile explicitly. This is fine when we have manually defined extras like dev, but I am afraid this could be a problem, when these extras are dynamically generated e.g. by specifying environment markers. In this case, I don't know what value should I use when calling pip-compile - maybe :"linux" in sys_platform, maybe :platform_system == "Linux", too many cases.

The original propose is to use pip install -e . to parse setup.py. To parse deps via pip install -e .[my-extra] (e.g., extra markers) proposed --extra=my-extra option. You are right, it won't work for environment markers, but it makes no sense since, for example, it's difficult (and nearly impossible) to build Windows dependencies on a Linux platform.

@iamhatesz
Copy link
Author

iamhatesz commented Jan 26, 2020

Hm, I don't consider parsing Windows-specific dependencies when being on Linux. I am fine with compiling dependencies twice: once on a Windows machine, and the second time on a Linux machine.

In my project, I have a few dependencies that can be satisfied only on Linux. Thus in my setup.py I use environment markers, so installing my package from pip works well. However, with current pip-tools I can't successfully compile my setup.py to requirements.txt, because it doesn't parse extra_requires. The solution with --extras flag is a workaround, but in this case I have to know what value I have to provide there. And for some cases, I can't be sure that, because some other packages could use environment markers as well, and I don't know to what exact extras key they will be transformed.

If pip install -e . parses extras which come from environment markers (satisfying only these appropriate for current platform), then this is exactly what I'd like to achieve.

@atugushev
Copy link
Member

atugushev commented Jan 26, 2020

@iamhatesz

If pip install -e . parses extras which come from environment markers (satisfying only these appropriate for current platform), then this is exactly what I'd like to achieve.

Yes, it works for the environment markers. You can try how it works by:

$ echo "-e ." | pip-compile - -qo- | sed '/^-e / d'

@iamhatesz
Copy link
Author

@atugushev cool, just tested, seems like a solution for me. What's the status of #908 then? Can I help you with the implementation?

@atugushev
Copy link
Member

@iamhatesz

cool, just tested, seems like a solution for me. What's the status of #908 then?

It's still under discussion, you might join there and express your thoughts.

Can I help you with the implementation?

Sure, any contributions are highly welcomed and encouraged 😉

@atugushev atugushev added enhancement Improvements to functionality setuptools Related to compiling requirements form setup.py labels Jan 30, 2020
from setuptools import setup
setup(
install_requires=[
'small-fake-a==0.1 ; "linux" in sys_platform',
Copy link
Member

Choose a reason for hiding this comment

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

I'd add some corner cases. Like empty or whitespace-only string after ;, also multiple conditions

@anthrotype
Copy link

Hi. Any updates on this? I'd love to see support for environment markers integrated in pip-compile setup.py. Thank you

@atugushev
Copy link
Member

I'm going to close this in favor of #1311. Thanks for the PR anyway! 🙏🏼

@atugushev atugushev closed this Mar 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improvements to functionality setuptools Related to compiling requirements form setup.py
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants