Add PWD to sys.path as part of the PEP 517 build

[fchorney]

in reference to this: pypa/pip#6163

[pganssle]

I am amenable to this change, but I think if we start messing around with sys.path in the build hooks, I don't see a good way to switch that behavior back - we won't necessarily be able to detect that someone is counting on this, and so we cannot give any sort of deprecation warning about it.

Given that you need to opt in to using PEP 517, we have an opportunity now to decide what we want the semantics to be, rather than trying to maintain the existing semantics. What are the downsides of requiring that setup.py be more explicit about where it's importing from? I suppose adding to the difficulty of migration is one.

[h-vetinari]

Given that you need to opt in to using PEP 517

That's not the case, hence the swift response to the pip-release, see https://github.com/pypa/pip/labels/C%3A%20PEP%20517%20impact
Rather, it is opt-out with --no-use-pep517.

[pganssle]

Maybe I'm wrong, but this should only be affecting projects that have added build-backend into their pyproject.toml, which greatly limits the scope of this problem (though the fact that there's a bunch of stuff in the wild getting burned is concerning). If pip started using setuptools.build_meta as the backend by default then I agree it's not opt-in.

[fchorney]

I’ll note that this affects any project with a pyproject.toml, regardless if there is a build-backend.

For example, I have a project that uses a pyproject.toml as a config for python black, and this affects that project.

[altendky]

https://github.com/altendky/tempexample/tree/82bd123917bedce24b7e19acbfc4a289f681ca28

There's a pretty minimal example showing the success without pyproject.toml and failure with it.

Click to expand log

python3 -m venv venv
+ python3 -m venv venv
venv/bin/python -m pip install --upgrade pip==19.0.1 setuptools==40.6.3
+ venv/bin/python -m pip install --upgrade pip==19.0.1 setuptools==40.6.3
Collecting pip==19.0.1
  Using cached https://files.pythonhosted.org/packages/46/dc/7fd5df840efb3e56c8b4f768793a237ec4ee59891959d6a215d63f727023/pip-19.0.1-py2.py3-none-any.whl
Collecting setuptools==40.6.3
  Using cached https://files.pythonhosted.org/packages/37/06/754589caf971b0d2d48f151c2586f62902d93dc908e2fd9b9b9f6aa3c9dd/setuptools-40.6.3-py2.py3-none-any.whl
Installing collected packages: pip, setuptools
  Found existing installation: pip 18.1
    Uninstalling pip-18.1:
      Successfully uninstalled pip-18.1
  Found existing installation: setuptools 40.6.2
    Uninstalling setuptools-40.6.2:
      Successfully uninstalled setuptools-40.6.2
Successfully installed pip-19.0.1 setuptools-40.6.3
rm pyproject.toml
+ rm pyproject.toml
venv/bin/python -m pip install --ignore-installed .
+ venv/bin/python -m pip install --ignore-installed .
Processing /home/altendky/tempexample
Installing collected packages: pkg
  Running setup.py install for pkg ... done
Successfully installed pkg-0.0.0
touch pyproject.toml
+ touch pyproject.toml
venv/bin/python -m pip install --ignore-installed .
+ venv/bin/python -m pip install --ignore-installed .
Processing /home/altendky/tempexample
  Installing build dependencies ... done
  Getting requirements to build wheel ... error
  Complete output from command /home/altendky/tempexample/venv/bin/python /home/altendky/tempexample/venv/lib/python3.7/site-packages/pip/_vendor/pep517/_in_process.py get_requires_for_build_wheel /tmp/tmptq1h87o9:
  Traceback (most recent call last):
    File "/home/altendky/tempexample/venv/lib/python3.7/site-packages/pip/_vendor/pep517/_in_process.py", line 207, in <module>
      main()
    File "/home/altendky/tempexample/venv/lib/python3.7/site-packages/pip/_vendor/pep517/_in_process.py", line 197, in main
      json_out['return_val'] = hook(**hook_input['kwargs'])
    File "/home/altendky/tempexample/venv/lib/python3.7/site-packages/pip/_vendor/pep517/_in_process.py", line 54, in get_requires_for_build_wheel
      return hook(config_settings)
    File "/tmp/pip-build-env-3pas05w4/overlay/lib/python3.7/site-packages/setuptools/build_meta.py", line 115, in get_requires_for_build_wheel
      return _get_build_requires(config_settings, requirements=['wheel'])
    File "/tmp/pip-build-env-3pas05w4/overlay/lib/python3.7/site-packages/setuptools/build_meta.py", line 101, in _get_build_requires
      _run_setup()
    File "/tmp/pip-build-env-3pas05w4/overlay/lib/python3.7/site-packages/setuptools/build_meta.py", line 85, in _run_setup
      exec(compile(code, __file__, 'exec'), locals())
    File "setup.py", line 3, in <module>
      import pkg
  ModuleNotFoundError: No module named 'pkg'
  
  ----------------------------------------
Command "/home/altendky/tempexample/venv/bin/python /home/altendky/tempexample/venv/lib/python3.7/site-packages/pip/_vendor/pep517/_in_process.py get_requires_for_build_wheel /tmp/tmptq1h87o9" failed with error code 1 in /tmp/pip-req-build-3ek7dus1

[uranusjr]

For reference—here’s the thread discussing how a pyproject.toml missing build-backend.requires is handled. pyproject.toml is treated as the indicator whether a project wishes to use PEP 517; if a project supplies that file, the maintainers should be responsible to ensure it works with isolated builds.

https://mail.python.org/archives/list/distutils-sig@python.org/message/4QWJJAPLH7HECEMOPUFVFX35VVR5MJ4L/

[pganssle]

Hm, if pip is defaulting to the setuptools backend, then the opportunity is not as great as I thought, I do think we basically have to fix this. If anyone wants to take a first crack at this I'd be happy to review it. Preferably we want to make the absolute minimum change that preserves something like the existing semantics.

I also do not want to break anyone who has deliberately removed PWD from their sys.path.

[uranusjr]

The old code path (before PEP 517) in pip chdir to the project root before running setup.py, so in most cases people have pwd in path simply because sys.path contains . by default. Maybe it’d be reasonable to do the same chdir in the build hook? This would only break in cases where people run the PEP 517 tool on its own (not via pip), which should be fairly unlikely given the relatively recent support.

I’ll try to find some time to come up with something today.

[uranusjr]

While working on this, I realise I am personally against the practice of importing the module in setup.py. This kind of runtime-dependent setup is fundamentally at odds with the recent move to declarative build systems, and is a very common source of breakages in environment-agnostic tools (e.g. Pipenv). I kind of wish this can be an opportunity to push people away from this.

[mig4]

@uranusjr I agree, I think it's a bad practice to import the module being installed from its setup.py. The PyPA guide on Single Sourcing Package Version has a couple of recommendations on a better way of handling this requirement.

PEP 517's Build Environment section specifies nothing about an environment should be assumed except that the dependencies declared in pyproject.toml are available, so I'm not sure if this is a bug in pip/setuptools but rather a change that exposes bugs in various projects' setup.py files.