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

Drop python3-distutils from Noble package list (onwards) #2374

Merged
merged 8 commits into from
Apr 19, 2024

Conversation

j-rivero
Copy link
Contributor

🎉 New feature

Summary

There is no python3-distutils package anymore in Ubuntu starting with Noble. Drop it and keep a copy of current packages for Jammy.

Checklist

  • Signed all commits for DCO
  • Added tests
  • Added example and/or tutorial
  • Updated documentation (as needed)
  • Updated migration guide (as needed)
  • Consider updating Python bindings (if the library has them)
  • 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.

Signed-off-by: Jose Luis Rivero <[email protected]>
@j-rivero j-rivero requested a review from mjcarroll as a code owner April 17, 2024 17:31
@github-actions github-actions bot added the 🎵 harmonic Gazebo Harmonic label Apr 17, 2024
@azeey
Copy link
Contributor

azeey commented Apr 17, 2024

AFAIK, the only place we need it is in

if(USE_SYSTEM_PATHS_FOR_PYTHON_INSTALLATION)
if(${CMAKE_VERSION} VERSION_LESS "3.12.0")
execute_process(
COMMAND "${PYTHON_EXECUTABLE}" -c "if True:
from distutils import sysconfig as sc
print(sc.get_python_lib(plat_specific=True))"
which is for cmake<3.12. Jammy has a much newer cmake, so we can drop it altogether.

@scpeters
Copy link
Member

scpeters commented Apr 18, 2024

AFAIK, the only place we need it is in

if(USE_SYSTEM_PATHS_FOR_PYTHON_INSTALLATION)
if(${CMAKE_VERSION} VERSION_LESS "3.12.0")
execute_process(
COMMAND "${PYTHON_EXECUTABLE}" -c "if True:
from distutils import sysconfig as sc
print(sc.get_python_lib(plat_specific=True))"

which is for cmake<3.12. Jammy has a much newer cmake, so we can drop it altogether.

attempting to drop it from gz-math8 in gazebosim/gz-math#587

@j-rivero
Copy link
Contributor Author

AFAIK, the only place we need it is in

You are right, aebfe55

Don't need it anymore

Signed-off-by: Jose Luis Rivero <[email protected]>
CMakeLists.txt Outdated Show resolved Hide resolved
# Python3_SITEARCH is available from 3.12 on, workaround if needed:
find_package(Python3 COMPONENTS Interpreter)
endif()
find_package(Python3 COMPONENTS Interpreter)
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we can leave the code as is. It's technically possible for someone to build this with cmake 3.10. But we know for sure our CI will have a newer version of cmake.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

reverted

@j-rivero j-rivero changed the title Drop python3-disutils from Noble package list (onwards) Drop python3-distutils from Noble package list (onwards) Apr 19, 2024
Signed-off-by: Jose Luis Rivero <[email protected]>
@j-rivero j-rivero force-pushed the jrivero/gz_sim8_noble_apt_packages branch from 1d9e211 to 7918121 Compare April 19, 2024 13:04
@j-rivero j-rivero requested review from azeey and scpeters April 19, 2024 13:06
@@ -0,0 +1 @@
python3-distutils
Copy link
Contributor

Choose a reason for hiding this comment

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

Are you opposed to removing python3-distutils from any of the .apt files? I think it would be fine since Jammy has a newer version of cmake.

Copy link
Member

Choose a reason for hiding this comment

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

I think it would be fine to remove for Harmonic since 22.04 has cmake 3.22.1

Copy link
Contributor Author

Choose a reason for hiding this comment

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

+1. Sorry did not see the whole picture before. Removed.

Signed-off-by: Jose Luis Rivero <[email protected]>
@j-rivero j-rivero enabled auto-merge (squash) April 19, 2024 16:57
@j-rivero j-rivero merged commit 363f582 into gz-sim8 Apr 19, 2024
6 of 8 checks passed
@j-rivero j-rivero deleted the jrivero/gz_sim8_noble_apt_packages branch April 19, 2024 17:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🎵 harmonic Gazebo Harmonic
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants