Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

pex fails to run on windows: "No module named 'fcntl'" #1155

Closed
mgrandi opened this issue Dec 28, 2020 · 11 comments
Closed

pex fails to run on windows: "No module named 'fcntl'" #1155

mgrandi opened this issue Dec 28, 2020 · 11 comments
Assignees
Labels

Comments

@mgrandi
Copy link

mgrandi commented Dec 28, 2020


PS C:\Users\mgrandi> py -3 -m venv .\Temp\testvenv
PS C:\Users\mgrandi> .\Temp\testvenv\Scripts\Activate.ps1
(testvenv) PS C:\Users\mgrandi> pip3 install pex
Collecting pex
  Using cached pex-2.1.24-py2.py3-none-any.whl (2.5 MB)
Installing collected packages: pex
Successfully installed pex-2.1.24
WARNING: You are using pip version 20.2.3; however, version 20.3.3 is available.
You should consider upgrading via the 'c:\users\mgrandi\temp\testvenv\scripts\python.exe -m pip install --upgrade pip' command.
(testvenv) PS C:\Users\mgrandi> pex --help
Traceback (most recent call last):
  File "C:\Python39\lib\runpy.py", line 197, in _run_module_as_main
    return _run_code(code, main_globals, None,
  File "C:\Python39\lib\runpy.py", line 87, in _run_code
    exec(code, run_globals)
  File "C:\Users\mgrandi\Temp\testvenv\Scripts\pex.exe\__main__.py", line 4, in <module>
  File "c:\users\mgrandi\temp\testvenv\lib\site-packages\pex\bin\pex.py", line 19, in <module>
    from pex.common import die, safe_delete, safe_mkdtemp
  File "c:\users\mgrandi\temp\testvenv\lib\site-packages\pex\common.py", line 9, in <module>
    import fcntl
ModuleNotFoundError: No module named 'fcntl'
(testvenv) PS C:\Users\mgrandi> python --version
Python 3.9.0
(testvenv) PS C:\Users\mgrandi> pip3 list
Package    Version
---------- -------
pex        2.1.24
pip        20.2.3
setuptools 49.2.1
WARNING: You are using pip version 20.2.3; however, version 20.3.3 is available.
You should consider upgrading via the 'c:\users\mgrandi\temp\testvenv\scripts\python.exe -m pip install --upgrade pip' command.
(testvenv) PS C:\Users\mgrandi> $PSVersionTable.PSVersion

Major  Minor  Build  Revision
-----  -----  -----  --------
5      1      19041  610


(testvenv) PS C:\Users\mgrandi> [environment]::OSVersion.Version

Major  Minor  Build  Revision
-----  -----  -----  --------
10     0      19042  0

@jsirois
Copy link
Member

jsirois commented Dec 30, 2020

@mgrandi unfortunately, this is expected:
https://github.com/pantsbuild/pex/blob/46adfb48ba53c8663dc34644dc8e04686f7b83a3/pyproject.toml#L13-L17

We currently have no contributors that use Windows which is likely the boost we need to get Windows CI set up to give us a green baseline to develop against and ensure we don't start providing Windows support while continually degrading. If you're willing to step in and champion a Windows support effort, that would be lovely. Besides CI setup, at a minimum work will be needed where you hit this error - which is file locking code and likely various bits of code that assume creating symlinks will work (IIUC they require special privileges on Windows).

@jsirois jsirois self-assigned this Dec 30, 2020
@jsirois jsirois closed this as completed Dec 30, 2020
@chrisjbremner
Copy link
Contributor

I have the same issue as OP. I had built my toolchain around pex==1.6.12, which worked on Windows, but then I ran into this issue when I tried updating to the latest version. My proposal would be to remove the import of fcntl and instead use portalocker, which is a cross-platform equivalent for locking (which from a quick search, looks like all that fcntl is used for in this package). I am happy to take a crack at implementing that in a new PR, but wanted to run it by you to make sure that it would be ok to introduce a new dependency.

@jsirois
Copy link
Member

jsirois commented Dec 30, 2020

Thanks for the anecdote @chrisjbremner and the fcntl alternative suggestion. Note though that Pex can never support Windows in good faith until it grows Windows CI jobs to ensure things stay green. If either of you wants to pitch in and get this all set up then we can do the 1st official release that adds Operating System :: Microsoft :: Windows ... classifiers.

@jsirois
Copy link
Member

jsirois commented Dec 30, 2020

Sorry @chrisjbremner missed your last sentences... Something like portalocker would be fine as long as its small, focused and well tested. It would be added and accessed via the vendoring system. So that means:

  1. Add a line here:
    https://github.com/pantsbuild/pex/blob/46adfb48ba53c8663dc34644dc8e04686f7b83a3/pex/vendor/__init__.py#L102-L129
  2. Run tox -evendor
  3. Use the code via from pex.third_party.portalocker import ...

Its not quite that simple since this library would be needed at both build-time and runtime though. So you'd also need to add the portalocker package to the list of runtime root module names here:
https://github.com/pantsbuild/pex/blob/bee95d65fcf7ad2beb75a923890de6b47f726b7d/pex/pex_builder.py#L493-L501

@chrisjbremner
Copy link
Contributor

The vendoring does throw a wrench in things a little bit as portalocker has a dependency of pywin32 (on Windows) -- if this were included as a vendored package, would that mean that all of the pywin32 source would be included in the generated .pex file, even in those built on Linux/Mac?

@jsirois
Copy link
Member

jsirois commented Dec 30, 2020

Yeah, that's not awesome. Maybe https://github.com/harlowja/fasteners ? That also has the pywin32 dependency, but opts to vendor in the small slices it needs.

@chrisjbremner
Copy link
Contributor

I'll try out a few as suggested here and report back

@jsirois
Copy link
Member

jsirois commented Dec 30, 2020

Alternatively - and I think preferrably - just add the windows-specific code. Pex only uses an exlusive write lock and that impl in fasteners does not require pywin32:
https://github.com/harlowja/fasteners/blob/a39ce891ca9c833b554cd806f2395ec43f017349/fasteners/process_lock.py#L413-L436

That code verbatim would not be exactly right since Pex does a blocking lock, but presumably some flags could be changed to get the desired result.

@chrisjbremner
Copy link
Contributor

chrisjbremner commented Dec 30, 2020

Do you know how long files are locked for? Would using LK_LOCK (which blocks for 10s) work? Otherwise, I imagine we could make it nonblocking and add a loop that checks the lock periodically (which is what filelock does), which would presumably more closely match the behavior of LOCK_EX from fcntl

@jsirois
Copy link
Member

jsirois commented Dec 30, 2020

The lock is forever - its a blocking write lock:
https://github.com/pantsbuild/pex/blob/46adfb48ba53c8663dc34644dc8e04686f7b83a3/pex/common.py#L404-L413

So some sort of loop over the lock attempt that allows for OSError will be needed it seems. It would be great if that OSError coud be narrowed down though via an errno - that's a disturbingly broad exception catch on the surface.

@chrisjbremner
Copy link
Contributor

It seems to manifest itself as a PermissionError when tested on my machine, which is slightly more specific than an OSError, although I cannot be confident that it'd always raise that. Anyways, I made a PR with the change here, I suppose we should continue discussion there?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants