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

Revert "cartographer: 0.2.0-0 in 'lunar/distribution.yaml' [bloom]" #15984

Merged
merged 1 commit into from
Sep 28, 2017

Conversation

mikaelarguedas
Copy link
Member

Reverts #15983

Actually 0.2.0 is missing at least some cmake changes to support debian packaging (e.g. cartographer-project/cartographer@e345ae8). I'm going to remove this from the index and make a new release when 0.3.0 is tagged

@k-okada
Copy link
Contributor

k-okada commented Mar 12, 2018

since it seems we need to solve several issues before releasing latest cardographer (#16823)
how about releasing with 0.2.0? I have confirmed with adding cmake fix (see last commits https://github.com/k-okada/cartographer-release/commits/master)

@mikaelarguedas
Copy link
Member Author

Thanks @k-okada for following up on this.
Given the limited user base using Lunar and that I couldn't find time to confirm that the CMake fix was the only one needed to successfully build debs, I figured I would just release cartographer 0.3.0 in the upcoming ROS Melodic (that should have all the dependencies available upstream). If you have interest in providing the patches necessary for Lunar on the release repo and making the initial release I can happily give you write access to the release repository.

@k-okada
Copy link
Contributor

k-okada commented Mar 16, 2018

@mikaelarguedas thanks for offer, yeah, I could help to release Kinetic/Lunar release until Melogic is out.

Before releasing into these distro, @7675t. Can you check if ros-kinetic-cartographer_0.2.0-2xenial_amd64.deb works ok? I just confirmed if this is compilable on kinetic, but not sure if it actually runs .

@mikaelarguedas
Copy link
Member Author

Note: I backported the dependencies only for Lunar targeted platforms. I don't expect the dependencies to be satisfied for Debian Jessie (thread here)

@7675t
Copy link
Contributor

7675t commented Mar 17, 2018

@k-okada
I tried the deb file with my laptop, I can install it with gdebi and no errors.
However, it seems not contain some required binary, such as cartographer_node and cartographer_occupancy_grid_node . Please check your packaging.

The result of dpkg -L:

dpkg-L.txt

@k-okada
Copy link
Contributor

k-okada commented Mar 18, 2018

I see, I think they are installed from cartographer_ros package, (see
https://github.com/googlecartographer/cartographer_ros/blob/master/cartographer_ros/cartographer_ros/CMakeLists.txt#L66)
so we have to checkout that package and compile them.

I have update track.yalm for them https://github.com/k-okada/cartographer_ros-release/ and created deb file baesd on that. Note that we have to use 0.2.0 of cartographer_ros,

https://github.com/k-okada/cartographer_ros-release

please check with these files
https://drive.google.com/open?id=1R1v93gNXNal7yll_dLhs52Mzc1IT0g1-
https://drive.google.com/open?id=1VCVXZPAmgfq1ru3U1phqCIF3ntA4bKc9

@7675t
Copy link
Contributor

7675t commented Mar 19, 2018

I have confirmed they work correctly with my bag file.

Some points:

  • occupancy_grid_node is added from ver 0.3
  • configuration files are not compatible between versions
  • So, we seems good to use the latest version

What is the reason to use 0.2 instead of 0.3?

@7675t
Copy link
Contributor

7675t commented Mar 19, 2018

Ah, yes, it is described on the top of the thread. Sorry.

@k-okada
Copy link
Contributor

k-okada commented Mar 19, 2018 via email

@7675t
Copy link
Contributor

7675t commented Mar 19, 2018

Yes, I can make a map correctly with v0.2 deb files, removing occupancy_grid_node.

@k-okada
Copy link
Contributor

k-okada commented Mar 19, 2018

@mikaelarguedas we’re ready for release kinetic/lunar. Can you give us a write permission to relevant repositories? Thanks in advance

@mikaelarguedas
Copy link
Member Author

@k-okada great!

2-3 things we need to check/do before releasing:

  • confirm with the Cartographer owners that they invited the buildfarm to post messages on cartographer-owners@googlegroups.com and update the maintainer tag in the package.xml accordingly. If not who should be listed as a maintainer? @7675t and yourself ?
  • Re Revert "cartographer: 0.2.0-0 in 'lunar/distribution.yaml' [bloom]" #15984 (comment): can you point me to the deb you created for ceres 1.12.0 on Debian Jessie? I will need to import it in the ROS repositories before we can release for Kinetic
  • move the patches to the package.xml to a distro specific patch folder as the list of dependencies is different between 0.2 and 0.3 and the file would be overwritten if we keep the patched version at the root of the master branch

Thanks!

@mikaelarguedas
Copy link
Member Author

confirm with the Cartographer owners that they invited the buildfarm to post messages on cartographer-owners@googlegroups.com

I pinged the cartographer maintainers here for confirmation

@k-okada
Copy link
Contributor

k-okada commented Mar 20, 2018

@mikaelarguedas
Copy link
Member Author

@k-okada I gave you access to https://github.com/ros-gbp/cartographer-release and https://github.com/ros-gbp/cartographer_ros-release yesterday so that we can make the changes directly on the ros-gbp repo. Should @7675t be added as a contributor as well?

What do you mean by "point me to the deb you created for ceres 1.12.0 on Debian Jessie"?

What I meant is that there is no package libceres-dev 1.12.0 in Debian Jessie (that is the version required by cartographer 0.2). And I didn't go through the effort of backporting ceres and its dependencies to Jessie, only for the platforms targeted by Lunar (see discourse thread).
As backporting on Jessie will likely be a significant amount of work and given that Jessie will be EOL in July, I think it's fine to just release cartographer on Xenial for Kinetic and blacklist it on Jessie.

move the patches to the package.xml to a distro specific patch folder

As I wrote above, we changed to use package.xml in upstream repositories, so may be we do not need to move to the distro specific patch folder.

I guess there was a misunderstanding here: the cartographer team created a new mailing list for dev only called "cartographer-owners" so that the "google-cartographer" mailing list doesnt get spammed by buildfarm emails (see discussion here). So we need to patch the package.xml accordingly. I would also prefer having the person making the releases receiving emails directly in case of failures, rather than monitoring the cartographer-owners mailing list. That's why I was asking who should be listed as a maintainer in the package.xml

@mikaelarguedas
Copy link
Member Author

@k-okada friendly ping
Waiting for your feedback about who should have access to the release repositories and be listed as maintainers in the released packages for kinetic/lunar.

FYI: cartographer 0.3.0 has been released in ROS melodic, using a package.xml with the fixed rosdep key for ceres as well as the new maintainer tags

@k-okada
Copy link
Contributor

k-okada commented Mar 28, 2018

@mikaelarguedas Sorry for late, please include @k-okada and @7675t (my colleague at tork) to the release repositories and maintainers.

I have created two pull requests (ros-gbp/cartographer-release#2, ros-gbp/cartographer_ros-release#1) so that you can check.

@mikaelarguedas
Copy link
Member Author

Thanks @k-okada !
I added @7675t as a collaborator on the release repositories and gave feedback directly on the PRs

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