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

Update collada_urdf repo url #95

Closed
wants to merge 1 commit into from
Closed

Conversation

sloretz
Copy link

@sloretz sloretz commented May 6, 2017

Updates the url to the upstream repository containing collada_urdf.

Solves #94

A more robust fix would use rosinstall_generator and rosinstall to download the source. Realistically the collada_urdf package is not going to move again prior to indigo EOL, so the extra effort would be wasted.

cc @mikaelarguedas

Updates the url to the collada_urdf upstream repository
@sloretz sloretz changed the title Update collada URDF Update collada_urdf repo url May 6, 2017
@mikaelarguedas
Copy link
Contributor

tested this change in an indigo docker container and I can confirm it fixes the problem.

A more robust fix would use rosinstall_generator and rosinstall to download the source

Indeed scripts should not rely on hardcoded path in general. In this case I agree that it's not expected that this package changes location before Indigo goes End of life so 👍 for the current fix

@sloretz
Copy link
Author

sloretz commented May 6, 2017

@k-okada what's the best way to get a successful travis build?

Looks like it failed because the new repo doesn't have a hydro-devel branch.

[collada_urdf_jsk_patch] Cloning into 'build/robot_model/src'...
[collada_urdf_jsk_patch] error: pathspec 'hydro-devel' did not match any file(s) known to git.
[collada_urdf_jsk_patch] make[3]: *** [build/robot_model/src] Error 1
[collada_urdf_jsk_patch] make[2]: *** [/home/travis/ros/ws_jsk_3rdparty/devel/lib/collada_urdf_jsk_patch/urdf_to_collada] Error 2
[collada_urdf_jsk_patch] make[1]: *** [CMakeFiles/urdf_to_collada.dir/all] Error 2
[collada_urdf_jsk_patch] make: *** [all] Error 2
[collada_urdf_jsk_patch] <== '/home/travis/ros/ws_jsk_3rdparty/build/collada_urdf_jsk_patch/build_env.sh /usr/bin/make --jobserver-fds=3,4 -j' failed with return code '2'
Failed <== collada_urdf_jsk_patch   [ 2.1 seconds ]

@k-okada
Copy link
Member

k-okada commented May 8, 2017

see #96 for workaround for #94, but another fix is to revert ros/robot_model#197, https://github.com/ros/rosdistro/pull/14898/files and https://github.com/ros/rosdistro/pull/14859/files,

I could understand what you're going to do in ros/robot_model#195, That's OK. But that's is Lunar or (kinetic-devel) problem, not Indigo nor Jade (indigo-devel). Indigo is released on 2014 and will be used until 2019. so it's already middle of life and working well until now for everyone.
I'm really appreciate @sloretz @mikaelarguedas @tfoote look into this problem and feel sorry for you to spend time on try to fix it, even creating PR for this, but I think best strategy was to keep untouched until it's EOF. This should make everyone happy.
At this moment, I'm ready for both way, revert robot-model's indigo-devel or release jsk_3rdparty, but I'm be grad if you keep following in your mind for future release.

  • There might be someone who using your repository in unexpected way
  • If you want to change something try on new branch, that's why we have new ROS distro every year and different branch for that.

Best

@sloretz
Copy link
Author

sloretz commented May 8, 2017

I agree that packages must keep a stable interface during releases. The question then becomes what interfaces must maintainers guarantee. I think the location of the source code shouldn't be one of them.

In general a maintainer can't guarantee github will still be around in 2019. In this case I'm concerned maintaining collada_urdf at two repositories will be a burden. Pull requests will have to be pushed between both repositories. I think it will be difficult for users to determine where they should open issues.

I think a maintainer also can't guarantee code changes will not conflict with downstream patches. In the worst case an upstream change to collada_urdf could result in collada_urdf_jsk_patch building but not working as intended. This seems like a non-trivial risk given the small number of tests in collada_parser and collada_urdf. If you see value in it, I recommend an alternative workflow of forking the upstream repository, applying the patches then committing, and manually pulling when upstream updates are desired. This gives an opportunity to check if the package still works as intended after merging.

@sloretz
Copy link
Author

sloretz commented Feb 27, 2019

Since it's been a while, I'll assume this is PR isn't needed anymore and close it.

@sloretz sloretz closed this Feb 27, 2019
@k-okada
Copy link
Member

k-okada commented Feb 28, 2019 via email

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.

3 participants