Install build dependencies as specified in PEP 518 #4144
Conversation
|
Awesome, I haven't looked at it yet but thanks a lot for taking this on. It will be really great to get this into pip, and that unlocks further future efforts (as you know!). |
|
:-) This is very WIP at the moment - I'm largely opening a PR as a lazy way to see what tests I've broken. But all the conceptual pieces are in place. |
|
The sysconfig module I'm using is not there on Python 2.6. When are we dropping 2.6? ;-) Assuming the answer is 'not yet', should I use the undocumented data in |
|
pip 9 is the last version of pip to support Python 2.6. That makes pip 10 a version that no longer does. We might end up releasing a 9.1 or so that doesn't drop support for 2.6. So it really depends. You can use 2.7 specific stuff but that means it will take longer before this can be released (since we don't want to release pip 10 so soon after releasing pip 9, normally wait 4-6 months) or you can keep 2.6 compat and this can be released as part of a 9.1. |
|
Thanks, that's good to know. I think I'll copy in a fallback for 2.6 for now. |
|
This is not mandatory to implement this feature, but #4182 has an interesting sub-problem with build time dependencies that may be useful to consider. |
|
Bumping to ask the state of this... |
|
This is now working, I believe. It's still pretty rough, but I'd like someone more familiar with pip to look over it and tell me what should be done totally differently before I invest too much effort polishing this way of doing it. I'll de-WIP it to make that clearer. |
takluyver
changed the title
|
@dstufft @xavfernandez RFC? |
|
Sorry, I've been sick. I'll take a look at this PR this week. |
|
No worries; I hope you're recovering nicely! |
|
Get well soon! ^.^ |
There was a problem hiding this comment.
The source code of tests/data/packages/pep518-3.0.tar.gz should also be provided in https://github.com/pypa/pip/tree/master/tests/data/src
pip/wheel.py
Outdated
| else: | ||
| os.environ['PYTHONPATH'] = self.save_pythonpath | ||
|
|
||
| rmtree(self.prefix) |
There was a problem hiding this comment.
Ideally, this could depend on the value of options.no_clean (which could be passed to the context manager's __init__)
|
|
||
| def __exit__(self, exc_type, exc_val, exc_tb): | ||
| if self.save_path is None: | ||
| os.environ.pop('PATH', None) |
There was a problem hiding this comment.
del os.environ['PATH'] should work
There was a problem hiding this comment.
It should, but if something inside the context removes one of these environment variables, del will fail with a KeyError. For the extra few characters, I prefer the more defensive option. Let me know if you disagree and I'll change it.
There was a problem hiding this comment.
I agree with @takluyver here. You wouldn't want this to fail fast if the code in context removed this environment variable (for whatever reason).
|
I added the source of the |
pip/wheel.py
Outdated
|
|
||
| def _install_build_reqs(self, reqs, prefix): | ||
| # Local install to avoid circular import (wheel <-> req_install) | ||
| from pip.req.req_install import InstallRequirement |
There was a problem hiding this comment.
s/Local install/Local import/ perhaps?
There was a problem hiding this comment.
Well spotted, done. :-)
|
Bump. Could someone re-run the PyPy build for this PR? It seems to be broken due to something not related to this change. I guess this is ready to be merged. |
|
Looks good to me |
|
Is the news-file/pr check the only thing holding this PR up at the moment? |
pip/wheel.py
Outdated
| except ImportError: | ||
| # This section is for compatibility with Python 2.6, and can be removed | ||
| # once Python 2.7 is the minimum supported version. | ||
| sysconfig = None |
There was a problem hiding this comment.
2.6 is not supported so you should be fine to just delete this whole bit and depend entirely on sysconfig.
pip/wheel.py
Outdated
| pp_toml = pytoml.load(f) | ||
| return pp_toml.get('build-system', {}).get('requires', []) | ||
|
|
||
| return [] # No pyproject.toml |
There was a problem hiding this comment.
If there is no pyproject.toml, can we return ["setuptools", "wheel"] so we can then drop the need to have setuptools/wheel preinstalled for building from sdists? The assumption would then be if someone has a pyproject.toml they are responsible for setting the build-system requires correctly?
It might make sense to gate that on whether or not they have a [build-system] key though rather than just whether the file exists or not, since some projects have already started to use [tool.*].
There was a problem hiding this comment.
I think the simplest would be to say that if the requires key is missing, then it defaults to that?
There was a problem hiding this comment.
Hmm, that is reasonable I suppose. The use case of something that has a contained within itself build system is probably small enough that optimizing for that case where they might have to do requires = [] is probably small enough (I assume that TOML allows an empty list?).
There was a problem hiding this comment.
Yeah... Though I'm actually flip flopping a bit mentally. The spec actually says that build-system.requires is mandatory in all pyproject.toml files. It's annoying if people are already creating invalid files; are you sure we can't break them? 😛
The discussion below about isolation reminds us too that we need some trigger for enabling isolated builds. I think the original idea was that either you would have a pyproject.toml and it would have explicit requires and you would get an isolated build, or else you would have none of those. If we have to treat pyproject.toml's that are missing the build-system section as legacy packages, then we need something different.
Of course even if we were able to make build-system.requires mandatory, we'd still have to default on the actual build system key (the one that's not specified yet). This is a late and somewhat wacky idea, but I guess we could say that it's mandatory to have a key build-system = "setuptools", exactly that string, and then later we could redefine it as the package that gets imported to provide the build system API once that's defined...
There was a problem hiding this comment.
Well, I guess we could pull down the sdists from PyPI and see if anyone is shipping a pyproject.toml to PyPI yet or not. If nobody is actually shipping one to PyPI yet we can possibly just gate on it.
There was a problem hiding this comment.
One other thing to consider is whether we should automatically add wheel if they list setuptools, since it will almost certainly be an error if they don't.
There was a problem hiding this comment.
The PEP actually specifies that setuptools and wheel are implied when there's no pyproject.toml: "Because the use of setuptools and wheel are so expansive in the community at the moment, build tools are expected to use the example configuration file above as their default semantics when a pyproject.toml file is not present.".
There was a problem hiding this comment.
This will take a bit more working out, because as @njsmith mentioned, the code here currently falls back to a non-isolated build if pyproject.toml is missing. If we just add default packages here without changing anything else, all builds will be isolated by default, which may break a number of packages.
There was a problem hiding this comment.
Ah yea that is true, though I think it's also true if a project has a pyproject.toml that has an empty build-system.requires, so we probably need to propagate up whether or not to expect an isolated environment or not.
pip/wheel.py
Outdated
| if sys.version_info >= (2, 7): | ||
| mpip = 'pip' | ||
| else: | ||
| mpip = 'pip.__main__' # Python 2.6 can't execute a package with -m |
There was a problem hiding this comment.
Again, master doesn't support 2.6 anymore, so this can be dropped.
pip/wheel.py
Outdated
| python_tag=python_tag, | ||
| isolate=True) | ||
| else: | ||
| # Old style build, in the current environment |
There was a problem hiding this comment.
I'd probably not call it old style, since it'd also be legtimately the style that would be used if someone had a completely self contained build system yea?
There was a problem hiding this comment.
Ideally a self contained build system would mean building inside an empty clean environment, not the user's current environment.
There was a problem hiding this comment.
Well this isn't that either :) If someone explicitly has no build deps it will be inside the current environment.
|
What else is allowed in |
|
As I understand it, that is not valid. Only the Here's the relevant bit of PEP 518:
|
|
It might make sense to have PyPI validate a pyproject.toml file (if one exists) on upload to ensure the schema is correct.
… On Sep 25, 2017, at 6:35 PM, Thomas Kluyver ***@***.***> wrote:
As I understand it, that is not valid. Only the build-system table has been specified; any other information should be under the tool namespace, arranged by the name of the tool that is specifying it.
Here's the relevant bit of PEP 518 <https://www.python.org/dev/peps/pep-0518/>:
All other top-level keys and tables are reserved for future use by other PEPs except for the [tool] table. Within that table, tools can have users specify configuration data as long as they use a sub-table within [tool], e.g. the flit tool would store its configuration in [tool.flit].
We need some mechanism to allocate names within the tool.* namespace, to make sure that different projects don't attempt to use the same sub-table and collide. Our rule is that a project can use the subtable tool.$NAME if, and only if, they own the entry for $NAME in the Cheeseshop/PyPI.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub <#4144 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAI6O2Tsm-nW5Nbz25ddCwJP0sOYGGlGks5smCrHgaJpZM4K_ARO>.
|
That might be an issue is someone make an sdist and a wheel, the wheel get uploaded first, then the sdist get refused because of invalid |
This isn't a new problem, and is essentially an unsolvable problem unless we either mandate that sources are uploaded first (can't / won't) or we remove all validation of a sdist upon upload (can't / won't). |
I know you want to also have a "staging area" when things could get uploaded, but not made public first. This could be a place where this is handled. Only allow moving from "staging" to published is all checks are passing. |
|
So, this might require a separate discussion but... are there any plans to have just 1 file to control Python projects? Like Pipfile and pyproject.toml combined. |
|
|
|
Yes I understand they serve different purposes but is there a disadvantage to using just 1 file for all the functionality? |
|
Yes, not everything you'd use |
|
@dstufft Is this not actually valid then? https://gist.github.com/dstufft/72ede814fcf35246df6a4a9a700b4b2f |
|
Nope, it was a sketch of some ideas not an example of a valid |
|
Is |
|
From an author's POV it is meant to replace |
|
Thanks for the clarification! |
Pip will do this automatically if needed according to pypa/pip#4144.
|
This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
lock
bot
added
the
auto-locked
Fixes #3691.
PEP 518 describes a way for a source tree (e.g. a VCS checkout or an sdist) to declare build dependencies in a toml file. This is my WIP attempt to support that in pip.
This change is