Skip to content

Use sysconfig for Python module locations#5423

Merged
skef merged 10 commits intofontforge:masterfrom
iorsh:py_sysconfig
Oct 7, 2024
Merged

Use sysconfig for Python module locations#5423
skef merged 10 commits intofontforge:masterfrom
iorsh:py_sysconfig

Conversation

@iorsh
Copy link
Copy Markdown
Contributor

@iorsh iorsh commented May 24, 2024

distutils is deprecated with removal planned for Python 3.12.

  • Replace distutils with a similar functionality from sysconfig to avoid CI failure
  • Update macos CI runner to 12

This change is necessary to update CI to macos-12, which doesn't have Python 3.10.

TODO: Revert iorsh/fontforgebuilds repo before merge and after jtanx/fontforgebuilds#21 is merged into fontforgebuilds

@iorsh
Copy link
Copy Markdown
Contributor Author

iorsh commented May 24, 2024

Ok, appveyor is failing. I'll need to look into this with a separate dummy PR

@iorsh iorsh marked this pull request as draft May 24, 2024 14:26
@iorsh iorsh marked this pull request as ready for review May 24, 2024 15:55
@iorsh
Copy link
Copy Markdown
Contributor Author

iorsh commented May 24, 2024

Pointed appveyor to iorsh/fontforgebuilds repo, tests now pass

Comment thread .github/workflows/main.yml Outdated
# use the exact one discovered by CMake.
PYTHON=`cat build/CMake_Python3_EXECUTABLE`
PYTHONPATH=$PYTHONPATH:$PREFIX/$($PYTHON -c "import distutils.sysconfig as sc; print(sc.get_python_lib(prefix='', plat_specific=True,standard_lib=False))")
PYTHONPATH=$PYTHONPATH:$PREFIX/$($PYTHON -c "import sysconfig as sc; print(sc.get_path('platlib', sc.get_preferred_scheme('user'), vars={'userbase': '.'}))")
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comment as on the fontforgebuilds pr, I think this should be

Suggested change
PYTHONPATH=$PYTHONPATH:$PREFIX/$($PYTHON -c "import sysconfig as sc; print(sc.get_path('platlib', sc.get_preferred_scheme('user'), vars={'userbase': '.'}))")
PYTHONPATH=$PYTHONPATH:$PREFIX/$($PYTHON -c "import sysconfig as sc; print(sc.get_path('platlib', sc.get_preferred_scheme('prefix'), vars={'platbase': '.'}))")

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please see jtanx/fontforgebuilds#21 (comment)

Unfortunately 'prefix' key is inconsistent across platforms. On MacOS the preferred scheme for prefix is osx_framework_library, and its platlib entry looks like

'platlib': '/usr/local/{platlibdir}/python{py_version_short}/site-packages'

This forces absolute path (which is discouraged by CMake) and the variable names are different for each platform. You can check this at https://github.com/iorsh/fontforge/actions/runs/9223726161/ - see the "Run pyhook smoke test" entry for Mac and Linux

@jtanx jtanx force-pushed the py_sysconfig branch 2 times, most recently from f76eb53 to 7e3a2be Compare July 21, 2024 01:21
@iorsh
Copy link
Copy Markdown
Contributor Author

iorsh commented Sep 21, 2024

@jtanx, do you want any additional changes here? Could you please merge if this is ok with you?

Copy link
Copy Markdown
Contributor

@skef skef left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a little scary but I suppose since it's CI stuff the results should be self-evident. If you give me the go-ahead I'll merge.

@jtanx
Copy link
Copy Markdown
Contributor

jtanx commented Oct 6, 2024

Woops, forgot to reply to this one. I think there may have still been something but I've forgotten now, so if it builds, send it

@skef skef merged commit 8c75293 into fontforge:master Oct 7, 2024
@iorsh iorsh deleted the py_sysconfig branch December 23, 2024 22:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants