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

Support or forbid catkin and rosbuild manifest sibling #30

Closed
tkruse opened this issue Feb 25, 2013 · 21 comments
Closed

Support or forbid catkin and rosbuild manifest sibling #30

tkruse opened this issue Feb 25, 2013 · 21 comments
Assignees

Comments

@tkruse
Copy link
Contributor

tkruse commented Feb 25, 2013

See ros-infrastructure/bloom#94
Whether the ROS tooling infrastructure considers it a valid use case for a folder to contain both a rosbuild manifest (stack.xml or manifest.xml) and a package.xml should be agreed upon by all tools, and documented.

@jbohren
Copy link
Contributor

jbohren commented Mar 8, 2013

+1 to support

I think since it's so easy to just add a "USE_ROSBUILD" CMake variable to the rosbuild Makefile to switch between rosbuild and catkin, it would be really helpful to not have to maintain two forks of a package if someone wants to support Fuerte or electric.

Test case here: https://github.com/jhu-lcsr-forks/gscam/tree/hybrid-buildsystem

@jbohren
Copy link
Contributor

jbohren commented Mar 8, 2013

Or, additionally, if someone wants to enable people to keep using a package in a source checkout which is rosbuild-based.

@jack-oquin
Copy link
Contributor

+1 seems good to me

@jbohren
Copy link
Contributor

jbohren commented Mar 11, 2013

Does anyone know why this breaks currently? Shouldn't calling "rosmake" or "make" in a rosbuild directory just ignore the package.xml file?

@tkruse
Copy link
Contributor Author

tkruse commented Mar 11, 2013

If you can reproduce breakage, report the steps so that this symptom can be
fixed.

rospkg has been adapted for cooperation with catkin, so that rosbuild
package can depend on catkin packages. The case of dual packages was not
considered at the time, I guess.

On Mon, Mar 11, 2013 at 1:19 PM, Jonathan Bohren
notifications@git.luolix.topwrote:

Does anyone know why this breaks currently? Shouldn't calling "rosmake" or
"make" in a rosbuild directory just ignore the package.xml file?


Reply to this email directly or view it on GitHubhttps://github.com//issues/30#issuecomment-14710217
.

@jbohren
Copy link
Contributor

jbohren commented Mar 11, 2013

Yeah, I posted a link to a demonstration in an earlier post:

Test case here: https://github.com/jhu-lcsr-forks/gscam/tree/hybrid-buildsystem

@jbohren
Copy link
Contributor

jbohren commented Mar 11, 2013

All I do is create a package with both a package.xml file and a manifest.xml file, then add an additional CMake flag to the Makefile (rosbuild) so that when the package's CMakeLists is loaded, it switches to use a rosbuild block of code instead of a catkin block of code. Then, when I try to build it with rosbuild, everything is fine until it gets to the linking stage, at which point it can't seem to find any of the symbols that should have been linked in from roscpp.

@dirk-thomas
Copy link
Member

I tried to build the mentioned branch. With catkin it built fine. rosmake failed linking with missing symbols. The reason for that is that rospack find the package.xml file and treats it as a wet package and return no libraries to link against.

The logic in rospack could be changed to first look for manifest.xml and second for package.xml (ros/rospack#11).

Since any tool like rospack can not figure out if you use such a hybrid package as wet or dry I recommend to branch for this. It might be fixed with the upcoming rospack change but there might be also other corner cases and scenarios which might even be unfixable.

@jack-oquin
Copy link
Contributor

Dirk,

Why close this, yet?

The whole point was to be able to build a package either way from the same sources. That would be very helpful for developers supporting code for multiple ROS distributions. I realize it is not an issue for core ROS developers, because you always work in a different branch for each distro.

If I understand your fix correctly, you made it work for rosbuild-only instead of for catkin-only. We need something like Jonathan's idea for a "USE_ROSBUILD" CMake variable. I suppose that would require an additional rospack option with logic in the build to set it appropriately.

@dirk-thomas
Copy link
Member

I closed this issue because there is a pending pull request for rospack now as referenced in my last comment.

With that patch I can build the gscam package with catkin as well as with rosmake. I thought that was the original goal. If there is a specific use case which does not yet work with the proposed patch please describe it so that I can reproduce it.

@jack-oquin
Copy link
Contributor

Dirk,

That was the original goal. If it's working now, then I misunderstood the fix (easy to do).

Thanks for fixing this. I think it will help a lot.

How will I know when the patched version has been released, and I can start using it?

@dirk-thomas
Copy link
Member

@tfoote announces when package updates have been pushed to the public repo. The current state of all repos can be seen at http://ros.org/debbuild/. In the future the changelog file will contain the changes for each released version and the issues might be marked with the milestone of the version.

@jack-oquin
Copy link
Contributor

That's what I was worried about.

Starting with a closed issue, there is no straightforward way to find out when it has been released or what package version includes it.

@dirk-thomas
Copy link
Member

If you have a better proposal please post it to the corresponding discussion on the mailing list. Marking a bug report with the milestone of the version it will be released into is a very common practice.

@jack-oquin
Copy link
Contributor

This issue has no milestone right now.

@jbohren
Copy link
Contributor

jbohren commented Mar 12, 2013

@dirk-thomas Thanks for the fix!

@jack-oquin I think the PR referenced above should be assigned to a version number milestone once it's been merged, then people can know when to expect it. What about that?

@jack-oquin
Copy link
Contributor

I don't see how that helps, but this is not the right place to discuss development transparency.

@dirk-thomas
Copy link
Member

@jbohren Could you please check that the pending rospack change (ros/rospack#11) works for your use case?

@jbohren
Copy link
Contributor

jbohren commented Mar 12, 2013

@jack-oquin I'll try it out tonight

@jbohren
Copy link
Contributor

jbohren commented Mar 12, 2013

@jack-oquin I'm also going to try it with SMACH (this is where I really care about it, really)

dirk-thomas added a commit to ros/rospack that referenced this issue Mar 12, 2013
invert order of package type detection (dry before wet) (ros-infrastructure/rospkg#30)
@jbohren
Copy link
Contributor

jbohren commented Mar 13, 2013

@jack-oquin Yeah, the patch works for my fork of gscam!

PierrickKoch pushed a commit to PierrickKoch/robotpkg that referenced this issue Feb 21, 2014
While here, rename the package to ros-rospack, for consistency with other ros
packages.

The ChangeLog since 2.0.13 stops at the "groovy" boundary.

2.1.19 (2013-06-06)
-------------------
* modified command 'list-duplicates' to output the paths where the packages
  were found (`#3 <https://github.com/ros/rospack/issues/3>`_)
* modified 'rospack plugins' to not use rosdep
  (`#5 <https://github.com/ros/rospack/issues/5>`_)
* improve Windows support  (`#10 <https://github.com/ros/rospack/issues/10>`_)
* use find_package() for tinyxml (if available)

2.1.18 (2013-03-21)
-------------------
* invert order of package type detection (dry before wet)
  (`ros-infrastructure/rospkg#30 <https://github.com/ros/rospkg/issues/30>`_)

2.1.17 (2013-03-08)
-------------------
* output full pkg-config command in case of errors
  (`#8 <https://github.com/ros/rospack/issues/8>`_)
* handle None as return value for call_pkg_config
  (`#8 <https://github.com/ros/rospack/issues/8>`_)
* fix crawling to always recrawl when forced
  (`#9 <https://github.com/ros/rospack/issues/9>`_)

2.1.16 (2013-01-13)
-------------------
* fix segfault for command depends1 which ignores exceptions and calls
  isSysPackage again (`#4 <https://github.com/ros/rospack/issues/4>`_)

2.1.15 (2012-12-06)
-------------------
* first public release for Groovy
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants