include exception for parser handling of file type: fixes unexpected keyword 'encoding'

[dirksavage88]

Fixes #499

[clalancette]

So we don't see this problem in our CI; see https://ci.ros2.org/job/ci_linux/20136/ , for example.

Further, running the following locally on Ubuntu 22.04 works fine:

>>> import argparse
>>> p = argparse.ArgumentParser()
>>> p.add_argument('package_xml', type=argparse.FileType('r', encoding='utf-8'))
_StoreAction(option_strings=[], dest='package_xml', nargs=None, const=None, default=None, type=FileType('r', encoding='utf-8'), choices=None, required=True, help=None, metavar=None)

@dirksavage88 Can you please try the above locally? Also, can you run argparse.__version__ in a local Python terminal and paste the results (I get '1.1').

[dirksavage88]

So we don't see this problem in our CI; see https://ci.ros2.org/job/ci_linux/20136/ , for example.

Further, running the following locally on Ubuntu 22.04 works fine:

>>> import argparse
>>> p = argparse.ArgumentParser()
>>> p.add_argument('package_xml', type=argparse.FileType('r', encoding='utf-8'))
_StoreAction(option_strings=[], dest='package_xml', nargs=None, const=None, default=None, type=FileType('r', encoding='utf-8'), choices=None, required=True, help=None, metavar=None)

@dirksavage88 Can you please try the above locally? Also, can you run argparse.__version__ in a local Python terminal and paste the results (I get '1.1').

Using python3:

import argparse
argparse.version
'1.4.0'

[dirksavage88]

[clalancette]

OK, so that's the problem. Your version of argparse is incorrect somehow. That is actually quite odd, given that argparse was incorporated into Python years ago.

Can you also run pip3 freeze as well as:

>>> import argparse
>>> argparse.__file__

[dirksavage88]

freeze_stdout.txt

[clalancette]

Hm. So that is quite weird, your version of argparse seems to be in the right place, but it is not the correct one. I'm not sure what is going on here, but I'll suggest that you uninstall python3 from your .local directory, and instead just use the system one (/usr/bin/python3). That should fix this problem, and is what we test in CI every night (https://ci.ros2.org/view/nightly/).

[dirksavage88]

I understand the issue is probably my environment but the fix in this PR does allow me to continue without uninstalling any version of python. The .local python3 lib was placed there after the ubuntu 22.04 upgrade, I did not put it there. Python 3.10 was not part of 20.04 (and I didn't install it manually) so it could not have been installed previously either.

Would the fix prevent future users experiencing this issue from messing with python versions anyway? I can't help but think uninstalling python in any context (not just in .local) is risky and getting it wrong could result in system wide issues.

CI is great but not 100% representative of the what the user sees, and in this specific edge case CI can't replicate at all since it does not capture the build environment nuances from upgrading 20.04 to 22.04.

[clalancette]

So we can consider taking the fix, if we understand where the problem with argparse is coming from. In particular, argparse is part of the standard Python library, so I don't see how the version you have installed locally can differ from the system version, given that they are both Python 3.10. If we can solve the mystery of where that argparse is coming from, and we determine that this is a fix needed to move forward, we can consider this change.

[dirksavage88]

It's coming from pip installing argparse on top of what is already on the system: https://pypi.org/project/argparse/

[dirksavage88]

This issue was due to installing PX4 Autopilot. It installs argparse as a required dependency and explicitly called out in dependency

Installing argparse via pip due to a requirement in another piece of software should not break ament cmake core.

[dirksavage88]

@clalancette is there any blocker for this going in?

[clalancette]

@clalancette is there any blocker for this going in?

I just don't see why we need it. There has been no release of argparse for close to a decade, and the functionality is built-in to Python itself.

And to be clear, this is a regression in functionality. If a user does happen to have argparse installed as a pip package, then they will not be able to successfully open package.xml files that contain UTF-8 characters. So I'd rather that users have to uninstall argparse (which they should do anyway), then workaround it here and fail later anyway.

[dirksavage88]

@clalancette is there any blocker for this going in?

I just don't see why we need it. There has been no release of argparse for close to a decade, and the functionality is built-in to Python itself.

And to be clear, this is a regression in functionality. If a user does happen to have argparse installed as a pip package, then they will not be able to successfully open package.xml files that contain UTF-8 characters. So I'd rather that users have to uninstall argparse (which they should do anyway), then workaround it here and fail later anyway.

If I'm not the only user to face this issue, and I guarantee I am not, I think it's worth it. I have recently ran into the same issue again on my development machine. Wouldn't you better be serving the open source community with robust software instead of fragile toolchains?

The issue at hand is that users also need argparse installed via another toolchain on their system, namely PX4. I will be adding this issue as a blocker to the ROS Aerial WG as a blocker for those wanting to build ROS 2 applications in combination with PX4 software, since the toolchains apparently are not compatible out of the box....

[clalancette]

Can you please point me to the part of the PX4 documentation where it suggests to install argparse via pip?

[dirksavage88]

The ubuntu setup script is step 2 as indicated here: https://docs.px4.io/main/en/dev_setup/dev_env_linux_ubuntu.html#simulation-and-nuttx-pixhawk-targets

This setup script invokes the python pip install of the requirements.txt file: https://github.com/PX4/PX4-Autopilot/blob/c22f725d858e93aeaa638dac41c28be42ddfb293/Tools/setup/ubuntu.sh#L108

Lastly, here is the argparse requirement listed in that file: https://github.com/PX4/PX4-Autopilot/blob/c22f725d858e93aeaa638dac41c28be42ddfb293/Tools/setup/requirements.txt#L2

[clalancette]

See PX4/PX4-Autopilot#23230