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.

Sign up for GitHub

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

Jump to bottom

do not overwrite pkg_map #42

Merged
merged 6 commits into from
Nov 21, 2015
Merged

do not overwrite pkg_map #42

merged 6 commits into from
Nov 21, 2015

Conversation

k-okada
Copy link
Member

@k-okada k-okada commented Nov 21, 2015

background:

  • ros-indigo-roseus is already installed
  • jsk_roseus is in workspace too

problem:
get_pkg_map returns roseus package under /opt/ros/indigo, not workspace

failure case:
https://travis-ci.org/jsk-ros-pkg/geneus/builds/92400732

-- catkin 0.6.16
-- Using these message generators: gencpp;geneus;genlisp;genpy
CMake Error at /opt/ros/indigo/share/catkin/cmake/catkinConfig.cmake:75 (find_package):
  Could not find a package configuration file provided by
  "actionlib_tutorials" with any of the following names:
    actionlib_tutorialsConfig.cmake
    actionlib_tutorials-config.cmake
  Add the installation prefix of "actionlib_tutorials" to CMAKE_PREFIX_PATH
  or set "actionlib_tutorials_DIR" to a directory containing one of the above
  files.  If "actionlib_tutorials" provides a separate development package or
  SDK, be sure it has been installed.
Call Stack (most recent call first):
  CMakeLists.txt:7 (find_package)

which i added actionlib_tutorials to the run_depend, but not released yet

k-okada added a commit to k-okada/jsk_roseus that referenced this pull request Nov 21, 2015
@k-okada
hot fix until jsk-ros-pkg/geneus#42 has released
a74a1ab
k-okada added a commit to k-okada/geneus that referenced this pull request Nov 21, 2015
@k-okada
add test code to check jsk-ros-pkg#42 isseus
3be82ec
@k-okada k-okada mentioned this pull request Nov 21, 2015
Closed
wkentaro
wkentaro reviewed Nov 21, 2015
View reviewed changes
src/geneus/geneus_main.py
# packages.find_packages(workspaces.get_spaces()) returns package in high-priority-first-order, so we should not overwirte package map which is already found
# https://github.com/ros-infrastructure/catkin_pkg/blob/fa4b136b16e2d2886ab97257684f6bff243edefb/src/catkin_pkg/workspaces.py#L43
# https://github.com/ros-infrastructure/catkin_pkg/blob/fa4b136b16e2d2886ab97257684f6bff243edefb/src/catkin_pkg/packages.py#L71
if not pkg_map.has_key(pkg.name):
Copy link
Member

Choose a reason for hiding this comment

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

Why not use in not has_key?
has_key will be unsupported in Python 3.4.

so it will be >>>>>

if pkg.name not in pkg_map:

Copy link
Member Author

Choose a reason for hiding this comment

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

thx

@k-okada k-okada force-pushed the fix_overwrite branch 2 times, most recently from 7afbfe4 to 2969421 Compare November 21, 2015 08:04
k-okada added a commit to k-okada/geneus that referenced this pull request Nov 21, 2015
@k-okada
add test code to check jsk-ros-pkg#42 isseus
2969421
k-okada added a commit to k-okada/geneus that referenced this pull request Nov 21, 2015
@k-okada
add test code to check jsk-ros-pkg#42 isseus
29cb8d7
k-okada added a commit to k-okada/geneus that referenced this pull request Nov 21, 2015
@k-okada
add test code to check jsk-ros-pkg#42 isseus
c30c2d4
k-okada added a commit that referenced this pull request Nov 21, 2015
@k-okada
Merge pull request #42 from k-okada/fix_overwrite
2b7a141 
do not overwrite pkg_map
@k-okada k-okada merged commit 2b7a141 into jsk-ros-pkg:master Nov 21, 2015
@k-okada k-okada deleted the fix_overwrite branch November 21, 2015 08:58
@k-okada k-okada mentioned this pull request Nov 21, 2015
Merged
k-okada added a commit to jsk-ros-pkg/jsk_roseus that referenced this pull request Nov 21, 2015
@k-okada
Merge pull request #409 from k-okada/use_local_package_xml
feb5cc9 
hot fix until jsk-ros-pkg/geneus#42 has released
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@k-okada @wkentaro