-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
Stop excluding files from package list... #1098
Stop excluding files from package list... #1098
Conversation
techalchemy
commented
Nov 19, 2017
- Fixes Editables from pipfile are installed shallow #1079 and Cannot install package due to a not really missing dependency #909
- I don't know why I implemented this logic in the first place
- I might be an idiot
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we get a test for the case we're trying to fix here. That will keep it from being regressed later.
pipenv/project.py
Outdated
if not (is_installable_file(k) or is_installable_file(v) or | ||
any(file_prefix in v for file_prefix in ['path', 'file'])): | ||
ps.update({k: v}) | ||
ps.update({k: v}) | ||
else: | ||
if not (any(is_vcs(i) for i in [k, v]) or |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We really need a comment expressing what this is doing and why.
pipenv/project.py
Outdated
else: | ||
if not (any(is_vcs(i) for i in [k, v]) or | ||
any(is_installable_file(i) for i in [k, v]) or | ||
any(is_valid_url(i) for i in [k, v])): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any reasons we're deviating from the use of i
in the other checks? They should probably all be entry
or i
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I was contemplating swapping everything to entry
but I had to re-think all my logic anyway so I just left it as i
across the board (which is not very descriptive but uh, sometimes it keeps our lines under 80 characters?)
- Will be even more complete with pypa#1098
CI is lying, transient failures. Things are fine |
Btw appveyor passes on my own account: You can fix a lot of transient appveyor failures if you add the following to https://ci.appveyor.com/project/kennethreitz/pipenv/settings/build => before build script => cmd: rmdir /s /q "c:\users\appveyor\appdata\local\temp\1\tmp*\configparser"
rmdir /s /q "c:\users\appveyor\appdata\local\pip-tools\cache" |
9ffe911
to
29e8447
Compare
- Fixes pypa#1079 and pypa#909 - Need to exclude non-editable file/path/URI deps from the list we give to pip-tools - We should really separate this logic from the logic that supplies what is supposed to be a list of packages Add tests and comments to prevent regression - Ensure we resolve install_requires from local setup.py files - Squash commits
29e8447
to
c92e331
Compare
- Re-add non-editable setuptools-installable directory paths to the exclusion list for Project package generation - This is used to generate package lists for pip-tools