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

Use FindPython3 instead of FindPythonInterp #7

Merged
merged 11 commits into from
Feb 11, 2024
Merged

Conversation

sloretz
Copy link
Contributor

@sloretz sloretz commented Jul 26, 2021

First pass at #6 - marking as draft because I've learned a few prerequisites

Also: I haven't tested if find_package(Python3 REQUIRED COMPONENTS ...) can be found multiple times with additional comments - if so then the documentation in this PR is incorrect. I've tested it now, and FindPython3 can be found multiple times to get more components, so this PR can be a little bit simpler :)

Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
Signed-off-by: Chris Lalancette <clalancette@gmail.com>
Signed-off-by: Chris Lalancette <clalancette@gmail.com>
@clalancette
Copy link
Contributor

clalancette commented Nov 16, 2023

All right, I've rebased this onto the latest, and also updated it to export the PYTHON_EXECUTABLE and PYTHON_EXECUTABLE_DEBUG variables. That seems to keep most downstream things working, and succeeds in all my local tests. I'm going to run a full-up CI on this along with ros2/rosidl_python#140 to see what happens:

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Windows Build Status

Signed-off-by: Chris Lalancette <clalancette@gmail.com>
Signed-off-by: Chris Lalancette <clalancette@gmail.com>
@clalancette
Copy link
Contributor

I also pushed a fix for Windows, so let's see what happens now:

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Windows Build Status

@clalancette
Copy link
Contributor

Let's see what happens with Windows Debug, RHEL, and clang:

  • Windows Debug Build Status
  • RHEL Build Status
  • Clang Build Status

@clalancette
Copy link
Contributor

Here's another try with the more recent code:

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Windows Build Status
  • Windows Debug Build Status
  • RHEL Build Status
  • Clang Build Status

@clalancette
Copy link
Contributor

clalancette commented Jan 3, 2024

I've put a new fix in place in rosidl_python, which should fix Windows Debug. Here's another CI try:

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Windows Build Status
  • Windows Debug Build Status
  • RHEL Build Status
  • Clang Build Status

@clalancette
Copy link
Contributor

clalancette commented Jan 4, 2024

One more CI with an additional fix in rosidl_python:

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Windows Build Status
  • Windows Debug Build Status
  • RHEL Build Status
  • Clang Build Status

@clalancette
Copy link
Contributor

OK, I think I've now finally licked this on all platforms. This should get rid of our warnings. @sloretz if you get a chance, I'd appreciate a review from you on this and ros2/rosidl_python#140 , since I have made some substantial changes since you last opened it.

@clalancette clalancette marked this pull request as ready for review January 5, 2024 01:20
Signed-off-by: Chris Lalancette <clalancette@gmail.com>
@clalancette
Copy link
Contributor

Here's new Windows Debug CI, which is the one most likely to have an issue:

  • Windows Debug Build Status

Signed-off-by: Chris Lalancette <clalancette@gmail.com>
@clalancette
Copy link
Contributor

clalancette commented Jan 26, 2024

CI again:

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Windows Build Status
  • Windows Debug Build Status
  • RHEL Build Status
  • Clang Build Status

Signed-off-by: Chris Lalancette <clalancette@gmail.com>
@clalancette
Copy link
Contributor

Another fix for Windows is in, here is CI again:

CI again:

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Windows Build Status
  • Windows Debug Build Status
  • RHEL Build Status
  • Clang Build Status

@sloretz
Copy link
Contributor Author

sloretz commented Feb 2, 2024

Running --packages-up-to tf2_py with --trace-expand to try to figure out what's causing the test failures in windows debug: Build Status

This simplifies things so we no longer need a PythonExtra::Interpreter.

Signed-off-by: Chris Lalancette <clalancette@gmail.com>
@clalancette
Copy link
Contributor

clalancette commented Feb 9, 2024

After the latest changes, here's another round of CI:

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Windows Build Status
  • Windows Debug Build Status

@sloretz
Copy link
Contributor Author

sloretz commented Feb 9, 2024

cmake/Modules/FindPythonExtra.cmake Outdated Show resolved Hide resolved
cmake/Modules/FindPythonExtra.cmake Outdated Show resolved Hide resolved
cmake/Modules/FindPythonExtra.cmake Outdated Show resolved Hide resolved
Signed-off-by: Chris Lalancette <clalancette@gmail.com>
Copy link
Contributor Author

@sloretz sloretz left a comment

Choose a reason for hiding this comment

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

LGTM

@clalancette
Copy link
Contributor

clalancette commented Feb 9, 2024

I put a linting fix in place, and fixed the review comments. Here is another round of CI:

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Windows Build Status
  • Windows Debug Build Status

@clalancette
Copy link
Contributor

Here's another try that includes ros-visualization/rqt_plot#93 and ros-visualization/rqt_bag#151:

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Windows Build Status
  • Windows Debug Build Status

@clalancette
Copy link
Contributor

And just to complete the set, here is RHEL and clang:

  • RHEL Build Status
  • Clang Build Status

@clalancette clalancette merged commit a3db97e into rolling Feb 11, 2024
2 checks passed
@clalancette clalancette deleted the use_FindPython3 branch February 11, 2024 17:41
@clalancette clalancette mentioned this pull request Feb 11, 2024
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.

2 participants