Allow backends to be called in fresh subprocesses #2712
Comments
|
Your package seems to be violating https://peps.python.org/pep-0518. No, you cannot use setup_requires https://github.com/stephenfin/tox-issue-2717/blob/main/setup.py#L6 to specify your dependencies; it must be within pyproject.toml, see https://docs.openstack.org/pbr/latest/user/using.html |
|
Hi @gaborbernat, I'd already tested this but didn't push it as I thought it would confuse matters. I've now pushed a follow-up that configures the build-time dependencies but the exact same thing happens. I've contributed to |
|
Also, as an aside, I don't think the
Given tox is defaulting to calling |
|
PR welcome 👍
Can't really guide beyond debug and see where and how breaks 👍 |
gaborbernat
added
bug:minor
|
This is not truly just a pbr issue because see #2567, other pbr projects do pass. So must be something else too at play. |
|
Okay, so I've narrowed the hanging issue down to this line: If I comment that out, things pass successfully. That's a bit dumb though since I suspect the "amicable shutdown" is desirable. The bigger issue is why is being called. The executor should be reported as dead. If I slap an This suggests to me that the |
|
I've run into this independently and done a bit of digging before I saw this issue and thought I would share a bit of what I've learned. Editing fixes the problem. Documentation for this is at https://setuptools.pypa.io/en/latest/userguide/package_discovery.html#custom-discovery. What is weird is that the Another thing I notice is that when you add the |
|
One difference I've found is that Tox via pyproject-api appears to call the backend with the flag with process reuse set to true: Perhaps this looping is the source of not exiting? For the package content lookup and version behavior differences it is less clear to me how the behavior is changing. Perhaps |
This seems to happen because setuptools is causing Additionally, we'll avoid any unexpected side effecting that happens by running these
in order within the same process. The expectation from |
What makes you think this is true?
Note the wording here; it says |
I was primarily looking at how
The pep explicitly calls out why this is desirable. Enabling backends to change process global state.
I'm not sure if PEPs follow the RFC definitions for these words. Its possible that this is an oversight in the PEP? In particular they have explicitly called out the reason for doing that and tox seems to be running into that problem. To me a tool like tox should be forgiving because you don't know how old the existing code bases are that may be trying to use the tool. One option may be only fall back to the extra processes when using the legacy system? Though I guess the PEP gives potential freedom for non legacy backends to also modify process global state. I've made a hacked up patch to tox to which does seem to get this working without any changes to the host project. I think this confirms that the issue is in reusing global process state. I know this isn't mergeable as is, I just wanted to see if changing the forking behavior addressed the underling problem. |
Doesn't matter. build is just one implementation of PEP-517, it's not authoritative. The spec is authoritative.
Desirable != required.
It does. We've had this discussion in the past within PyPA.
Nope, we could not get a consensus on requiring this, so we settled on encouragement but not a requirement.
People also expect it to be fast. One of their major complaints about it is that it's slow. We worked hard on improving this, so I'm not willing to compromise. I'd be happy to accept a PR, though, that configures this behavior for those backends requiring this fresh subprocess behavior. PR for that is welcome. |
gaborbernat
changed the title
gaborbernat
added
feature:new
Do you think this should happen automatically when falling into the |
It should be explicitly configurable. However, the default should be False unless the backend is |
|
If you want to put in a PR you want to do changes around here https://github.com/tox-dev/tox/blob/main/src/tox/tox_env/python/virtual_env/package/pyproject.py#L309 and the configuration defined here https://github.com/tox-dev/tox/blob/main/src/tox/tox_env/python/virtual_env/package/pyproject.py#L123 |
Tox 4.x broke our testing and they don't seem to care about backwards compatibility tox-dev/tox#2712
Tox 4 hangs with this repo and I don't know how to fix it. I don't have the time or patience to figure it out. Someone should make this repo compatible. tox-dev/tox#2712
|
I actually think the hanging is a design issue with Still trying to figure out how to resolve the actual underlying issue with pbr, which simply highlighted this hanging issue. |
|
I think I have sussed out the That brings us on to what to do with this. It feels like adding a knob to run commands in separate subprocesses could be helpful for debugging tox-backend incompatibilities (which seem rather likely - at least for now - since |
|
Also, a random point for observers: (or, for projects without a At first this seems odd and it's actually the combination of this call (which results in generation of a Just noting this in case anyone else was momentarily caught out by this 😄 |
|
You could switch your packaging to wheel from sdist, to save the need to call prepare_metadata_for_build_wheel+ build_sdist. Alternatively, we need those two. |
In change I1f2b4d34e587389f7e11b99d000e14477cf5091b, we attempted to resolve an issue whereby using PBR as a PEP-517 backend could result in recursion. We did this by setting 'dist.pbr' to 'None' but later discovered this introduced a regression that prevented us from writing the 'pbr.json' file into generated sdists. The fix for the regression, change I407ae88ab8de4b61f94034b3d79a2ca7f7d79d16, stopped setting 'dist.pbr' to 'None' and instead set a global flag to indicate whether we had already been called or not. Unfortunately it seems this fix is also insufficent. As discussed in tox#2712 [1], tox v4 has implemented its own implementation of PEP-517 - 'pyproject-api' [2] - and unlike 'build' this implementation optionally allows re-use of the backend process for multiple PEP-517 commands. tox's 'Pep517VirtualEnvFrontend' does just this. This means if we run multiple commands that require generation of a 'Distribution' object - say, 'prepare_metadata_for_build_wheel' followed by 'build_sdist' - anything but the first step will not result in proper population of said 'Distribution' objects. The solution to this issue is simple: per $subject, instead of setting our recursion-detection canary at the module level, set it at the Distribution level. [1] tox-dev/tox#2712 [2] github.com/tox-dev/pyproject-api/ Change-Id: I67909d732a74550fbcd7c06a9e2f4ac88c063444 Signed-off-by: Stephen Finucane <sfinucan@redhat.com>
* Update pbr from branch 'master'
to 1ca77814931463993b15fbbb13a26e027a485cd3
- Merge "Tie recursion calls to Dist object, not module"
- Tie recursion calls to Dist object, not module
In change I1f2b4d34e587389f7e11b99d000e14477cf5091b, we attempted to
resolve an issue whereby using PBR as a PEP-517 backend could result in
recursion. We did this by setting 'dist.pbr' to 'None' but later
discovered this introduced a regression that prevented us from writing
the 'pbr.json' file into generated sdists. The fix for the regression,
change I407ae88ab8de4b61f94034b3d79a2ca7f7d79d16, stopped setting
'dist.pbr' to 'None' and instead set a global flag to indicate whether
we had already been called or not. Unfortunately it seems this fix is
also insufficent. As discussed in tox#2712 [1], tox v4 has implemented
its own implementation of PEP-517 - 'pyproject-api' [2] - and unlike
'build' this implementation optionally allows re-use of the backend
process for multiple PEP-517 commands. tox's 'Pep517VirtualEnvFrontend'
does just this. This means if we run multiple commands that require
generation of a 'Distribution' object - say,
'prepare_metadata_for_build_wheel' followed by 'build_sdist' - anything
but the first step will not result in proper population of said
'Distribution' objects.
The solution to this issue is simple: per $subject, instead of setting
our recursion-detection canary at the module level, set it at the
Distribution level.
[1] tox-dev/tox#2712
[2] github.com/tox-dev/pyproject-api/
Change-Id: I67909d732a74550fbcd7c06a9e2f4ac88c063444
Signed-off-by: Stephen Finucane <sfinucan@redhat.com>
Tox 4 hangs with this repo and I don't know how to fix it. I don't have the time or patience to figure it out. Someone should make this repo compatible. tox-dev/tox#2712
Issue
I'm using
pbrfor a number of projects. I've observed that when using this with tox 4.x, the package fails to build with the following error message being emitted:However, I'm already defining
packagesin mysetup.cfg:(Note that
[files]is the correct section forpbr)From reading the traceback, it feels like
toxis not detecting that I am usingpbrand is attempting to use plain-oldsetuptools. However, tools likebuild(i.e.python -m build) manage to detectpbrjust fine so I don't know what's going on here.This process also doesn't quietly die. Instead, it hangs around forever. I've seen CI failures on some packages where the build finally timed out after 3 hours. Attempting to kill the process locally with
Ctrl + Cfails and I need to issue akillto the parent process.Environment
Provide at least:
pip listof the host Python wheretoxis installed:Output of running tox
Provide the output of
tox -rvv:Minimal example
You can find a minimal reproducer here.
The full output given when running
tox -e py311is:The text was updated successfully, but these errors were encountered: