Fix problem with deleting temporary folders on Windows

[eblis]

This is another attempt for pull request https://github.com/python-poetry/poetry/pull/5171

• [x] Added tests for changed code.
• [x] Updated documentation for changed code.

From what I can tell the relevant code has been moved from poetry to poetry-core, I've created a new pull request targeting this repository.

[sonarqubecloud[bot]]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
No Duplication information

[da1910]

This issue still exists (albeit intermittently) and is again blocking our CI process, is there anything we as users can do to get this fix expedited?

[Andy-Grigg]

I'm seeing this issue as well. From what I can tell, this fix will resolve the issue and stop our intermittent Windows build failures.

[HenriqueAJNB]

Hi, I'm waiting for this PR to merge. Can anyone give us some feedback on how is it going? How can we help to make it happen?

[DudeNr33]

I would also really appreciate this fix, as I am once again sitting in front of my computer, restarting poetry commands over and over for simply adding a new dependency.

[eblis]

@da1910 sorry, github suggested you as a reviewer so I just went ahead and accepted. I hope it doesn't take too long for this to get reviewed again, or else we'll have the same problem with stale code yet again.

[xmnlab]

that would be great to see this merged soon <3 this problem is affecting me in a CI workflow for windows thanks @eblis for working on that

[dimbleby]

https://github.com/python/cpython/pull/24793 adds ignore_cleanup_errors to the parameters of tempfile.TemporaryDirectory(), from python 3.10 onwards - which seems to be aimed at exactly this.

So perhaps the implementation of temporary_directory() should go: "if python >= 3.10 then use TemporaryDirectory(ignore_cleanup_errors=True), else do this much more complicated thing"

Or perhaps - given how longstanding this must be (https://github.com/python-poetry/poetry/issues/1031) - poetry should simply use TemporaryDirectory always, with ignore_cleanup_errors=True where available. Windows users on older pythons must be an ever-smaller minority.

[da1910]

python/cpython#24793 adds ignore_cleanup_errors to the parameters of tempfile.TemporaryDirectory(), from python 3.10 onwards - which seems to be aimed at exactly this.

So perhaps the implementation of temporary_directory() should go: "if python >= 3.10 then use TemporaryDirectory(ignore_cleanup_errors=True), else do this much more complicated thing"

Or perhaps - given how longstanding this must be (python-poetry/poetry#1031) - poetry should simply use TemporaryDirectory always, with ignore_cleanup_errors=True where available. Windows users on older pythons must be an ever-smaller minority.

I would strongly favour making some effort to fix this for python 3.8 and 3.9, the fact that it's taken this long to address isn't really a reason to wash our hands of support, Python 3.9 is supported for another two and a half years after all!

[dimbleby]

Python 3.9 is supported for another two and a half years after all!

Sure. However, the current situation has been tolerated for all of that and then some; and for the huge majority who are not using older Pythons on Windows, I'd have a much higher degree of confidence in the standard library's temporary-directory implementation than in a home-grown implementation .

So I do think it would be preferable in general to use TemporaryDirectory(). I don't have very strong opinions about whether Windows with python 3.8 and 3.9 are important; but IMO it would at least be not absurd to choose to keep poetry code simple and decline to fix this for them.

[da1910]

Python 3.9 is supported for another two and a half years after all!

Sure. However, the current situation has been tolerated for all of that and then some; and for the huge majority who are not using older Pythons on Windows, I'd have a much higher degree of confidence in the standard library's temporary-directory implementation than in a home-grown implementation .

So I do think it would be preferable in general to use TemporaryDirectory(). I don't have very strong opinions about whether Windows with python 3.8 and 3.9 are important; but IMO it would at least be not absurd to choose to keep poetry code simple and decline to fix this for them.

I would agree about simplicity for certain, but I'm going to disagree with you about the scope of the problem. I've personally wasted several days of my life rerunning CI because of this issue, and there are plenty of other people on the original thread who have had the same experience for the last four years (I've only been using poetry 'in anger' for the last year or so).

Windows may be a minority for open-source developers, but for casual python users it's vastly the most common OS, and to have a project like poetry deprioritise fixes for windows specific bugs with in-support python versions seems slightly unfriendly.

[Secrus]

So perhaps the implementation of temporary_directory() should go: "if python >= 3.10 then use TemporaryDirectory(ignore_cleanup_errors=True), else do this much more complicated thing"

This is the solution I would go for and which is the most likely going to be accepted.

[dimbleby]

"unfriendly" isn't the aim, more like "pragmatic"!

Windows makes up less than 3% of poetry's downloads according to https://pypistats.org/packages/poetry - bottom of page. This is surely a very imperfect measure but, so far as I know, the best we have.

I don't know how to split this up by python version, and I certainly don't know how to split this up by "open-source developers" vs "casual users". Either way I suppose it's true that 3% of poetry's users today is a larger absolute number than 3% of poetry's users was in 2019.

Anyway: if maintainers prefer to take on the additional complication, I've no fight to pick with that.

[dimbleby]

re the typing errors in the pipeline: so far as I can see no-one ever passes any arguments to temporary_directory(), so I'd just remove the args and kwargs altogether.

[eblis]

re the typing errors in the pipeline: so far as I can see no-one ever passes any arguments to temporary_directory(), so I'd just remove the args and kwargs altogether.

I saw this message too late, spent a bit of time fighting with mypy and in the end just added the argument to kwargs before calling the function, seemed to please mypy.

[xmnlab]

Windows makes up less than 3% of poetry's downloads according to https://pypistats.org/packages/poetry

but maybe this analysis about this number could be a bit biased ... because poetry would be used more by devs and on CI ... the experience programming on windows is not that great ... so probably most python dev doesn't use windows (IMHO) ... and if we it is not working on CI using windows, no one is testing their packages with poetry for windows. (that is my case)

so I guess that this number makes sense. but maybe if the poetry works well for windows .. that number will increase (at least a bit)

[xmnlab]

and just as an extra info, on conda/conda-forge the downloads for windows is almost the same number for osx (for the latest version there): https://anaconda.org/conda-forge/poetry/files

[sonarqubecloud[bot]]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
No Duplication information