[Windows] Disable multiprocessing usage for package.py on Windows

[seanyen]

On Windows, the multiprocessing will require the script who consumes the catkin_pkg to be patched in order to work properly.

Instead of touching up all the scripts consuming catkin_pkg, disable multiprocessing on Windows. Otherwise, catkin_pkg won't work for a workspace with more than 100 packages on Windows.

Traceback (most recent call last):
  File "F:\workspace\catkin_pkg\test\test_metapackage.py", line 76, in test_validate_metapackage
    pkgs_dict = find_packages(test_data_dir)
  File "f:\workspace\catkin_pkg\src\catkin_pkg\packages.py", line 90, in find_packages
    packages = find_packages_allowing_duplicates(basepath, exclude_paths=exclude_paths, exclude_subspaces=exclude_subspaces, warnings=warnings)
  File "f:\workspace\catkin_pkg\src\catkin_pkg\packages.py", line 141, in find_packages_allowing_duplicates
    pool = multiprocessing.Pool()
  File "C:\Python27\lib\multiprocessing\__init__.py", line 232, in Pool
    return Pool(processes, initializer, initargs, maxtasksperchild)
  File "C:\Python27\lib\multiprocessing\pool.py", line 161, in __init__
    self._repopulate_pool()
  File "C:\Python27\lib\multiprocessing\pool.py", line 225, in _repopulate_pool
    w.start()
  File "C:\Python27\lib\multiprocessing\process.py", line 130, in start
    self._popen = Popen(self)
  File "C:\Python27\lib\multiprocessing\forking.py", line 258, in __init__
    cmd = get_command_line() + [rhandle]
  File "C:\Python27\lib\multiprocessing\forking.py", line 358, in get_command_line
    is not going to be frozen to produce a Windows executable.''')
RuntimeError:
            Attempt to start a new process before the current process
            has finished its bootstrapping phase.

            This probably means that you are on Windows and you have
            forgotten to use the proper idiom in the main module:

                if __name__ == '__main__':
                    freeze_support()
                    ...

            The "freeze_support()" line can be omitted if the program
            is not going to be frozen to produce a Windows executable.

[dirk-thomas]

On Windows, the multiprocessing will require the script who consumes the catkin_pkg to be patched in order to work properly.

Can you please provide more information about this?

[seanyen]

@dirk-thomas based on Python manual, the multiprocessing usage on Windows requires additional treatment to get it worked properly. https://docs.python.org/2/library/multiprocessing.html

Here is one example how the downstream could be impacted by this multiprocessing issue:
ros-infrastructure/rosdep#645

[dirk-thomas]

based on Python manual, the multiprocessing usage on Windows requires additional treatment to get it worked properly. https://docs.python.org/2/library/multiprocessing.html

I am sorry but this page is really long and I don't find the information you are trying to point to. Please either use a specific link to the section describing the documentation you are referring to or quote the relevant text from the docs here.

[seanyen]

Here is the specific link to the Windows section: https://docs.python.org/2/library/multiprocessing.html#windows

[dirk-thomas]

Ok, I have read the section "16.6.3.2. Windows" - in particular "Safe importing of main module".

Have you tried to apply the suggest call of freeze_support() to address the problem described in the rosdep ticket?

[seanyen]

For rosdep, the least change making it worked to me are:

• Change the file name of https://github.com/ros-infrastructure/rosdep/blob/master/scripts/rosdep to rosdep.py (It seems to be only required for Python2. I ran some tests on Python3.7 and I don't need to postfix .py as file extension.)
• Add if __name__ == '__main__': before rosdep_main()

Alternatively, we can use the console_scripts feature will auto generate such compliant format, so this PR ros-infrastructure/rosdep#656 (cc @nuclearsandwich) just happens to help solve the multiprocessing problems.

However, my questions are that since catkin_pkg are a so much low level module in ROS stack, there are many tools using catkin_pkg which potentially need this treatment, do we address them all one by one? or we firstly disable it (or optionally opt-out by flags) on Windows to unblock users who hits this problem on the downstream projects (before the fix is addressed on the downstream tools)?

[dirk-thomas]

do we address them all one by one?

I wonder if this can't be worked around in catkin_pkg - specifically where the multiprocessing module is being used. That would address all problems at once.

we firstly disable it on Windows to unblock users who hits this problem

I would rather not apply a workaround if the possibility of a actual fix in catkin_pkg hasn't been ruled out. From past experiences a hacky fix like this tends to stay around for a long since there is no reason to look at it again.

[seanyen]

I would rather not apply a workaround if the possibility of a actual fix in catkin_pkg hasn't been ruled out. From past experiences a hacky fix like this tends to stay around for a long since there is no reason to look at it again.

That's fair enough. I will look for impacted tools and send fix PR there instead.

[dirk-thomas]

I would rather not apply a workaround if the possibility of a actual fix in catkin_pkg hasn't been ruled out. From past experiences a hacky fix like this tends to stay around for a long since there is no reason to look at it again.

That's fair enough. I will look for impacted tools and send fix PR there instead.

There might be a misunderstanding. I am suggesting to try to find a fix for catkin_pkg (so a patch against this repository) to address the problem you are seeing with e.g. rosdep and likely other tools too.

[seanyen]

@dirk-thomas Thanks for the clarification! I created #251 to propose a change to be only scoped in catkin_pkg. Please take a look and let me know what do you think.

[dirk-thomas]

@seanyen With #251 closed how do you want to proceed with this ticket?

[seanyen]

@dirk-thomas Since workaround is not really preferred at this moment, and I haven't really found a good way to isolate a fix to only catkin_pkg. I will keep pushing the entry_points approach to downstream projects, which I believe it is also more sustainable. That being said, I will close this one now. Thanks for your time!