This is an archive of the discontinued Mercurial Phabricator instance.

Differential D7444

setup: conditionalize access to `sys.dllhandle` when building extensions
ClosedPublic

Authored by mharbison72 on Nov 16 2019, 12:29 PM.

Details

Reviewers
indygreg
Group Reviewers
hg-reviewers
Commits
rHG52e4bfebc4ba: setup: conditionalize access to `sys.dllhandle` when building extensions
rHG10875cf85f22: setup: conditionalize access to `sys.dllhandle` when building extensions
Summary

This code is only run on Windows, and was crashing PyOxidizer when running in
setup-py-install mode. Now an oxidized binary can be built by simply pointing
to setup.py.

Something is slightly different now that it's not being built from a virtualenv.
Previously, hg version could print to the screen, but now it aborts saying
"Incorrect function". But I can see the output if redirected to a file, and
it's not complaining about missing C extensions, so I think those are loading
now (unlike from the virtualenv). The interesting this about this incorrect
function output is that it failed when initially built. I then went back and
did a make clean and make local with py3 and then py2 to ensure I didn't
break the existing code. At that point I ran the oxidized executable again and
it was able to print to the screen normally! So I ran pyoxidizer build again,
it only output the following, and then running the executable failed to output
again:

(pyO2_venv) C:\Users\Matt\hg3\hg_pyO2>pyoxidizer build
    Finished dev [unoptimized + debuginfo] target(s) in 0.12s
packaging application into C:/Users/Matt/hg3/hg_pyO2\build\apps\hg_pyO2\x86_64-pc-windows-msvc\debug
purging C:/Users/Matt/hg3/hg_pyO2\build\apps\hg_pyO2\x86_64-pc-windows-msvc\debug
copying C:/Users/Matt/hg3/hg_pyO2\build\target\x86_64-pc-windows-msvc\debug\hg_pyO2.exe to
    C:/Users/Matt/hg3/hg_pyO2\build\apps\hg_pyO2\x86_64-pc-windows-msvc\debug\hg_pyO2.exe
resolving packaging state...
writing license for [...]
hg_pyO2 packaged into C:/Users/Matt/hg3/hg_pyO2\build\apps\hg_pyO2\x86_64-pc-windows-msvc\debug
executable path: C:/Users/Matt/hg3/hg_pyO2\build\apps\hg_pyO2\x86_64-pc-windows-msvc\debug\hg_pyO2.exe

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

mharbison72 created this revision.Nov 16 2019, 12:29 PM
indygreg accepted this revision.Nov 16 2019, 2:19 PM
indygreg added a subscriber: indygreg.

This workaround is fine for PyOxidizer. But pythonlib and hgpythonlib.h and exewrapper.c aren't needed for PyOxidizer. So there is further room to make large parts of this code conditional on running in PyOxidizer. The path I was going in my patch was to have setup.py look for an environment variable that identified the current build environment as PyOxidizer and tweak behavior appropriately.

I do like the approach of making this patch agnostic of PyOxidizer, as other environments could also lack a sys.dllhandle. And arguably the proper way to disable the building of the exe wrapper is to add an argument to our setup.py to control it. PyOxidizer could then pass in this argument.

As for the output issues, I'm not sure what's going on. I added support for Py_LegacyWindowsStdioFlag to PyOxidizer a few hours ago. I'd start there. Feel free to make noise on PyOxidizer's issue tracker!

This revision is now accepted and ready to land.Nov 16 2019, 2:19 PM
mharbison72 added a comment.Nov 16 2019, 9:00 PM
In D7444#109357, @indygreg wrote:

As for the output issues, I'm not sure what's going on. I added support for Py_LegacyWindowsStdioFlag to PyOxidizer a few hours ago. I'd start there. Feel free to make noise on PyOxidizer's issue tracker!

That did the trick, and now I've got a usable (so far) oxidized binary.

What do you think about this in hg debuginstall:

  File "mercurial.debugcommands", line 1486, in debuginstall
    os.path.dirname(pycompat.fsencode(os.__file__)),
AttributeError: module 'os' has no attribute '__file__'

That's not really a resource, so just special case it to print the exe? (This is trying to print the equivalent of this: checking Python lib (C:\Program Files\Python37\lib)...) There's also similar not-quite-resources use of file in service of hg extensions, for example.

indygreg added a comment.Nov 16 2019, 9:09 PM

PyOxidizer doesn't set __file__ for modules imported from memory. You can work around by installing the Python standard library in an app-relative directory (like we do for the Mercurial modules in my proof-of-concept PyOxidizer patch) or you can teach Mercurial to react gracefully when __file__ isn't defined. The debuginstall use of __file__ is pretty dubious and not critical to behavior, so I highly favor making it behave better when __file__ is missing.

Revision Contents
Changeset List

PathPackages
Msetup.py (50 lines)

Diff 18188

setup.py

def build_extensions(self): def build_extensions(self):
if os.name != 'nt': if os.name != 'nt':
return return
if isinstance(self.compiler, HackedMingw32CCompiler): if isinstance(self.compiler, HackedMingw32CCompiler):
self.compiler.compiler_so = self.compiler.compiler # no -mdll self.compiler.compiler_so = self.compiler.compiler # no -mdll
self.compiler.dll_libraries = [] # no -lmsrvc90 self.compiler.dll_libraries = [] # no -lmsrvc90
pythonlib = None
if getattr(sys, 'dllhandle', None):
# Different Python installs can have different Python library # Different Python installs can have different Python library
# names. e.g. the official CPython distribution uses pythonXY.dll # names. e.g. the official CPython distribution uses pythonXY.dll
# and MinGW uses libpythonX.Y.dll. # and MinGW uses libpythonX.Y.dll.
_kernel32 = ctypes.windll.kernel32 _kernel32 = ctypes.windll.kernel32
_kernel32.GetModuleFileNameA.argtypes = [ _kernel32.GetModuleFileNameA.argtypes = [
ctypes.c_void_p, ctypes.c_void_p,
ctypes.c_void_p, ctypes.c_void_p,
ctypes.c_ulong, ctypes.c_ulong,
] ]
_kernel32.GetModuleFileNameA.restype = ctypes.c_ulong _kernel32.GetModuleFileNameA.restype = ctypes.c_ulong
size = 1000 size = 1000
buf = ctypes.create_string_buffer(size + 1) buf = ctypes.create_string_buffer(size + 1)
filelen = _kernel32.GetModuleFileNameA( filelen = _kernel32.GetModuleFileNameA(
sys.dllhandle, ctypes.byref(buf), size sys.dllhandle, ctypes.byref(buf), size
) )
if filelen > 0 and filelen != size: if filelen > 0 and filelen != size:
dllbasename = os.path.basename(buf.value) dllbasename = os.path.basename(buf.value)
if not dllbasename.lower().endswith(b'.dll'): if not dllbasename.lower().endswith(b'.dll'):
raise SystemExit( raise SystemExit(
'Python DLL does not end with .dll: %s' % dllbasename 'Python DLL does not end with .dll: %s' % dllbasename
) )
pythonlib = dllbasename[:-4] pythonlib = dllbasename[:-4]
else:
if not pythonlib:
log.warn( log.warn(
'could not determine Python DLL filename; assuming pythonXY' 'could not determine Python DLL filename; assuming pythonXY'
) )
hv = sys.hexversion hv = sys.hexversion
pythonlib = b'python%d%d' % (hv >> 24, (hv >> 16) & 0xFF) pythonlib = b'python%d%d' % (hv >> 24, (hv >> 16) & 0xFF)
log.info('using %s as Python library name' % pythonlib) log.info('using %s as Python library name' % pythonlib)