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.

Sign up for GitHub

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

Dedup and merge reqs to pass into pip #15

Closed
wants to merge 1 commit into from
Closed

Dedup and merge reqs to pass into pip #15

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 24 additions & 1 deletion pep517/envbuild.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
"""Build wheels/sdists by installing build deps to a temporary environment.
"""

import re
import os
import logging
import pytoml
Expand All @@ -14,13 +15,35 @@

log = logging.getLogger(__name__)


def _load_pyproject(source_dir):
with open(os.path.join(source_dir, 'pyproject.toml')) as f:
pyproject_data = pytoml.load(f)
buildsys = pyproject_data['build-system']
return buildsys['requires'], buildsys['build-backend']


REQUIREMENT_PATTERN = re.compile(r'^(?P<name>[^~><=!]+)(?P<spec>.*)$')

Copy link

Choose a reason for hiding this comment

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

should we use instead here the pep 440 parser instead of the naive regex? I've used it when I hit this edge case in tox to resolve the conflict. Tbh though I think we should really warn if the two versions don't match exactly. it's likely a bug.

Copy link
Member

Choose a reason for hiding this comment

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

@pfmoore @takluyver Thoughts?

Copy link
Member

Choose a reason for hiding this comment

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

I suspect so, but I don't use this class in pip, so I've no real idea.

Copy link
Member Author

@uranusjr uranusjr Sep 19, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

AFAICT there are no existing PEP 440 implementations that can tell if a version specifier has conflicts by looking at it. So the only viable thing to do IMO is to just feed it into pip and see what happens.

Theoratically the input here can be an arbitrary PEP 508-compliant string, complete with markers and such, so the complete solution should likely use packaging.requirements to do the parsing. I can do that if @takluyver thinks it’s a good idea to add packaging as a dependency.

Copy link

@gaborbernat gaborbernat Sep 19, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

https://packaging.pypa.io/en/latest/requirements/ should allow you to parse, and then you can make the decision if the two versions (if both exists) matches or not. but yeah overall I agree with your assessment. your regex here is a poor mans packaging and if needed we might as well depend on upstream to avoid bugs.


def _as_req_list(reqs):
specs = {}
for req in reqs:
match = REQUIREMENT_PATTERN.match(req)
if not match:
specs[req] = []
continue
name, spec = match.group('name', 'spec')
try:
specs[name].append(spec)
except KeyError:
specs[name] = [spec]
return [
'{}{}'.format(name, ','.join(s for s in spec if s))
for name, spec in specs.items()
]


class BuildEnvironment(object):
"""Context manager to install build deps in a simple temporary environment

Expand Down Expand Up @@ -90,7 +113,7 @@ def pip_install(self, reqs):
return
log.info('Calling pip to install %s', reqs)
check_call([sys.executable, '-m', 'pip', 'install', '--ignore-installed',
'--prefix', self.path] + list(reqs))
'--prefix', self.path] + _as_req_list(reqs))

def __exit__(self, exc_type, exc_val, exc_tb):
if self._cleanup and (self.path is not None) and os.path.isdir(self.path):
Expand Down