Get Python 3.8.2 in Windows Dockerfile for Foxy

[jacobperron]

This is an attempt to switch to Python 3.8.2 for our Windows CI.
This is the version of Python we propose for Foxy ros-infrastructure/rep#217

This is a draft PR for two reasons:

1. It doesn't actually work. There errors coming from rclpy of the nature:

    E   ImportError: DLL load failed while importing _rclpy: The specified module could not be found.
   

   It's not clear to me what DLL's are failing to load. Here's an example build: https://ci.ros2.org/job/ci_windows/10081

2. Should we have an option in our CI config to continue building with Python 3.7.6?
   I think the answer is "yes", so that we can continue building for Dashing and Eloquent.
   I haven't looked closely, but I think it should be easy to pass in an ARG to the Dockerfile for the Python version.

Feedback on one or both of the above points is appreciated.

[brawner]

I don't have immediate input on issue 1. You're not seeing that error in focal?

For part 2, it might be better to support separate dockerfiles for different releases. We started doing that with a VS2017/2019 dockerfile split, but after crystal went eol, we dropped 2017. Perhaps instead we have dashing/eloquent/foxy splits of the dockerfiles?

[jacobperron]

You're not seeing that error in focal?

Our nightlies are using Focal and we don't see the error.

Perhaps instead we have dashing/eloquent/foxy splits of the dockerfiles?

Sounds fine to me. I guess there may be significant overlap, but it's probably the easiest thing to do.

[nuclearsandwich]

I have rebased this PR onto master which has now incorporated #450 and #451 so that this change now targets the Foxy-specific Windows Dockerfile.

[nuclearsandwich]

Trying a build just up to rclpy to see what the damage is there.

[jacobperron]

So, it turns out in Python 3.8 DLLs are loaded differently.

DLL dependencies for extension modules and DLLs loaded with ctypes on Windows are now resolved more securely. Only the system paths, the directory containing the DLL or PYD file, and directories added with add_dll_directory() are searched for load-time dependencies. Specifically, PATH and the current working directory are no longer used, and modifications to these will no longer have any effect on normal DLL resolution. If your application relies on these mechanisms, you should check for add_dll_directory() and if it exists, use it to add your DLLs directory while loading your library

https://docs.python.org/3/whatsnew/3.8.html#bpo-36085-whatsnew

We're appending package bin directories (containing DLLs) to PATH, but Python no longer uses PATH for DLL resolution.

After messing around with the new os.add_dll_directory API, I was able to call rclpy.init successfully. I had to call add_dll_directory for every recursive DLL dependency of _rclpy.pyd.

Here's some example code that uses the new add_dll_directory() function, which seems to work:

import os

import rclpy

dll_dir_handles = []

# We could probably just use PATH here
ament_prefix_path = os.environ['AMENT_PREFIX_PATH'].split(';')
for prefix_path in ament_prefix_path:
    # Assuming all DLLs are in 'bin'
    prefix_path_bin = os.path.join(prefix_path, 'bin')
    if os.path.exists(prefix_path_bin):
        # New in Python 3.8: 'add_dll_directory()'
        dll_dir_handles.append(os.add_dll_directory(prefix_path_bin))

rclpy.init()

for handle in dll_dir_handles:
    handle.close()

I think the next step is to figure out a good way to integrate the call to add_dll_library() in rclpy.

[jacobperron]

Testing rclpy and demo_nodes_py with ros2/rclpy#558:

[jacobperron]

Looks like the rclpy patch didn't fix things; I'll try out some more things locally.

[jacobperron]

I also had to patch rosidl_generator_py (ros2/rosidl_python#113). Now the build passes (rclpy and demo_nods_py) with Python 3.8:

[jacobperron]

Full CI with all connected PRs:

Edit: Look like similar logic needs to be applied to tf2_ros as well. Other than that the other test failures and warnings also occur on the nightlies.

[dirk-thomas]

Instead of all PRs implementing the same non-trivial logic why not provide a context manager implementing the logic which is then used in all these places?

[nuclearsandwich]

Instead of all PRs implementing the same non-trivial logic why not provide a context manager implementing the logic which is then used in all these places?

And thus begins the creation tale of rpyutils. 😉

[jacobperron]

Since the logic relies on the directories being retrieved from PATH and PATH is set by the template provided by ament_package, a common implementation could live in ament_package. Thoughts?

[jacobperron]

And thus begins the creation tale of rpyutils.

voila ros2/rpyutils#2

[jacobperron]

Full CI will all connected PRs:

• Linux
• Linux-aarch64
• macOS
• Windows

PRs:

• Add rpyutils ros2#920
• Add context manager for adding DLL directories to the search path rpyutils#2
• Explicitly add DLL directories for Windows before importing rclpy#558
• Explicitly add DLL directories for Windows before importing rosidl_python#113
• Explicitly add DLL directories for Windows before importing geometry2#266
• Explicitly add DLL directories for Windows before importing rosbag2#411

[nuclearsandwich]

Very tempted to change the 3.8.2 to 3.8.3 and mope for the best. I will let @jacobperron decide and approve either way.

[jacobperron]

I think the test failures and warnings are already occurring in the latest nightlies. I'm going to merge this and the connected PRs now and open up a second PR to try out Python 3.8.3.