Windows file locking requires pywin32

[anatoly techtonik]

I've noticed that file locking logic in ​django/core/files/locks.py will fail on Windows if no pywin32 is not installed. If this part is critical, ctypes version of portalocker is available from ​http://roundup.hg.sourceforge.net/hgweb/roundup/roundup/file/tip/roundup/backends/portalocker.py

[Russell Keith-Magee]

PyWin32 isn't listed as an install dependency on Windows, but perhaps it should be.

I'll leave this as DDN because someone who cares/knows about Windows needs to make a call here -- either documenting PyWin32 as a dependency, or finding other options, like portalocker.

[Michael Manfre]

I'm not in a position to make a decision, but I'll share some of my insights.

I've needed PyWin32 installed on every windows system because of django-mssql, the file locking support was secondary. The installation is easy and the project doesn't release new version very often.

The only real downside to PyWin32 that I've encountered is that it doesn't play well with virtualenv because --system-site-packages is required. This requirement is not exclusive to PyWin32 and almost every binary package that drops DLLs needs to be installed globally and use --system-site-packages in virtualenvs. This can become a problem with maintaining an automated build system and for some deployment scenarios.

If requiring --system-site-packages is an acceptable, then documenting PyWin32 would be the easiest path forward and I'd be happy to submit a patch. If Django doesn't want to force --system-site-packages, then definitely look for another option that only addresses the file locking issue, instead of all the other stuff that is rolled in to PyWin32.

[Florian Apolloner]

We should at least document the gain of an installed PyWin32.

[Philippe Ombredanne]

I personally like techtonik proposed solution to use the ctypes port of portalocker he made for Roundup instead of PyWin32.

This could have the benefit --if included in Django -- of not requiring the installation of a third-party package to get the core Django to run, which is the general approach to date. The code is simple enough and no documentation changes would be needed, especially no windows-specific section with added requirements.

Locking is the only area where PyWin32 is needed. Removing that dependency would be a good thing imho.

[Kevin Christopher Henry]

I definitely think removing the PyWin32 dependency is the way to go, especially since the ctypes version is pretty straightforward.

I've added a patch based on the ctypes port: ​https://github.com/django/django/pull/1639.

[Tim Graham]

Someone who is more familiar with file locking will have to do a final review, but this looks reasonable to me.

[Anssi Kääriäinen]

In don't know who could do the final review here.

Unless it is actually known who should do a final review for this I suggest we merge. Just waiting seems like a bad approach...

[Tim Graham]

Ok, I've asked for a rebase of the patch and possibly a mention in the release notes. Will merge it once that's done.

[Kevin Christopher Henry <k@…>]

In 6fe26bd3ee75a6d804ca3861181ad61b1cca45ea:

Fixed #19373 -- Ported Windows file locking from PyWin32 to ctypes

There wasn't any file locking under Windows unless PyWin32 was
installed. This removes that (undocumented) dependency by using ctypes
instead.

Thanks to Anatoly Techtonik for writing the ctypes port upon which this
is based.

[Tim Graham]

There's been on regression here on some platforms like App Engine according to this ​PR.

[Tim Graham]

​Updated PR.

[Aymeric Augustin]

LGTM

[Tim Graham <timograham@…>]

In 61fdb8d487f62e5b89b3bef9cb2b212dcec6d1e5:

Fixed regression in file locking on some platforms.

Some platforms with os.name == 'posix' do not have the
fcntl module, e.g. AppEngine.

refs #19373.