paulscherrerinstitute / pcaspy

Portable Channel Access Server in Python
BSD 3-Clause "New" or "Revised" License
32 stars 24 forks source link

Fix python38 windows build #65

Closed Tom-Willemsen closed 4 years ago

Tom-Willemsen commented 4 years ago

Only copy DLLs if they're not already present in the build.

This is similar to https://github.com/CaChannel/CaChannel/pull/4 and fixes pip installs under windows 10 and python 3.8

xiaoqiangwang commented 4 years ago

Now if I look at this issue, wouldn't be more reliable to use the following logic (pseudo code)

if dll_dest exists:
  remove dll_dest
copy dll to dll_dest

This avoid stale dlls being packaged in this scenario: firstly it is built with base 3.15.5 and then it is built with base 3.15.6 again. Oops, the new versions are not used.

Tom-Willemsen commented 4 years ago

Hi @xiaoqiangwang , I originally thought that too, but that approach leads to a permission error as the DLL is in use when you attempt to delete it - for the same reason that prevents the copy from working in the first place. I also saw this when I made the equivalent PR on CaChannel.

Here is a traceback from python.exe -m pip install git+git://github.com/IsisComputingGroup/pcaspy@d21f07ae4d7825b8be5b441acf6fd92602149d66, which uses this branch and implements the change you suggested:

Running setup.py install for pcaspy ... error
    ERROR: Command errored out with exit status 1:
     command: 'c:\Instrument\Apps\python3\python.exe' -u -c 'import sys, setuptools, tokenize; sys.argv[0] = '"'"'C:\\Users\\myusername\\AppData\\Local\\Temp\\pip-req-build-0h_xfb2_\\setup.py'"'"'; __file__='"'"'C:\\Users\\myusername\\AppData\\Local\\Temp\\pip-req-build-0h_xfb2_\\setup.py'"'"';f=getattr(tokenize, '"'"'open'"'"', open)(__file__);code=f.read().replace('"'"'\r\n'"'"', '"'"'\n'"'"');f.close();exec(compile(code, __file__, '"'"'exec'"'"'))' install --record 'C:\Users\myusername\AppData\Local\Temp\pip-record-iluluetw\install-record.txt' --single-version-externally-managed --compile
         cwd: C:\Users\myusername\AppData\Local\Temp\pip-req-build-0h_xfb2_\
    Complete output (5 lines):
    Traceback (most recent call last):
      File "<string>", line 1, in <module>
      File "C:\Users\myusername\AppData\Local\Temp\pip-req-build-0h_xfb2_\setup.py", line 96, in <module>
        os.remove(dll_dest)
    PermissionError: [WinError 5] Access is denied: 'C:\\Users\\myusername\\AppData\\Local\\Temp\\pip-req-build-0h_xfb2_\\pcaspy\\cas.dll'
    ----------------------------------------

I think the copy is getting executed twice per install, and the second time it doesn't work on windows as it is still "in use" by the first execution. This stackoverflow post seems to suggest checking for sys.argv[1] == "install" but that feels like a bit of a hack...

xiaoqiangwang commented 4 years ago

The use case to trigger this problem is when building/testing inplace. Weighing between the convenience and potential problems of stale dlls, I favor to avoid the problem.

In any case, you can still test without installation by pointing your PYTHONPATH to build/lib.win-amd64-3.7.

xiaoqiangwang commented 4 years ago

The check of sys.argv[1] == "install" is insufficient for cases like python setup.py bdist_wheel. In those cases the install is invoked as part of the building script.

Tom-Willemsen commented 4 years ago

OK, I think the above commit should now cover the case of building in-place with potentially stale DLLs in the build directory, and without relying on hacky argv parsing. Are there any other cases we need to consider?

xiaoqiangwang commented 4 years ago

Thanks. That will do.