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

Remove use of pkg_resources from osrf_pycommon. #66

Merged
merged 6 commits into from
Feb 1, 2022

Conversation

clalancette
Copy link
Contributor

Replace it with the use of the more modern importlib*
libraries. There are a couple of reasons to do this:

  1. pkg_resources is quite slow to import; on my machine,
    just firing up the python interpreter takes ~35ms, while
    firing up the python interpreter and importing pkg_resources
    takes ~175ms. Firing up the python interpreter and importing
    importlib_metadata takes ~70ms. Removing 100ms per invocation
    of the command-line both makes it speedier for users, and
    will speed up our tests (which call out to the command-line
    quite a lot).

  2. pkg_resources is somewhat deprecated and being replaced
    by importlib. https://importlib-metadata.readthedocs.io/en/latest/using.html
    describes some of it

Signed-off-by: Chris Lalancette clalancette@openrobotics.org

See ros2/ros2#919 for a lot more details on why we want to do this.

package.xml Outdated Show resolved Hide resolved
@wjwwood
Copy link
Member

wjwwood commented Feb 27, 2021

@clalancette I'm going to do one more release for Python2 before doing this.

@wjwwood wjwwood force-pushed the remove-pkg-resources branch from b1a3cb9 to 826816a Compare July 28, 2021 01:32
package.xml Show resolved Hide resolved
clalancette and others added 2 commits August 2, 2021 15:09
Replace it with the use of the more modern importlib*
libraries.  There are a couple of reasons to do this:

1.  pkg_resources is quite slow to import; on my machine,
just firing up the python interpreter takes ~35ms, while
firing up the python interpreter and importing pkg_resources
takes ~175ms.  Firing up the python interpreter and importing
importlib_metadata takes ~70ms.  Removing 100ms per invocation
of the command-line both makes it speedier for users, and
will speed up our tests (which call out to the command-line
quite a lot).

2.  pkg_resources is somewhat deprecated and being replaced
by importlib.  https://importlib-metadata.readthedocs.io/en/latest/using.html
describes some of it

Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
Signed-off-by: William Woodall <william@osrfoundation.org>
@clalancette clalancette force-pushed the remove-pkg-resources branch from efb21b2 to fd238ad Compare August 2, 2021 15:09
stdeb.cfg Outdated Show resolved Hide resolved
osrf_pycommon/cli_utils/verb_pattern.py Outdated Show resolved Hide resolved
Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
…data

Signed-off-by: William Woodall <william@osrfoundation.org>
Signed-off-by: William Woodall <william@osrfoundation.org>
@wjwwood wjwwood merged commit 6eaa13c into master Feb 1, 2022
@wjwwood wjwwood deleted the remove-pkg-resources branch February 1, 2022 23:19
@wjwwood
Copy link
Member

wjwwood commented Feb 2, 2022

Update on this, I've released versions 1.0.2 supporting < focal and 2.0.0 supporting >= focal. I released both to pip, and both to our bootstrap repository, making the python3-osrf-pycommon debs available for ROS 1 and 2's package repo.

I was going to release them into the current ROS 2 distributions, but after talking to @nuclearsandwich about it we may want to change that and the latest previous version (1.0.1) is not in foxy or galactic anyway. I added this to the next ROS 2 meeting's agenda.

@rgov
Copy link

rgov commented Feb 3, 2022

I think this change introduced a regression on ROS 1 Noetic / Debian Buster, because there is no Python 3.8 package for that release, and hence the python3-osrf-pycommon package cannot be installed. See question on answers.ros.org.

Can version 1.0.2 please be re-released for Buster?

@wjwwood
Copy link
Member

wjwwood commented Feb 4, 2022

@rgov thanks for letting me know. I will look into this, sorry for the disruption. We did a split release which was supposed to avoid this (e.g. we have separate debs for Ubuntu <focal and >=focal), but maybe we messed it up for buster (Debian).

@nuclearsandwich
Copy link
Member

I've opened #82 and #83 which update the release track for Debian Buster.

@wjwwood
Copy link
Member

wjwwood commented Feb 14, 2022

I'm doing the release today.

@wjwwood
Copy link
Member

wjwwood commented Feb 15, 2022

We've merged #82 and #83, released a 1.0.3 which includes buster, and pulled 2.0.0 for buster from our repos. I think that should be sufficient to fix your issue @rgov. I'm also going to release 2.0.1 which does not make a deb for buster, but that's just for consistency, it shouldn't be needed.

@wjwwood
Copy link
Member

wjwwood commented Feb 15, 2022

Thanks for reporting it!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants