Make roslaunch-check respect if/unless attribute on <include> #998

mikepurvis · 2017-02-24T02:52:04Z

As discussed in #993. With the fix removed, the updated test XML fails with:

======================================================================
ERROR: unit.test_roslaunch_depends.test_roslaunch_deps
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/usr/lib/python2.7/dist-packages/nose/case.py", line 197, in runTest
    self.test(*self.arg)
  File "/home/administrator/rocksteady_robot_ws/src/roslaunch/test/unit/test_roslaunch_depends.py", line 157, in test_roslaunch_deps
    base_pkg, file_deps, missing = roslaunch_deps(files, verbose=v)
  File "/home/administrator/rocksteady_robot_ws/src/roslaunch/src/roslaunch/depends.py", line 331, in roslaunch_deps
    rl_file_deps(file_deps, launch_file, verbose)
  File "/home/administrator/rocksteady_robot_ws/src/roslaunch/src/roslaunch/depends.py", line 225, in rl_file_deps
    parse_launch(launch_file, file_deps, verbose)
  File "/home/administrator/rocksteady_robot_ws/src/roslaunch/src/roslaunch/depends.py", line 210, in parse_launch
    _parse_launch(launch_tag.childNodes, launch_file, file_deps, verbose, context)
  File "/home/administrator/rocksteady_robot_ws/src/roslaunch/src/roslaunch/depends.py", line 182, in _parse_launch
    raise RoslaunchDepsException("Cannot load roslaunch include '%s' in '%s'"%(sub_launch_file, launch_file))
RoslaunchDepsException: Cannot load roslaunch include 'does/not/exist.launch' in '/home/administrator/rocksteady_robot_ws/src/roslaunch/resources/example.launch

mikepurvis · 2017-02-24T03:15:45Z

@dirk-thomas Getting a dependency failure here on the Lunar buildfarm:

KeyError: "The cache has no package named 'ros-lunar-rosbuild'"

I'm assuming we should be targeting Lunar with new changes now, and considering Kinetic for backport?

Oh blah, I see that there are a number of broken/missing packages at the moment, including rosbuild: http://repositories.ros.org/status_page/ros_lunar_default.html?q=rosbuild

mikepurvis · 2017-02-24T14:23:17Z

tools/roslaunch/src/roslaunch/depends.py

+            elif tag.attributes.has_key('unless'):
+                val = resolve_args(tag.attributes['unless'].value, context)
+                if convert_value(val, 'bool'):
+                    continue


Another possibility would be moving this logic up a level so that it would also do the right thing on <group> and <node>— not currently the case.

This is what I've done instead in df0dfb8.

Is this block still necessary with _check_ifunless being called at the beginning of the loop?

Oh— no it isn't, but I forgot to remove it. Fixed now.

Looks like it's still rospack 2.4.0 in the shadow repo, so this isn't going to pass yet.

rospack 2.4.1 has propagated to shadow-fixed already: http://repositories.ros.org/status_page/ros_lunar_default.html?q=rospack

Other jobs have turned green.

mikepurvis · 2017-02-27T21:26:34Z

@dirk-thomas I think this is good to go, the failed tests appear to be unrelated to this change.

dirk-thomas · 2017-02-27T21:28:30Z

The failing tests might be related to ros/rospack#70. I will retrigger the PR jobs once the new version of rospack has been synced to the testing repo.

dirk-thomas · 2017-02-27T22:17:44Z

@ros-pull-request-builder retest this please

mikepurvis · 2017-02-28T02:41:27Z

Most recent test got rospack 2.4.1 and ran clean.

dirk-thomas · 2017-03-02T20:57:53Z

tools/roslaunch/src/roslaunch/depends.py

-    elif tag.attributes.has_key('unless'):
+        if not convert_value(val, 'bool'):
+            return False
+    if tag.attributes.has_key('unless'):


Previously if a tag had both - an if as well as an unless attribute the second one wasn't considered. Is there a reason to change this?

Oh I suppose not; I'll change it back.

mikepurvis · 2017-04-28T20:06:38Z

@ros/ros_comm-maintainers This should be good to go in. Anyone got thoughts?

IanTheEngineer · 2017-04-28T20:57:11Z

Not a maintainer, but changes look good on my end. Is this a candidate for backporting into Kinetic?

dirk-thomas · 2017-04-28T21:18:22Z

@mikepurvis Looks good to me 👍

@IanTheEngineer First it will be released into Lunar to give it some soak time. It can be backported if there is a strong need for it, the chance for regressions are low, and it doesn't change API.

…comm#953 could not load launch file with args directory

…_comm#953 could not load launch file with args directory

mikepurvis added the hitlist label Feb 24, 2017

mikepurvis commented Feb 24, 2017

View reviewed changes

mikepurvis added a commit that referenced this pull request Feb 24, 2017

Make roslaunch-check respect if/unless attribute on <include> (#998)

2d94db2

mikepurvis force-pushed the fix-993 branch 2 times, most recently from 781aea4 to 7ffd338 Compare February 27, 2017 20:22

mikepurvis force-pushed the fix-993 branch from 7ffd338 to 85e00be Compare February 27, 2017 21:28

mikepurvis force-pushed the fix-993 branch from 85e00be to 73f5780 Compare February 27, 2017 21:31

mikepurvis force-pushed the fix-993 branch from 73f5780 to dba7d80 Compare February 28, 2017 00:14

mikepurvis added a commit that referenced this pull request Mar 2, 2017

Make roslaunch-check respect if/unless attribute on <include> (#998)

8033da9

dirk-thomas reviewed Mar 2, 2017

View reviewed changes

mikepurvis force-pushed the fix-993 branch 2 times, most recently from 0efa6ca to 83fb7c9 Compare March 10, 2017 19:42

Respect if/unless for roslaunch-check.

bb4eeec

mikepurvis force-pushed the fix-993 branch from 83fb7c9 to bb4eeec Compare March 10, 2017 19:44

mikepurvis added a commit that referenced this pull request Mar 28, 2017

Make roslaunch-check respect if/unless attribute on <include> (#998)

e373bee

dirk-thomas merged commit a0f4789 into lunar-devel Apr 28, 2017

dirk-thomas deleted the fix-993 branch April 28, 2017 21:18

dirk-thomas mentioned this pull request Apr 28, 2017

[roslaunch] roslaunch-check doesn't respect if/unless #993

Closed

mikepurvis removed the hitlist label Apr 28, 2017

k-okada added a commit to k-okada/jsk_robot that referenced this pull request Jul 12, 2017

ad roslaunch_depends.py from ros/ros_comm#998 to support if, ros/ros_…

3910eeb

…comm#953 could not load launch file with args directory

k-okada mentioned this pull request Jul 12, 2017

Enable safe teleop for fetch jsk-ros-pkg/jsk_robot#801

Merged

k-okada added a commit to k-okada/jsk_robot that referenced this pull request Jul 12, 2017

add roslaunch_depends.py from ros/ros_comm#998 to support if, ros/ros…

1a41de8

…_comm#953 could not load launch file with args directory

dirk-thomas mentioned this pull request Oct 26, 2017

changes between 1.12.7 and 1.13.4 for backporting #1205

Merged

miguelprada mentioned this pull request May 9, 2018

[roslaunch] roslaunch-check only considers "default" path #1394

Open

k-okada added a commit to furushchev/jsk_robot that referenced this pull request Jul 11, 2018

use ros/ros_comm#998 to respect if/false

df875c9

k-okada added a commit to furushchev/jsk_robot that referenced this pull request Jul 12, 2018

use ros/ros_comm#998 to respect if/false

ae2a076

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make roslaunch-check respect if/unless attribute on <include> #998

Make roslaunch-check respect if/unless attribute on <include> #998

mikepurvis commented Feb 24, 2017

mikepurvis commented Feb 24, 2017 •

edited

Loading

mikepurvis Feb 24, 2017

mikepurvis Feb 25, 2017 •

edited

Loading

dirk-thomas Feb 27, 2017

mikepurvis Feb 28, 2017

dirk-thomas Feb 28, 2017

mikepurvis commented Feb 27, 2017

dirk-thomas commented Feb 27, 2017

dirk-thomas commented Feb 27, 2017

mikepurvis commented Feb 28, 2017

dirk-thomas Mar 2, 2017

mikepurvis Mar 10, 2017

mikepurvis commented Apr 28, 2017

IanTheEngineer commented Apr 28, 2017

dirk-thomas commented Apr 28, 2017

Make roslaunch-check respect if/unless attribute on <include> #998

Make roslaunch-check respect if/unless attribute on <include> #998

Conversation

mikepurvis commented Feb 24, 2017

mikepurvis commented Feb 24, 2017 • edited Loading

mikepurvis Feb 24, 2017

Choose a reason for hiding this comment

mikepurvis Feb 25, 2017 • edited Loading

Choose a reason for hiding this comment

dirk-thomas Feb 27, 2017

Choose a reason for hiding this comment

mikepurvis Feb 28, 2017

Choose a reason for hiding this comment

dirk-thomas Feb 28, 2017

Choose a reason for hiding this comment

mikepurvis commented Feb 27, 2017

dirk-thomas commented Feb 27, 2017

dirk-thomas commented Feb 27, 2017

mikepurvis commented Feb 28, 2017

dirk-thomas Mar 2, 2017

Choose a reason for hiding this comment

mikepurvis Mar 10, 2017

Choose a reason for hiding this comment

mikepurvis commented Apr 28, 2017

IanTheEngineer commented Apr 28, 2017

dirk-thomas commented Apr 28, 2017

mikepurvis commented Feb 24, 2017 •

edited

Loading

mikepurvis Feb 25, 2017 •

edited

Loading