From ce4f361322ac8a75a575319b9a03b68433bfbfba Mon Sep 17 00:00:00 2001 From: "Isaac I.Y. Saito" <130s@2000.jukuin.keio.ac.jp> Date: Fri, 1 Feb 2019 15:10:24 -0800 Subject: [PATCH 01/12] [RosPack] Add a license introspection feature. [get_licenses] Delegate traversing all dependency to existing method (not exactly but hopefully closer to what's suggested in https://github.com/ros-infrastructure/rospkg/pull/133#issuecomment-367070169) [get_licenses] Add system dependency to the output. [get_licenses] Sort licenses and pkgs. [get_licenses] Conform to variable name convention. --- src/rospkg/rospack.py | 51 +++++++++++++++++++++++++++++ test/test_rospkg_catkin_packages.py | 10 ++++++ 2 files changed, 61 insertions(+) diff --git a/src/rospkg/rospack.py b/src/rospkg/rospack.py index 35203218..3d17d1e1 100644 --- a/src/rospkg/rospack.py +++ b/src/rospkg/rospack.py @@ -30,6 +30,7 @@ # ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE # POSSIBILITY OF SUCH DAMAGE. +from collections import defaultdict, OrderedDict import os from threading import Lock @@ -391,6 +392,56 @@ def stack_of(self, package): else: d = os.path.dirname(d) + def get_licenses(self, name, implicit=True, sort_by_license=True): + """ + @summary: Return a list of licenses the packages the given package declares + dependency on. + @return Dictionary. By default dict of license name and package name(s). + Note: For the packages that declare multiple licenses, those licenses + are returned in a single string with each license separated by comma. + E.g. if your package declares BSD and LGPL in separate tags in + package.xml, the returned key will look like "BSD, LGPL". + @rtype { k, [d] } + @raise ResourceNotFound + """ + MSG_LICENSE_NOTFOUND_SYSPKG = "(License not automatically detected)" + license_dict = defaultdict(list) + + self.get_depends(name, implicit) + + manifests = self._manifests + + for pkg_name, manifest in manifests.items(): + if not sort_by_license: + license_dict[pkg_name].append(manifest.license) + else: + license_dict[manifest.license].append(pkg_name) + + # Traverse for Non-ROS, system packages + try: + pkgnames_rosdep = self.get_rosdeps(name, implicit) + except ResourceNotFound as e: + raise e + for pkgname_rosdep in pkgnames_rosdep: + if not sort_by_license: + license_dict[pkgname_rosdep].append(MSG_LICENSE_NOTFOUND_SYSPKG) + else: + license_dict[MSG_LICENSE_NOTFOUND_SYSPKG].append(pkgname_rosdep) + + # Sort pkg names in each license + for list_key in license_dict.values(): + list_key.sort() + # Sort by license name. + licenses = OrderedDict(sorted(license_dict.items())) + + # List of tuples converted into yaml can look like the following, which isn't + # that useful. So here converting to a dict. + # - !!python/tuple + # - LGPL + # - - python_orocos_kdl + # - orocos_kdl + return dict(licenses) + class RosStack(ManifestManager): """ diff --git a/test/test_rospkg_catkin_packages.py b/test/test_rospkg_catkin_packages.py index 87ac2799..5136344d 100644 --- a/test/test_rospkg_catkin_packages.py +++ b/test/test_rospkg_catkin_packages.py @@ -59,6 +59,7 @@ def test_get_manifest(): assert(manif.type == "package") + def test_licenses(): rospack = rospkg.rospack.RosPack(ros_paths=[search_path]) licenses_list = ["BSD", "LGPL"] @@ -67,3 +68,12 @@ def test_licenses(): assert(len(manif.licenses) == 2) for l in manif.licenses: assert(l in licenses_list) + + +def test_get_licenses(): + search_path = os.path.abspath(os.path.join(os.path.dirname(__file__), 'catkin_package_tests')) + manager = rospkg.rospack.RosPack(ros_paths=[search_path]) + licenses = manager.get_licenses("foo", implicit=False) + # package foo declares these 2 licenses in separate tags, which the dict + # get_licenses returns contains as a single string. + assert("BSD, LGPL" in licenses) From 03c52cca96c0c70887d1595386af91a15b7aef4d Mon Sep 17 00:00:00 2001 From: "Isaac I.Y. Saito" <130s@2000.jukuin.keio.ac.jp> Date: Tue, 19 Jun 2018 21:06:42 -0700 Subject: [PATCH 02/12] Package name is missing in Manifest class member. --- src/rospkg/manifest.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/rospkg/manifest.py b/src/rospkg/manifest.py index 20cecfe5..80484bd5 100644 --- a/src/rospkg/manifest.py +++ b/src/rospkg/manifest.py @@ -42,7 +42,7 @@ from .common import MANIFEST_FILE, PACKAGE_FILE, STACK_FILE # stack.xml and manifest.xml have the same internal tags right now -REQUIRED = ['license'] +REQUIRED = ['license', "name"] ALLOWXHTML = ['description'] OPTIONAL = ['author', 'logo', 'url', 'brief', 'description', 'status', 'notes', 'depend', 'rosdep', 'export', 'review', @@ -318,7 +318,7 @@ class Manifest(object): 'author', 'license', 'licenses', 'license_url', 'url', 'depends', 'rosdeps', 'platforms', 'exports', 'version', - 'status', 'notes', + 'status', 'name', 'notes', 'unknown_tags', 'type', 'filename', 'is_catkin'] @@ -396,6 +396,7 @@ def parse_manifest_file(dirpath, manifest_name, rospack=None): p = parse_package(package_filename) # put these into manifest manifest.description = p.description + manifest.name = p.name manifest.author = ', '.join([('Maintainer: %s' % str(m)) for m in p.maintainers] + [str(a) for a in p.authors]) manifest.license = ', '.join(p.licenses) manifest.licenses = p.licenses @@ -499,6 +500,7 @@ def parse_manifest(manifest_name, string, filename='string'): pass # manifest is missing optional 'review notes' tag m.author = _check('author', True)(p, filename) + m.name = _check("name")(p, filename) m.url = _check('url')(p, filename) m.version = _check('version')(p, filename) From 6241e441fcc459fd2c1d7a3e101f94d9f6d8b8aa Mon Sep 17 00:00:00 2001 From: "Isaac I.Y. Saito" <130s@2000.jukuin.keio.ac.jp> Date: Sat, 23 Feb 2019 15:06:49 -0800 Subject: [PATCH 03/12] [manifest] Use list of str instead of a single str, to handle better packages with multiple licenses. Add a `licenses` field to Manifest class (in addition to `license`. Although {manifest, package}.xml format allows to define separate xml elements, rospkg handles multiple licenses as a single string joined by a comma, which is inconvenient depending on the consumers' usecase. Pointed out at https://github.com/ros-infrastructure/rospkg/pull/133/files#r253230420 If we want to maintain backward compatibility, `license` str field shouldn't change. So without making any change to it, this PR only adds a new field. This could cause a confusion. I thought to add printing a warning for deprecation for license, but because it looks like `license` is (and other fields are) intended to be accessed directly (without getter method/structure), I'm not sure how to notify the users during runtime. ``` $ ipython Python 2.7.12 (default, Nov 12 2018, 14:36:49) Type "copyright", "credits" or "license" for more information. IPython 2.4.1 -- An enhanced Interactive Python. ? -> Introduction and overview of IPython's features. %quickref -> Quick reference. help -> Python's own help system. object? -> Details about 'object', use 'object??' for extra details. In [1]: import rospkg In [2]: rp = rospkg.RosPack() In [4]: p = rp.get_manifest("rviz") In [5]: p.license Out[5]: 'BSD, Creative Commons' In [6]: p.licenses Out[6]: ['BSD', 'Creative Commons'] ``` - [x] Requester provides reproducible commands and sample test result. - [ ] CI should pass. - [ ] Code review. [get_licenses] Use Manifest.licenses, not Manifest.license in order to handle pkgs with multiple licenses better. --- src/rospkg/manifest.py | 1 + src/rospkg/rospack.py | 17 ++++++++--------- test/test_rospkg_catkin_packages.py | 7 ++++--- 3 files changed, 13 insertions(+), 12 deletions(-) diff --git a/src/rospkg/manifest.py b/src/rospkg/manifest.py index 80484bd5..c81b4dd7 100644 --- a/src/rospkg/manifest.py +++ b/src/rospkg/manifest.py @@ -334,6 +334,7 @@ def __init__(self, type_='package', filename=None, is_catkin=False): self.version = self.notes = '' self.licenses = [] self.depends = [] + self.licenses = [] self.rosdeps = [] self.exports = [] self.platforms = [] diff --git a/src/rospkg/rospack.py b/src/rospkg/rospack.py index 3d17d1e1..2498d082 100644 --- a/src/rospkg/rospack.py +++ b/src/rospkg/rospack.py @@ -394,13 +394,11 @@ def stack_of(self, package): def get_licenses(self, name, implicit=True, sort_by_license=True): """ - @summary: Return a list of licenses the packages the given package declares - dependency on. - @return Dictionary. By default dict of license name and package name(s). - Note: For the packages that declare multiple licenses, those licenses - are returned in a single string with each license separated by comma. - E.g. if your package declares BSD and LGPL in separate tags in - package.xml, the returned key will look like "BSD, LGPL". + @summary: Return a list of licenses and the packages in the dependency tree + for the given package. Format of the returned object can vary depending on + the argument. + @return Dictionary. By default dict of license name and a list of packages. + When sort_by_license=False, dict of package name and a list of licenses. @rtype { k, [d] } @raise ResourceNotFound """ @@ -413,9 +411,10 @@ def get_licenses(self, name, implicit=True, sort_by_license=True): for pkg_name, manifest in manifests.items(): if not sort_by_license: - license_dict[pkg_name].append(manifest.license) + license_dict[pkg_name].append(manifest.licenses) else: - license_dict[manifest.license].append(pkg_name) + for license in manifest.licenses: + license_dict[license].append(pkg_name) # Traverse for Non-ROS, system packages try: diff --git a/test/test_rospkg_catkin_packages.py b/test/test_rospkg_catkin_packages.py index 5136344d..41c45f01 100644 --- a/test/test_rospkg_catkin_packages.py +++ b/test/test_rospkg_catkin_packages.py @@ -74,6 +74,7 @@ def test_get_licenses(): search_path = os.path.abspath(os.path.join(os.path.dirname(__file__), 'catkin_package_tests')) manager = rospkg.rospack.RosPack(ros_paths=[search_path]) licenses = manager.get_licenses("foo", implicit=False) - # package foo declares these 2 licenses in separate tags, which the dict - # get_licenses returns contains as a single string. - assert("BSD, LGPL" in licenses) + # package foo declares these 2 licenses. + assert not "BSD, LGPL" in licenses + assert("BSD" in licenses) + assert("LGPL" in licenses) From 0f3b81cbf6ee7c3d676c6e8af926768f94df3995 Mon Sep 17 00:00:00 2001 From: "Isaac I.Y. Saito" <130s@2000.jukuin.keio.ac.jp> Date: Sat, 23 Feb 2019 16:37:56 -0800 Subject: [PATCH 04/12] [get_licenses] Remove an option to sort by pkg name. There might not be usecase, and if there is, users can easily do by themselves. [get_licenses] Call methods with kw arguments to better clarify args. Rename an arg for better clarity. [get_licenses] Clean up. --- src/rospkg/manifest.py | 4 ++-- src/rospkg/rospack.py | 32 +++++++++-------------------- test/test_rospkg_catkin_packages.py | 1 - 3 files changed, 12 insertions(+), 25 deletions(-) diff --git a/src/rospkg/manifest.py b/src/rospkg/manifest.py index c81b4dd7..e3d6788a 100644 --- a/src/rospkg/manifest.py +++ b/src/rospkg/manifest.py @@ -42,7 +42,7 @@ from .common import MANIFEST_FILE, PACKAGE_FILE, STACK_FILE # stack.xml and manifest.xml have the same internal tags right now -REQUIRED = ['license', "name"] +REQUIRED = ['license', 'name'] ALLOWXHTML = ['description'] OPTIONAL = ['author', 'logo', 'url', 'brief', 'description', 'status', 'notes', 'depend', 'rosdep', 'export', 'review', @@ -501,7 +501,7 @@ def parse_manifest(manifest_name, string, filename='string'): pass # manifest is missing optional 'review notes' tag m.author = _check('author', True)(p, filename) - m.name = _check("name")(p, filename) + m.name = _check('name')(p, filename) m.url = _check('url')(p, filename) m.version = _check('version')(p, filename) diff --git a/src/rospkg/rospack.py b/src/rospkg/rospack.py index 2498d082..2aa503d4 100644 --- a/src/rospkg/rospack.py +++ b/src/rospkg/rospack.py @@ -392,40 +392,28 @@ def stack_of(self, package): else: d = os.path.dirname(d) - def get_licenses(self, name, implicit=True, sort_by_license=True): + def get_licenses(self, pkg_name, implicit=True): """ @summary: Return a list of licenses and the packages in the dependency tree - for the given package. Format of the returned object can vary depending on - the argument. - @return Dictionary. By default dict of license name and a list of packages. - When sort_by_license=False, dict of package name and a list of licenses. + for the given package. + @param pkg_name: Name of the package the dependency tree begins from. + @return Dictionary of license name and a list of packages. @rtype { k, [d] } @raise ResourceNotFound """ MSG_LICENSE_NOTFOUND_SYSPKG = "(License not automatically detected)" license_dict = defaultdict(list) - self.get_depends(name, implicit) + self.get_depends(name=pkg_name, implicit=implicit) - manifests = self._manifests - - for pkg_name, manifest in manifests.items(): - if not sort_by_license: - license_dict[pkg_name].append(manifest.licenses) - else: - for license in manifest.licenses: - license_dict[license].append(pkg_name) + for p_name, manifest in self._manifests.items(): + for license in manifest.licenses: + license_dict[license].append(p_name) # Traverse for Non-ROS, system packages - try: - pkgnames_rosdep = self.get_rosdeps(name, implicit) - except ResourceNotFound as e: - raise e + pkgnames_rosdep = self.get_rosdeps(name=pkg_name, implicit=implicit) for pkgname_rosdep in pkgnames_rosdep: - if not sort_by_license: - license_dict[pkgname_rosdep].append(MSG_LICENSE_NOTFOUND_SYSPKG) - else: - license_dict[MSG_LICENSE_NOTFOUND_SYSPKG].append(pkgname_rosdep) + license_dict[MSG_LICENSE_NOTFOUND_SYSPKG].append(pkgname_rosdep) # Sort pkg names in each license for list_key in license_dict.values(): diff --git a/test/test_rospkg_catkin_packages.py b/test/test_rospkg_catkin_packages.py index 41c45f01..b6b0f1d0 100644 --- a/test/test_rospkg_catkin_packages.py +++ b/test/test_rospkg_catkin_packages.py @@ -59,7 +59,6 @@ def test_get_manifest(): assert(manif.type == "package") - def test_licenses(): rospack = rospkg.rospack.RosPack(ros_paths=[search_path]) licenses_list = ["BSD", "LGPL"] From 86596cf38a494c2e1d28b0e88e913637230e6a79 Mon Sep 17 00:00:00 2001 From: "Isaac I.Y. Saito" <130s@2000.jukuin.keio.ac.jp> Date: Sat, 23 Feb 2019 17:09:18 -0800 Subject: [PATCH 05/12] [get_licenses] Comment for sort (address https://github.com/ros-infrastructure/rospkg/pull/133/files#r253231615). --- src/rospkg/rospack.py | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/src/rospkg/rospack.py b/src/rospkg/rospack.py index 2aa503d4..3462ce85 100644 --- a/src/rospkg/rospack.py +++ b/src/rospkg/rospack.py @@ -411,22 +411,16 @@ def get_licenses(self, pkg_name, implicit=True): license_dict[license].append(p_name) # Traverse for Non-ROS, system packages - pkgnames_rosdep = self.get_rosdeps(name=pkg_name, implicit=implicit) + pkgnames_rosdep = self.get_rosdeps(package=pkg_name, implicit=implicit) for pkgname_rosdep in pkgnames_rosdep: license_dict[MSG_LICENSE_NOTFOUND_SYSPKG].append(pkgname_rosdep) # Sort pkg names in each license for list_key in license_dict.values(): list_key.sort() - # Sort by license name. + # Sort license names licenses = OrderedDict(sorted(license_dict.items())) - - # List of tuples converted into yaml can look like the following, which isn't - # that useful. So here converting to a dict. - # - !!python/tuple - # - LGPL - # - - python_orocos_kdl - # - orocos_kdl + # Convert to dict for user friendlier output. return dict(licenses) From 56f04f41d851c7252dd35ceb91b93ea19c9685ec Mon Sep 17 00:00:00 2001 From: "Isaac I.Y. Saito" <130s@2000.jukuin.keio.ac.jp> Date: Sat, 23 Feb 2019 17:14:57 -0800 Subject: [PATCH 06/12] [get_licenses] Not using long sentence as the license name for the pkgs license could not be detected (address https://github.com/ros-infrastructure/rospkg/pull/133/files#r253230933). --- src/rospkg/rospack.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/rospkg/rospack.py b/src/rospkg/rospack.py index 3462ce85..71ff8e24 100644 --- a/src/rospkg/rospack.py +++ b/src/rospkg/rospack.py @@ -395,13 +395,14 @@ def stack_of(self, package): def get_licenses(self, pkg_name, implicit=True): """ @summary: Return a list of licenses and the packages in the dependency tree - for the given package. + for the given package. Special value 'ERR' is used as the license for the + packages that license was not detected for. @param pkg_name: Name of the package the dependency tree begins from. @return Dictionary of license name and a list of packages. @rtype { k, [d] } @raise ResourceNotFound """ - MSG_LICENSE_NOTFOUND_SYSPKG = "(License not automatically detected)" + MSG_LICENSE_NOTFOUND_SYSPKG = "ERR" license_dict = defaultdict(list) self.get_depends(name=pkg_name, implicit=implicit) From 761ac850f179bcab1886756ed51d65a0e49e496a Mon Sep 17 00:00:00 2001 From: "Isaac I.Y. Saito" <130s@2000.jukuin.keio.ac.jp> Date: Sat, 30 Mar 2019 14:57:14 -0700 Subject: [PATCH 07/12] [get_licenses] Add a test case. [test][catkin] Removing a ROS package from a dependency list as it is unresolvable during the set of nosetests in this package. --- test/catkin_package_tests/p1/bar/package.xml | 21 ++++++++++++++++++++ test/catkin_package_tests/p1/foo/package.xml | 3 --- test/test_rospkg_catkin_packages.py | 16 +++++++-------- 3 files changed, 29 insertions(+), 11 deletions(-) create mode 100644 test/catkin_package_tests/p1/bar/package.xml diff --git a/test/catkin_package_tests/p1/bar/package.xml b/test/catkin_package_tests/p1/bar/package.xml new file mode 100644 index 00000000..d454f3bd --- /dev/null +++ b/test/catkin_package_tests/p1/bar/package.xml @@ -0,0 +1,21 @@ + + + bar + 1.2.3 + + I AM THE VERY MODEL OF A MODERN MAJOR GENERAL + + Someone + + MIT + + http://wiki.ros.org/bar + http://www.github.com/my_org/bar/issues + Jane Doe + Jane Doe + + liburdfdom-dev + foo + gtest + + diff --git a/test/catkin_package_tests/p1/foo/package.xml b/test/catkin_package_tests/p1/foo/package.xml index f6aa0ee2..6b6044bf 100644 --- a/test/catkin_package_tests/p1/foo/package.xml +++ b/test/catkin_package_tests/p1/foo/package.xml @@ -14,9 +14,6 @@ John Doe Jane Doe - catkin - genmsg - libboost-thread-dev libboost-thread diff --git a/test/test_rospkg_catkin_packages.py b/test/test_rospkg_catkin_packages.py index b6b0f1d0..434245b2 100644 --- a/test/test_rospkg_catkin_packages.py +++ b/test/test_rospkg_catkin_packages.py @@ -40,17 +40,18 @@ def test_find_packages(): + PKGS_EXIST = ['foo', 'bar'] manager = rospkg.rospack.ManifestManager(rospkg.common.MANIFEST_FILE, ros_paths=[search_path]) # for backward compatibility a wet package which is not a metapackage is found when searching for MANIFEST_FILE - assert(len(manager.list()) == 1) + assert(len(manager.list()) == 2) manager = rospkg.rospack.ManifestManager(rospkg.common.STACK_FILE, ros_paths=[search_path]) assert(len(manager.list()) == 0) manager = rospkg.rospack.ManifestManager(rospkg.common.PACKAGE_FILE, ros_paths=[search_path]) for pkg_name in manager.list(): - assert(pkg_name == 'foo') + assert(pkg_name in PKGS_EXIST) path = manager.get_path(pkg_name) - assert(path == os.path.join(search_path, 'p1', 'foo')) + assert(path == os.path.join(search_path, 'p1', PKGS_EXIST[PKGS_EXIST.index(pkg_name)])) def test_get_manifest(): @@ -70,10 +71,9 @@ def test_licenses(): def test_get_licenses(): - search_path = os.path.abspath(os.path.join(os.path.dirname(__file__), 'catkin_package_tests')) - manager = rospkg.rospack.RosPack(ros_paths=[search_path]) - licenses = manager.get_licenses("foo", implicit=False) - # package foo declares these 2 licenses. - assert not "BSD, LGPL" in licenses + """Check licenses from all packages in the dependency chain""" + rospack = rospkg.rospack.RosPack(ros_paths=[search_path]) + licenses = rospack.get_licenses("bar", implicit=True) + assert("MIT" in licenses) assert("BSD" in licenses) assert("LGPL" in licenses) From 2651d0ec0011a2e95131617797e59c20dd7a1dcd Mon Sep 17 00:00:00 2001 From: "Isaac I.Y. Saito" <130s@2000.jukuin.keio.ac.jp> Date: Sat, 30 Mar 2019 23:23:56 -0700 Subject: [PATCH 08/12] parse_manifest_file fails without having rosdep update run. Setting up apt-get for python-rosdep might be complicated. For CI pip seems to be an acceptable solution. Enabling sudo in Travis level might be needed to run rosdep init. Copying from bloom project how to run rosdep on Travis CI with pip. --- .travis.yml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index ed136ab0..77fcabcc 100644 --- a/.travis.yml +++ b/.travis.yml @@ -6,7 +6,9 @@ install: # develop seems to be required by travis since 02/2013 - if [ $TRAVIS_PYTHON_VERSION == "2.7" -o $TRAVIS_PYTHON_VERSION == "3.4" ]; then pip install pyparsing==2.4.7; fi - python setup.py build develop - - pip install nose coverage + - pip install nose coverage rosdep + - sudo `which rosdep` init + - rosdep update # command to run tests script: - nosetests --with-coverage --cover-package=rospkg --with-xunit test From e813c8b0c4ba9bb9bf059aed2859a0ccfd3bfa0d Mon Sep 17 00:00:00 2001 From: "Isaac I.Y. Saito" <130s@2000.jukuin.keio.ac.jp> Date: Tue, 5 May 2020 01:23:02 -0700 Subject: [PATCH 09/12] [rospack.py] Remove meaningless conversion from OrderedDict to dict (Address https://github.com/ros-infrastructure/rospkg/pull/133#discussion_r253231615). --- src/rospkg/rospack.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/rospkg/rospack.py b/src/rospkg/rospack.py index 71ff8e24..ff32ae94 100644 --- a/src/rospkg/rospack.py +++ b/src/rospkg/rospack.py @@ -398,7 +398,7 @@ def get_licenses(self, pkg_name, implicit=True): for the given package. Special value 'ERR' is used as the license for the packages that license was not detected for. @param pkg_name: Name of the package the dependency tree begins from. - @return Dictionary of license name and a list of packages. + @return OrderedDict of license name and a list of packages. @rtype { k, [d] } @raise ResourceNotFound """ @@ -421,8 +421,7 @@ def get_licenses(self, pkg_name, implicit=True): list_key.sort() # Sort license names licenses = OrderedDict(sorted(license_dict.items())) - # Convert to dict for user friendlier output. - return dict(licenses) + return licenses class RosStack(ManifestManager): From ca2a399254c5408a8da101fee0a0d08e11519732 Mon Sep 17 00:00:00 2001 From: "Isaac I.Y. Saito" <130s@2000.jukuin.keio.ac.jp> Date: Tue, 5 May 2020 02:01:35 -0700 Subject: [PATCH 10/12] [rospack.py] Added a test for the expected value (https://github.com/ros-infrastructure/rospkg/pull/133#discussion_r270962522). --- test/test_rospkg_catkin_packages.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/test_rospkg_catkin_packages.py b/test/test_rospkg_catkin_packages.py index 434245b2..e86e3336 100644 --- a/test/test_rospkg_catkin_packages.py +++ b/test/test_rospkg_catkin_packages.py @@ -73,7 +73,9 @@ def test_licenses(): def test_get_licenses(): """Check licenses from all packages in the dependency chain""" rospack = rospkg.rospack.RosPack(ros_paths=[search_path]) + licenses_list = [None, "MIT", "BSD", "LGPL"] # Licenses of 'bar and 'foo'. License for some deps are undetermined so 'None'. licenses = rospack.get_licenses("bar", implicit=True) assert("MIT" in licenses) assert("BSD" in licenses) assert("LGPL" in licenses) + assert(set(licenses) == set(licenses_list)) From 846353ea3a2633c4cdfd3669a68c12bdc690922e Mon Sep 17 00:00:00 2001 From: "Isaac I.Y. Saito" <130s@2000.jukuin.keio.ac.jp> Date: Tue, 5 May 2020 02:02:08 -0700 Subject: [PATCH 11/12] [rospack.py] Errored pkg name (https://github.com/ros-infrastructure/rospkg/pull/133#discussion_r270967801). 'None' causes TypeError ('<' not supported between instances of 'NoneType' and 'License) so defining non None. --- src/rospkg/rospack.py | 11 ++++++----- test/test_rospkg_catkin_packages.py | 4 +++- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/src/rospkg/rospack.py b/src/rospkg/rospack.py index ff32ae94..5b91e970 100644 --- a/src/rospkg/rospack.py +++ b/src/rospkg/rospack.py @@ -328,6 +328,8 @@ class RosPack(ManifestManager): direct_depends = rp.get_depends('roscpp', implicit=False) """ + LICENSE_NOT_FOUND= "license_not_found" + def __init__(self, ros_paths=None): """ :param ros_paths: Ordered list of paths to search for @@ -395,14 +397,13 @@ def stack_of(self, package): def get_licenses(self, pkg_name, implicit=True): """ @summary: Return a list of licenses and the packages in the dependency tree - for the given package. Special value 'ERR' is used as the license for the + for the given package. Special value 'license_not_found' is used as the license for the packages that license was not detected for. @param pkg_name: Name of the package the dependency tree begins from. @return OrderedDict of license name and a list of packages. @rtype { k, [d] } @raise ResourceNotFound """ - MSG_LICENSE_NOTFOUND_SYSPKG = "ERR" license_dict = defaultdict(list) self.get_depends(name=pkg_name, implicit=implicit) @@ -414,12 +415,12 @@ def get_licenses(self, pkg_name, implicit=True): # Traverse for Non-ROS, system packages pkgnames_rosdep = self.get_rosdeps(package=pkg_name, implicit=implicit) for pkgname_rosdep in pkgnames_rosdep: - license_dict[MSG_LICENSE_NOTFOUND_SYSPKG].append(pkgname_rosdep) + license_dict[self.LICENSE_NOT_FOUND].append(pkgname_rosdep) - # Sort pkg names in each license + # Sort pkg names within the set of pkgs with each license for list_key in license_dict.values(): list_key.sort() - # Sort license names + # Sort licenspe names licenses = OrderedDict(sorted(license_dict.items())) return licenses diff --git a/test/test_rospkg_catkin_packages.py b/test/test_rospkg_catkin_packages.py index e86e3336..d359bc4c 100644 --- a/test/test_rospkg_catkin_packages.py +++ b/test/test_rospkg_catkin_packages.py @@ -73,7 +73,9 @@ def test_licenses(): def test_get_licenses(): """Check licenses from all packages in the dependency chain""" rospack = rospkg.rospack.RosPack(ros_paths=[search_path]) - licenses_list = [None, "MIT", "BSD", "LGPL"] # Licenses of 'bar and 'foo'. License for some deps are undetermined so 'None'. + # Union on the licenses of 'bar and 'foo'. + # License for some deps are undetermined so LICENSE_NOT_FOUND. + licenses_list = [rospack.LICENSE_NOT_FOUND, "MIT", "BSD", "LGPL"] licenses = rospack.get_licenses("bar", implicit=True) assert("MIT" in licenses) assert("BSD" in licenses) From 2b64f55b651696e2bbaf700148f309172e610e9f Mon Sep 17 00:00:00 2001 From: "Isaac I.Y. Saito" <130s@2000.jukuin.keio.ac.jp> Date: Tue, 5 May 2020 02:03:35 -0700 Subject: [PATCH 12/12] [rospack.py] Align with the order of package.xml entry (https://github.com/ros-infrastructure/rospkg/pull/133#discussion_r270960413). --- src/rospkg/manifest.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/rospkg/manifest.py b/src/rospkg/manifest.py index e3d6788a..b9592f2d 100644 --- a/src/rospkg/manifest.py +++ b/src/rospkg/manifest.py @@ -314,11 +314,11 @@ class Manifest(object): Object representation of a ROS manifest file (``manifest.xml`` and ``stack.xml``) """ __slots__ = [ - 'description', 'brief', + 'name', 'description', 'brief', 'author', 'license', 'licenses', 'license_url', 'url', 'depends', 'rosdeps', 'platforms', 'exports', 'version', - 'status', 'name', 'notes', + 'status', 'notes', 'unknown_tags', 'type', 'filename', 'is_catkin']