test and fix tkinter initialization path

mattip commented 2 years ago

Checklist

[ ] Used a personal fork of the feedstock to propose changes
[ ] Bumped the build number (if the version is unchanged)
[ ] Reset the build number to 0 (if the version changed)
[ ] Re-rendered with the latest conda-smithy (Use the phrase code>@<space/conda-forge-admin, please rerender in a comment in this PR for automated rerendering)
[ ] Ensured the license file is being packaged.

Fixes #80 and adds a test to make sure tkinter works.

conda-forge-linter commented 2 years ago

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

mattip commented 2 years ago

@conda-forge-admin, please rerender

github-actions[bot] commented 2 years ago

Hi! This is the friendly automated conda-forge-webservice.

I tried to rerender for you, but it looks like there was nothing to do.

This message was generated by GitHub actions workflow run https://github.com/conda-forge/pypy3.6-feedstock/actions/runs/2147526746.

mattip commented 2 years ago

Builds on linux are failing to run 'import tkinter; tkinter.Tk()' because they are missing xorg-libx11. If that is a requirement for tk, why doesn't installing tk install xorg-libx11?

mattip commented 2 years ago

When I check the dependencies of the module produced by this feedstock I get

% otool -L ~/mambaforge-pypy3/envs/pypy38/lib/pypy3.8/_tkinter/tklib_cffi.pypy38-pp73-darwin.so
/Users/matti/mambaforge-pypy3/envs/pypy38/lib/pypy3.8/_tkinter/tklib_cffi.pypy38-pp73-darwin.so:
    ./_tkinter/tklib_cffi.pypy38-pp73-darwin.so (compatibility version 0.0.0, current version 0.0.0)
    /System/Library/Frameworks/Tcl.framework/Versions/8.5/Tcl (compatibility version 8.5.0, current version 8.5.9)
    /System/Library/Frameworks/Tk.framework/Versions/8.5/Tk (compatibility version 8.5.0, current version 8.5.9)
    /usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1197.1.1)

But RPATH in the pypy-packaged so is set during the packaging.py step and is reported in the logs:

Set RPATH of lib/pypy3.8/_tkinter/tklib_cffi.pypy38-pp73-darwin.so to @executable_path/../..

Diving deeper into the so I see that the LC_LOAD_DYLIB is set before the LC_RPATH.

% otool -l ~/mambaforge-pypy3/envs/pypy38/lib/pypy3.8/_tkinter/tklib_cffi.pypy38-pp73-darwin.so
/Users/matti/mambaforge-pypy3/envs/pypy38/lib/pypy3.8/_tkinter/tklib_cffi.pypy38-pp73-darwin.so:
...
Load command 8
      cmd LC_VERSION_MIN_MACOSX
  cmdsize 16
  version 10.9
      sdk n/a
Load command 9
      cmd LC_SOURCE_VERSION
  cmdsize 16
  version 0.0
Load command 10
          cmd LC_LOAD_DYLIB
      cmdsize 88
         name /System/Library/Frameworks/Tcl.framework/Versions/8.5/Tcl (offset 24)
   time stamp 2 Thu Jan  1 02:00:02 1970
      current version 8.5.9
compatibility version 8.5.0
Load command 11
          cmd LC_LOAD_DYLIB
      cmdsize 80
         name /System/Library/Frameworks/Tk.framework/Versions/8.5/Tk (offset 24)
   time stamp 2 Thu Jan  1 02:00:02 1970
      current version 8.5.9
compatibility version 8.5.0
Load command 12
          cmd LC_LOAD_DYLIB
      cmdsize 56
         name /usr/lib/libSystem.B.dylib (offset 24)
   time stamp 2 Thu Jan  1 02:00:02 1970
      current version 1197.1.1
compatibility version 1.0.0
Load command 13
      cmd LC_FUNCTION_STARTS
  cmdsize 16
  dataoff 22168
 datasize 88
Load command 14
      cmd LC_DATA_IN_CODE
  cmdsize 16
  dataoff 22256
 datasize 0
Load command 15
          cmd LC_RPATH
      cmdsize 40
         path @executable_path/../.. (offset 12)
Load command 16
          cmd LC_RPATH
      cmdsize 32
         path @loader_path/../../ (offset 12)

So now I need to figure out why the Framework is found in PyPy but not in CPython.

mattip commented 2 years ago

We heavily patch the build script tklib_build.py, the final version looks like:

...
# XXX find a better way to detect paths
# XXX pick up CPPFLAGS and LDFLAGS and add to these paths?
if sys.platform.startswith("openbsd"):
    incdirs = ['/usr/local/include/tcl8.5', '/usr/local/include/tk8.5', '/usr/X11R6/include']
    linklibs = ['tk85', 'tcl85']
    libdirs = ['/usr/local/lib', '/usr/X11R6/lib']
elif sys.platform.startswith("freebsd"):
    incdirs = ['/usr/local/include/tcl8.6', '/usr/local/include/tk8.6', '/usr/local/include/X11', '/usr/local/include']
    linklibs = ['tk86', 'tcl86']
    libdirs = ['/usr/local/lib']
elif sys.platform == 'win32':
    incdirs = []
    linklibs = ['tcl86t', 'tk86t']
    libdirs = []
elif sys.platform == 'darwin':
    incdirs = []
    linklibs = ['tcl', 'tk']
    libdirs = []
else:
    incdirs = []
    linklibs = ['tcl8.6', 'tk8.6']
    libdirs = []
...
tkffi = FFI()

tkffi.cdef("""
...
""")

tkffi.set_source("_tkinter.tklib_cffi", """
...
""",
include_dirs=incdirs,
libraries=linklibs,
library_dirs = libdirs
)

tkffi.compile()

On macOS this sets no libdirs, so we are dependent on the conda environment to properly set up the compiler to link to the conda libraries. It seems this is not happening, and the linker prefers the system Framework.

mattip commented 2 years ago

Ahh, no, maybe it needs linklibs = ['tcl8.6', 'tk8.6'] ?

mattip commented 2 years ago

This seems to be working, and on macOS and windows now tests tkinter.Tk(). It removed some extra work done to build portable PyPy, (which is not needed on conda), and properly links to the tk/tcl libs on macOS.

Anyone wish to review?

conda-forge / pypy3.6-feedstock

test and fix tkinter initialization path #81