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

[cmake] Create FindPythonInterpreter for host information #23877

Merged
merged 1 commit into from Oct 24, 2023

Conversation

fuzzard
Copy link
Contributor

@fuzzard fuzzard commented Oct 5, 2023

Description

Simplify existing FindPython and split host/target specifics

Motivation and context

This splits out host find information from the regular target find module. This allows us to not mix and check for host executable status, and only explicitly search for the host interpreter when required (eg ENABLE_EVENTCLIENTS=ON)
I will mention, this can NOT be a TARGET like everything else im heading towards for the fact that execute_process() can not use TARGETS. Its considered too low level, so a cache variable is the only way when combined with execute_process(). It could be changed to an add_custom_command, but i dont think its worthwhile for a single edge use case

@wsnipex ive been tinkering with the FindPython module, and moving the host stuff out would simplify my life greatly.
Ive got down the PLATFORM_REQUIRED_BUILDTOOLS route here (similar to req/optional dep platform variables), however i thought a little about this later, and it may just be simpler to make an explicit call

core_require_dep(PythonInterpreter)

at the following

if(ENABLE_EVENTCLIENTS)
execute_process(COMMAND ${PYTHON_EXECUTABLE} -c "from distutils.sysconfig import get_python_lib; print(get_python_lib(prefix=''))"
OUTPUT_VARIABLE PYTHON_LIB_PATH OUTPUT_STRIP_TRAILING_WHITESPACE)
# Install kodi-eventclients-common BT python files

What are your thoughts?

How has this been tested?

locally macos with forcing certain paths to trigger the FindPythonInterpreter call

What is the effect on users?

N/A

Screenshots (if appropriate):

Types of change

  • Bug fix (non-breaking change which fixes an issue)
  • Clean up (non-breaking change which removes non-working, unmaintained functionality)
  • Improvement (non-breaking change which improves existing functionality)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that will cause existing functionality to change)
  • Cosmetic change (non-breaking change that doesn't touch code)
  • Student submission (PR was done for educational purposes and will be treated as such)
  • None of the above (please explain below)

Checklist:

  • My code follows the Code Guidelines of this project
  • My change requires a change to the documentation, either Doxygen or wiki
  • I have updated the documentation accordingly
  • I have read the Contributing document
  • I have added tests to cover my change
  • All new and existing tests passed

@fuzzard fuzzard added this to the Omega 21.0 Beta 1 milestone Oct 5, 2023
@fuzzard fuzzard requested a review from wsnipex October 5, 2023 09:40
@fuzzard fuzzard force-pushed the cmake_findpythoninterp_host branch 2 times, most recently from 813aec4 to a5450d7 Compare October 24, 2023 04:14
This splits out host find information from the regular target find module.
This allows us to not mix and check for host executable status, and only explicitly
search for the host interpreter when required (eg ENABLE_EVENTCLIENTS=ON)
@fuzzard
Copy link
Contributor Author

fuzzard commented Oct 24, 2023

Have simplified this. We only need the find_package call to be made when ENABLE_EVENTLCLIENTS is done as part of the linux install.

@fuzzard fuzzard closed this Oct 24, 2023
@fuzzard fuzzard reopened this Oct 24, 2023
Copy link
Member

@garbear garbear left a comment

Choose a reason for hiding this comment

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

Splitting host and target is a good idea. I took at look at the new CMake file and didn't see any problems.

@fuzzard fuzzard merged commit bd91412 into xbmc:master Oct 24, 2023
2 checks passed
@fuzzard fuzzard deleted the cmake_findpythoninterp_host branch October 24, 2023 23:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants