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

Selection of dependency types with cmd line option #727

Closed
wants to merge 10 commits into from
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -27,4 +27,4 @@ testsetup:
echo "running rosdep tests"

test: testsetup
nosetests --with-coverage --cover-package=rosdep2 --with-xunit test
nosetests3 --with-coverage --cover-package=rosdep2 --with-xunit test
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
nosetests3 --with-coverage --cover-package=rosdep2 --with-xunit test
nosetests --with-coverage --cover-package=rosdep2 --with-xunit test

It was my bad. I accidentally committed it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If changes to the test runners are proposed I'd prefer them in a separate PR for ease of review.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Originally, the tests use nosetests (Python 2 version). I didn't intend to propose a test runner change. I changed it locally to try some stuff and committed it unintentionally. This suggestion reverts this change.

6 changes: 4 additions & 2 deletions src/rosdep2/lookup.py
Original file line number Diff line number Diff line change
Expand Up @@ -328,7 +328,7 @@ def get_resources_that_need(self, rosdep_name):
@staticmethod
def create_from_rospkg(rospack=None, rosstack=None,
sources_loader=None,
verbose=False):
verbose=False, dependency_types=[]):
"""
Create :class:`RosdepLookup` based on current ROS package
environment.
Expand All @@ -339,6 +339,8 @@ def create_from_rospkg(rospack=None, rosstack=None,
instance used to crawl ROS stacks.
:param sources_loader: (optional) Override SourcesLoader used
for managing sources.list data sources.
:param dependency_types: (optional) List of dependency types.
Allowed: "build", "buildtool", "run", "test"
Copy link
Contributor

@mateus-amarante mateus-amarante Oct 9, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Allowed: "build", "buildtool", "run", "test"
Allowed: "build", "buildtool", "build_export", "run", "test", "exec", "doc"

It also supports build_export, exec, and doc dependencies

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should these docs also link to the specification for dependency types either in catkin_pkg or the ROS REPs defining the package.xml format?

Copy link
Contributor

@mateus-amarante mateus-amarante Nov 12, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. The doc_depend seems to be introduced only in format 2 ( REP127, schema files ), so maybe it is better not to consider it.

"""
# initialize the loader
if rospack is None:
Expand All @@ -358,7 +360,7 @@ def create_from_rospkg(rospack=None, rosstack=None,

# Create the rospkg loader on top of the underlay
loader = RosPkgLoader(rospack=rospack, rosstack=rosstack,
underlay_key=underlay_key)
underlay_key=underlay_key, dependency_types=dependency_types)

# create our actual instance
lookup = RosdepLookup(rosdep_db, loader)
Expand Down
16 changes: 14 additions & 2 deletions src/rosdep2/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -123,16 +123,21 @@ class UsageError(Exception):
"""


def _get_default_RosdepLookup(options):
_global_options = None
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
_global_options = None
_global_options = None

This is not needed


def _get_default_RosdepLookup(options=None):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it is better to keep this

Suggested change
def _get_default_RosdepLookup(options=None):
def _get_default_RosdepLookup(options):

"""
Helper routine for converting command-line options into
appropriate RosdepLookup instance.
"""
global _global_options
if options is None:
options = _global_options
Comment on lines +133 to +135
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is no longer needed

Suggested change
global _global_options
if options is None:
options = _global_options

os_override = convert_os_override_option(options.os_override)
sources_loader = SourcesListLoader.create_default(sources_cache_dir=options.sources_cache_dir,
os_override=os_override,
verbose=options.verbose)
lookup = RosdepLookup.create_from_rospkg(sources_loader=sources_loader)
lookup = RosdepLookup.create_from_rospkg(sources_loader=sources_loader, dependency_types=options.dependency_types)
lookup.verbose = options.verbose
return lookup

Expand Down Expand Up @@ -295,6 +300,7 @@ def setup_environment_variables(ros_distro):

def _rosdep_main(args):
# sources cache dir is our local database.
global _global_options
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
global _global_options

This is not needed

default_sources_cache = get_sources_cache_dir()

parser = OptionParser(usage=_usage, prog='rosdep')
Expand Down Expand Up @@ -368,8 +374,14 @@ def _rosdep_main(args):
help="Affects the 'update' verb. "
'If specified end-of-life distros are being '
'fetched too.')
parser.add_option('-t', '--dependency-types', dest='dependency_types',
type="choice", choices=("build", "buildtool", "build_export", "exec", "run", "test", "doc"),
default=[], action='append',
help='Dependency types to install, can be given multiple times. '
'Chose from build, buildtool, build_export, exec, run, test, doc. Default: all except doc.')

options, args = parser.parse_args(args)
_global_options = options
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
_global_options = options

Not needed

if options.print_version or options.print_all_versions:
# First print the rosdep version.
print('{}'.format(__version__))
Expand Down
2 changes: 2 additions & 0 deletions src/rosdep2/rospack.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,9 @@ def __init__(self):
self.os_override = None
self.sources_cache_dir = get_sources_cache_dir()
self.verbose = False
self.dependency_types = []
lookup = _get_default_RosdepLookup(Options())

return lookup.get_rosdep_view(DEFAULT_VIEW_KEY)


Expand Down
31 changes: 29 additions & 2 deletions src/rosdep2/rospkg_loader.py
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@

class RosPkgLoader(RosdepLoader):

def __init__(self, rospack=None, rosstack=None, underlay_key=None):
def __init__(self, rospack=None, rosstack=None, underlay_key=None, dependency_types=[]):
"""
:param underlay_key: If set, all views loaded by this loader
will depend on this key.
Expand All @@ -78,6 +78,21 @@ def __init__(self, rospack=None, rosstack=None, underlay_key=None):
self._loadable_resource_cache = None
self._catkin_packages_cache = None

# Dependency types to include
def check_dep(dep_type):
if dependency_types:
return dep_type in dependency_types
else:
return dep_type in ['buildtool', 'build', 'build_export', 'exec', 'test']

self.include_build_depends = check_dep('build')
self.include_buildtool_depends = check_dep('buildtool')
self.include_build_export_depends = check_dep('build_export') or check_dep('run')
self.include_exec_depends = check_dep('exec') or check_dep('run')
self.include_test_depends = check_dep('test')
self.include_doc_depends = check_dep('doc')


def load_view(self, view_name, rosdep_db, verbose=False):
"""
Load view data into *rosdep_db*. If the view has already
Expand Down Expand Up @@ -140,7 +155,19 @@ def get_rosdeps(self, resource_name, implicit=True):
if resource_name in self.get_catkin_paths():
pkg = catkin_pkg.package.parse_package(self.get_catkin_paths()[resource_name])
pkg.evaluate_conditions(os.environ)
deps = pkg.build_depends + pkg.buildtool_depends + pkg.run_depends + pkg.test_depends + pkg.buildtool_export_depends
deps = []
if self.include_build_depends:
deps += pkg.build_depends
if self.include_buildtool_depends:
deps += pkg.buildtool_depends
if self.include_build_export_depends:
deps += pkg.build_export_depends
if self.include_exec_depends:
deps += pkg.exec_depends
if self.include_test_depends:
deps += pkg.test_depends
if self.include_doc_depends:
deps += pkg.doc_depends
return [d.name for d in deps if d.evaluated_condition]
elif resource_name in self.get_loadable_resources():
rosdeps = set(self._rospack.get_rosdeps(resource_name, implicit=False))
Expand Down
45 changes: 45 additions & 0 deletions test/test_rosdep_lookup.py
Original file line number Diff line number Diff line change
Expand Up @@ -329,6 +329,51 @@ def test_RosdepLookup_get_rosdeps():
assert set(lookup.get_rosdeps('metapackage_with_deps', implicit=False)) == set(['catkin', 'simple_catkin_package', 'another_catkin_package'])


def test_RosdepLookup_dependency_types():
from rosdep2.loader import RosdepLoader
from rosdep2.lookup import RosdepLookup
rospack, rosstack = get_test_rospkgs()

sources_loader = create_test_SourcesListLoader()
default_lookup = RosdepLookup.create_from_rospkg(rospack=rospack, rosstack=rosstack,
sources_loader=sources_loader)
buildtool_lookup = RosdepLookup.create_from_rospkg(rospack=rospack, rosstack=rosstack,
sources_loader=sources_loader, dependency_types=['buildtool'])
build_lookup = RosdepLookup.create_from_rospkg(rospack=rospack, rosstack=rosstack,
sources_loader=sources_loader, dependency_types=['build'])
build_export_lookup = RosdepLookup.create_from_rospkg(rospack=rospack, rosstack=rosstack,
sources_loader=sources_loader, dependency_types=['build_export'])
exec_lookup = RosdepLookup.create_from_rospkg(rospack=rospack, rosstack=rosstack,
sources_loader=sources_loader, dependency_types=['exec'])
run_lookup = RosdepLookup.create_from_rospkg(rospack=rospack, rosstack=rosstack,
sources_loader=sources_loader, dependency_types=['run'])
test_lookup = RosdepLookup.create_from_rospkg(rospack=rospack, rosstack=rosstack,
sources_loader=sources_loader, dependency_types=['test'])
doc_lookup = RosdepLookup.create_from_rospkg(rospack=rospack, rosstack=rosstack,
sources_loader=sources_loader, dependency_types=['doc'])
mix_lookup = RosdepLookup.create_from_rospkg(rospack=rospack, rosstack=rosstack,
sources_loader=sources_loader, dependency_types=['build', 'build_export'])

buildtool_deps = ['catkin']
build_deps = ['testboost', 'eigen']
build_export_deps = ['eigen', 'testtinyxml']
exec_deps = ['eigen', 'testlibtool']
run_deps = ['eigen', 'testlibtool', 'testtinyxml'] # build_export + exec
test_deps = ['curl']
doc_deps = ['epydoc']
default_deps = buildtool_deps + build_deps + build_export_deps + exec_deps + test_deps

assert set(buildtool_lookup.get_rosdeps('multi_dep_type_catkin_package')) == set(buildtool_deps)
assert set(build_lookup.get_rosdeps('multi_dep_type_catkin_package')) == set(build_deps)
assert set(build_export_lookup.get_rosdeps('multi_dep_type_catkin_package')) == set(build_export_deps)
assert set(exec_lookup.get_rosdeps('multi_dep_type_catkin_package')) == set(exec_deps)
assert set(run_lookup.get_rosdeps('multi_dep_type_catkin_package')) == set(run_deps)
assert set(test_lookup.get_rosdeps('multi_dep_type_catkin_package')) == set(test_deps)
assert set(mix_lookup.get_rosdeps('multi_dep_type_catkin_package')) == set(build_deps + build_export_deps)
assert set(default_lookup.get_rosdeps('multi_dep_type_catkin_package')) == set(default_deps)
assert set(doc_lookup.get_rosdeps('multi_dep_type_catkin_package')) == set(doc_deps)


def test_RosdepLookup_get_resources_that_need():
from rosdep2.lookup import RosdepLookup
rospack, rosstack = get_test_rospkgs()
Expand Down
6 changes: 6 additions & 0 deletions test/test_rosdep_main.py
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,12 @@ def test_keys(self):
rosdep_main(['keys', 'another_catkin_package'] + cmd_extras + ['-i'])
stdout, stderr = b
assert stdout.getvalue().strip() == 'catkin', stdout.getvalue()
with fakeout() as b:
rosdep_main(['keys', 'multi_dep_type_catkin_package', '-t', 'test', '-t', 'doc'] + cmd_extras)
stdout, stderr = b
output_keys = set(stdout.getvalue().split())
expected_keys = set(['curl', 'epydoc'])
assert output_keys == expected_keys, stdout.getvalue()
except SystemExit:
assert False, 'system exit occurred'
try:
Expand Down
19 changes: 19 additions & 0 deletions test/tree/catkin/multi_dep_type_catkin_package/package.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
<package format="2">
<name>multi_dep_type_catkin_package</name>
<version>0.0.0</version>
<description>
This is a package with depencencies of multiple types
</description>
<maintainer email="[email protected]">Nobody</maintainer>
<license>BSD</license>

<buildtool_depend>catkin</buildtool_depend>

<depend>eigen</depend>
<build_depend>testboost</build_depend>
<build_export_depend>testtinyxml</build_export_depend>
<exec_depend>testlibtool</exec_depend>
<test_depend>curl</test_depend>
<doc_depend>epydoc</doc_depend>

</package>