Escape $ENV{DESTDIR} everywhere in ament_python_install_package()#324
Escape $ENV{DESTDIR} everywhere in ament_python_install_package()#324clalancette merged 1 commit intomasterfrom
Conversation
Follow up after f80071e Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
|
Running CI up to
Applying this change to Rolling debian sources and running |
| install(CODE | ||
| "set(extra_install_args ${extra_install_args}) | ||
| set(install_dir \"${CMAKE_INSTALL_PREFIX}/${PYTHON_INSTALL_DIR}\") | ||
| if(DEFINED ENV{DESTDIR} AND NOT \"$ENV{DESTDIR}\" STREQUAL \"\") | ||
| if(DEFINED ENV{DESTDIR} AND NOT \"\$ENV{DESTDIR}\" STREQUAL \"\") | ||
| list(APPEND extra_install_args --root \$ENV{DESTDIR}) | ||
| file(TO_CMAKE_PATH \"\$ENV{DESTDIR}/\${install_dir}\" install_dir) | ||
| endif() |
There was a problem hiding this comment.
So just to make sure I understand what is going on here:
According to the install documentation, this form of install invokes the given CMake code during installation. Because of that, any cmake variables that you want expanded at install time need to be escaped. In this code snippet (and below) we are expecting that extra_install_args, CMAKE_INSTALL_PREFIX, PYTHON_INSTALL_DIR, package_name, PYTHON_EXECUTABLE, and build_dir are expanded at ament_python_install_package.cmake invocation time. We are expecting ENV{DESTDIR}, install_dir, and install_prefix to be expanded at install time. Is that correct?
There was a problem hiding this comment.
Exactly. Well, almost. There's scripts_install_dir and another extra_install_args being expanded at install time. The latter is perhaps a bit annoying, I'm not super creative with variable names.
Without this extra escape, one $ENV{DESTDIR} was resolved in CMake configuration time to nothing, which resulted in no --root given to python setup.py
|
CI's green, this is ready to roll |
Follow up after #323. Massive 🤦