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.

Sign up for GitHub

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

Jump to bottom

Replace deprecated PythonInterp with Python3 #213

Merged
merged 2 commits into from
Mar 31, 2022
Merged

Replace deprecated PythonInterp with Python3 #213

merged 2 commits into from
Mar 31, 2022

Conversation

Bi0T1N
Copy link
Contributor

@Bi0T1N Bi0T1N commented Mar 19, 2022 •
edited
Loading

🦟 Bug fix

Fixes #212

Summary

PythonInterp is deprecated for CMake versions > 3.11 and all scripts use Python3 features anyway, so replace it with the recommended find_package(Python3) but also provide fallback solutions until the required CMake version can be bumped.
Note: Some small adjustments may be required in other packages to use the new variables. If the changes here are fine I will create the other PR's.

Checklist

  • Signed all commits for DCO
  • Added tests
  • Updated documentation (as needed)
  • Updated migration guide (as needed)
  • codecheck passed (See contributing)
  • All tests passed (See test coverage)
  • While waiting for a review on your PR, please help review another open pull request to support the maintainers

Note to maintainers: Remember to use Squash-Merge and edit the commit message to match the pull request summary while retaining Signed-off-by messages.

@j-rivero
Copy link
Contributor

Thanks for the PR @Bi0T1N! Since the changes are carefully maintaining current behavior I'm going to target the PR against the ign-cmake2 branch so we get the changes ready in a released branch, ready to be shipped in the next release.

@j-rivero j-rivero changed the base branch from main to ign-cmake2 March 21, 2022 19:41
@j-rivero j-rivero changed the base branch from ign-cmake2 to main March 21, 2022 19:41
@j-rivero
Copy link
Contributor

j-rivero commented Mar 21, 2022 •
edited
Loading

Thanks for the PR @Bi0T1N! Since the changes are carefully maintaining current behavior I'm going to target the PR against the ign-cmake2 branch so we get the changes ready in a released branch, ready to be shipped in the next release.

Changing PR base in the PR did not work well, keeping it against main. We'll do the other way around, We can backport it to ign-cmake2.

j-rivero
j-rivero reviewed Mar 22, 2022
View reviewed changes
Copy link
Contributor

@j-rivero j-rivero left a comment

Choose a reason for hiding this comment

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

I've been testing Ignition Fortress and Ignition Garden packages with this and seems to work nicely in both Ubuntu Focal (version 3.16) and Debian Sid (version 3.22).

I left one suggestion to change the input name of the variable, otherwise we are good to go.

cmake/IgnPython.cmake Outdated Show resolved Hide resolved
cmake/IgnPython.cmake Show resolved Hide resolved
This was referenced Mar 23, 2022
Merged
Open
j-rivero
j-rivero approved these changes Mar 23, 2022
View reviewed changes
Copy link
Contributor

@j-rivero j-rivero left a comment

Choose a reason for hiding this comment

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

Great work @Bi0T1N . Good to go.

@j-rivero j-rivero merged commit ee900d2 into gazebosim:main Mar 31, 2022
j-rivero pushed a commit that referenced this pull request Mar 31, 2022
@Bi0T1N @j-rivero
Replace deprecated PythonInterp with Python3 (#213)
9cb519f 
* Replace deprecated PythonInterp with Python3

Signed-off-by: Bi0T1N <[email protected]>
@j-rivero j-rivero mentioned this pull request Mar 31, 2022
Merged
j-rivero added a commit that referenced this pull request Mar 31, 2022
@j-rivero
Replace deprecated PythonInterp with Python3 (#213) (#218)
8718636 
* Replace deprecated PythonInterp with Python3

Signed-off-by: Bi0T1N <[email protected]>
@Bi0T1N Bi0T1N deleted the find_python3 branch April 2, 2022 15:54
harshmahesheka pushed a commit to harshmahesheka/ign-cmake that referenced this pull request Apr 5, 2022
@j-rivero @harshmahesheka
Replace deprecated PythonInterp with Python3 (gazebosim#213) (gazebos…
c0bfff7 
…im#218)

* Replace deprecated PythonInterp with Python3

Signed-off-by: Bi0T1N <[email protected]>
Signed-off-by: Harsh Mahesheka <[email protected]>
@osrf-triage
Copy link

This pull request has been mentioned on Gazebo Community. There might be relevant details there:

https://community.gazebosim.org/t/new-releases-2022-04-27-fortress-citadel/1389/1

@azeey azeey mentioned this pull request Oct 5, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@j-rivero j-rivero j-rivero approved these changes

@mxgrey mxgrey Awaiting requested review from mxgrey

@adlarkin adlarkin Awaiting requested review from adlarkin

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Bi0T1N @j-rivero @osrf-triage