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

[geneus_main.py] Resolve package dependencies with attention to the order #18

Merged
merged 9 commits into from
Apr 21, 2015

Conversation

wkentaro
Copy link
Member

After #17

@k-okada
Copy link
Member

k-okada commented Apr 14, 2015

ok, restarted travis to pass the test.

@wkentaro
Copy link
Member Author

It fails because of this line.
https://github.com/jsk-ros-pkg/jsk_roseus/blob/master/roseus/test/test-genmsg.sh#L349

catkin config does not have --no-jobserver option.

@wkentaro
Copy link
Member Author

So we should fix it first.

@k-okada
Copy link
Member

k-okada commented Apr 14, 2015

jsk-ros-pkg/jsk_roseus#271 may solve this
2015年4月14日火曜日、Kentaro Wadanotifications@github.comさんは書きました:

So we should fix it first.


Reply to this email directly or view it on GitHub
#18 (comment).

◉ Kei Okada

@k-okada
Copy link
Member

k-okada commented Apr 15, 2015

can you rebase origin/master?

@wkentaro
Copy link
Member Author

I rebased it.

@wkentaro
Copy link
Member Author

In my env using latest jsk_roseus which is installed from github, the test passes.
The travis test downloads latest test scripts, but does not download other scripts.
Are there any dependencies on test-genmsg.sh or generate-all-msg-srv.sh which should be latest version?

@wkentaro
Copy link
Member Author

I think the test failure is because of this line.
https://github.com/jsk-ros-pkg/jsk_roseus/blob/master/roseus/test/test-genmsg.sh#L81

This means geneus_dep1 depends on roseus, right?
and roseus depends on angles but there is no angles package in the test environment. That's why this travis test fails.

@wkentaro
Copy link
Member Author

I committed temporary one for travis test.
4612ef0
So DON'T MERGE THIS

@wkentaro wkentaro force-pushed the fix_depends2 branch 7 times, most recently from 2ff3c1a to a650354 Compare April 16, 2015 14:45
@k-okada
Copy link
Member

k-okada commented Apr 17, 2015

by the way, it there any reason that you did not use implicit depends?

@k-okada k-okada closed this Apr 17, 2015
@k-okada k-okada reopened this Apr 17, 2015
@wkentaro
Copy link
Member Author

What line are you pointing at?

2015年4月17日金曜日、Kei Okadanotifications@github.comさんは書きました:

by the way, it there any reason that you did not use implicit depends?


Reply to this email directly or view it on GitHub
#18 (comment).

和田 健太郎 / Kentaro Wada
http://wkentaro.com http://www.wkentaro.com

@k-okada
Copy link
Member

k-okada commented Apr 17, 2015

Humm,
you use get_depends(True) at
https://github.com/jsk-ros-pkg/geneus/pull/18/files#diff-3b84e240f81b3651ac8336f3a5a15846R75
and this try to find build depends, can we use get_depends(Flase) ?

@wkentaro
Copy link
Member Author

Okay I chaned it.

@k-okada
Copy link
Member

k-okada commented Apr 17, 2015

humm, still try to find angles (buidl_depend)

[WARNING] cmake_modules is not found in ROS_PACKAGE_PATH 
[WARNING] mk is not found in ROS_PACKAGE_PATH 
Traceback (most recent call last): 
  File "/home/travis/ros/ws_geneus/src/geneus/src/geneus/geneus_main.py", line 120, in genmain 
    pkg_dependences = package_depends(pkg) + args[2:]   # args[1] ARG_PKG 
  File "/home/travis/ros/ws_geneus/src/geneus/src/geneus/geneus_main.py", line 71, in package_depends 
    for d in solve_order_of_depends(depends_impl): 
  File "/home/travis/ros/ws_geneus/src/geneus/src/geneus/geneus_main.py", line 61, in solve_order_of_depends 
    unsolved = filter(lambda x: x not in solved, rp.get_depends(d, False)) 
  File "/usr/lib/python2.7/dist-packages/rospkg/rospack.py", line 223, in get_depends 
    m = self.get_manifest(name) 
  File "/usr/lib/python2.7/dist-packages/rospkg/rospack.py", line 163, in get_manifest 
    return self._load_manifest(name) 
  File "/usr/lib/python2.7/dist-packages/rospkg/rospack.py", line 207, in _load_manifest 
    retval = self._manifests[name] = parse_manifest_file(self.get_path(name), self._manifest_name, rospack=self) 
  File "/usr/lib/python2.7/dist-packages/rospkg/rospack.py", line 199, in get_path 
    raise ResourceNotFound(name, ros_paths=self._ros_paths) 
ResourceNotFound: angles 
ROS path [0]=/opt/ros/hydro/share/ros 
ROS path [1]=/tmp/test_genmsg_18470/src/geneus_dep1 
ROS path [2]=/home/travis/ros/ws_geneus/src/geneus 
ROS path [3]=/opt/ros/hydro/share 
ROS path [4]=/opt/ros/hydro/stacks

@wkentaro
Copy link
Member Author

I changed to see only run_depend in package.xml referring to #20.

@k-okada
Copy link
Member

k-okada commented Apr 20, 2015

very nice, hopefully this is the last question, what is the difference between ros_depends and tmp_depends

@wkentaro
Copy link
Member Author

ros_depends are dependencies which all are ROS packages.

get_depends() collects package dependencies from package.xml so it can contain non ROS packages. (in other words python packages (in many cases) are removed when storing data to ros_depends)
https://github.com/jsk-ros-pkg/geneus/pull/18/files#diff-3b84e240f81b3651ac8336f3a5a15846R103

tmp_depends are dependencies which are not included by depends (which is arg in recursive function).
by filtering here, tmp_depends only includes ROS packages.

@k-okada
Copy link
Member

k-okada commented Apr 21, 2015

ok, so can we merge this one?

@wkentaro
Copy link
Member Author

I think this does work, please merge this.

k-okada added a commit that referenced this pull request Apr 21, 2015
[geneus_main.py] Resolve package dependencies with attention to the order
@k-okada k-okada merged commit 61856d2 into jsk-ros-pkg:master Apr 21, 2015
@k-okada
Copy link
Member

k-okada commented Apr 21, 2015

This will close #21, close #20 ,close #24

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.

2 participants