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

Deb dependency does not specify version #11

Closed
gaschler opened this issue Jun 7, 2018 · 12 comments
Closed

Deb dependency does not specify version #11

gaschler opened this issue Jun 7, 2018 · 12 comments

Comments

@gaschler
Copy link

gaschler commented Jun 7, 2018

The deb package ros-melodic-cartographer-ros does not depend on a specific version of ros-melodic-cartographer, but it will only run with the same major version.
This is also the case for the packages ros-melodic-cartographer-ros-msgs and ros-melodic-cartographer-rviz.
(See control file in http://repositories.ros.org/ubuntu/testing/pool/main/r/ros-melodic-cartographer-ros/ros-melodic-cartographer-ros_1.0.0-0bionic.debian.tar.xz)

I am not sure if version dependencies are commonly specified in ROS packages.
Of course, sudo apt-get upgrade will update all these packages to the latest version, and it will just work.

@mikaelarguedas
Copy link
Contributor

👍 this should be specified in the package.xml. You can see an example here https://github.com/ros/pluginlib/blob/b0299cfe88c0bbe742a859b9f38b364a9bd271fe/package.xml#L26

As we usually only release a compatible version (at least same major) in a ROS distribution, we dont always define it explicitly in the package.xml. In the case of cartographer we actualy bumped the version significantly since the first melodic release so it should be specified.

This is also the case for the packages ros-melodic-cartographer-ros-msgs and ros-melodic-cartographer-rviz.

This "shouldn't" be necessary as they are hosted in the same repository and that bloom (the ROS releasing utility) enforces all packages in the same source repository to have the same version number. Though it doesn't hurt to add it for people building parts from source and have mismatching versions of the dependencies already installed.

I opened cartographer-project/cartographer_ros#892 to address this

@k-okada
Copy link
Collaborator

k-okada commented Jun 7, 2018 via email

@mikaelarguedas
Copy link
Contributor

Thanks @k-okada for these pointers!

The assumption "the buildfarm rebuilds downstream when a new version of a dependency comes out" that is true today, but may change in the future once the buildfarm implements and leverages the "compatibility" feature of package format 3: ros-infrastructure/rep#138. At that point it may be worth bringing it up to the bloom/ros_buildfarm maintainers 👍

@k-okada
Copy link
Collaborator

k-okada commented Jun 7, 2018

May be I misunderstand something, but @gaschler 's original problem is ..

  1. suppose if we had machine which already installed old version of cartographer and cartographer-ros,
  2. and noticed that there is a newer version released,
  3. then try to install apt-get install cartographer-ros ,
  4. but it does not bring recent cartographer package . So if we run cartographer-ros, it is failing.

The only solution is to run apt-get install cartographer manually if you know the package dependencies (usually not), or run apt-get dist-upgrade (it may depend on users, but I usually do not want to run dist-upgrade, that sometimes break my working environment, for example nvidia drivers....)

@mikaelarguedas
Copy link
Contributor

The only solution is to run apt-get install cartographer manually if you know the package dependencies (usually not), or run apt-get dist-upgrade (it may depend on users, but I usually do not want to run dist-upgrade, that sometimes break my working environment, for example nvidia drivers....)

In that case I would suggest to use apt-get upgrade cartographer-ros that will upgrade cartographer-ros (or install it if not already installed) and upgrade its dependencies (but not upgrade the rest of the system like nvidia drivers).

It just seem that the explicit dependency at bloom time may go against the compatibility approach.
For example, If package foo depends on bar and foo can work with any bar >=1.0.0:

  • As a user if I have bar 1.0.0 installed, I should be able to install foo without having to upgrade bar to 1.0.X
    It seems to me that if we were to explicitly declare the dependency at bloom time, as a user I wouldn't be able to have this configuration.

I thinks that's where the compatibility approach comes in. If I say (as the maintainer of bar) that bar 1.0.10 is "compatible" (aka same ABI/API, same CMake variables etc) with any bar since 1.0.0, foo should not be rebuilt and should work as long as I have bar >=1.0.0 on my system.
An if we release a new version of foo that doesnt change API and declare it needs bar >=1.0.0, it should still work with bar 1.0.0 even if the version of bar in the repo at the time we bloom foo is 1.0.5.
But with the explicit dependency suggestion, foo will now have bar 1.0.5 in its control file while it shouldnt be necessary and will force users to upgrade bar to use the new version of foo.

(let me now if this is unclear, this "simple" example gets hard to follow I am aware)

@k-okada
Copy link
Collaborator

k-okada commented Jun 7, 2018

In that case I would suggest to use apt-get upgrade cartographer-ros

That good to know, I usually do not use upgrade with argument. But

k-okada@p51s:~$ sudo apt-get upgrade ros-kinetic-cartographer-ros
Reading package lists... Done
Building dependency tree       
Reading state information... Done
Calculating upgrade... Done
The following NEW packages will be installed:
  ros-kinetic-cartographer-ros
The following packages will be upgraded:
  code dus google-chrome-stable grub-common grub-efi-amd64 grub-efi-amd64-bin
  grub-efi-amd64-signed grub2-common guidus ifupdown insync libldap-2.4-2 libldap-2.4-2:i386
  libldap2-dev liblouis-data liblouis9 libplymouth4 libpq-dev libpq5 linux-firmware mkusb
  mkusb-common mkusb-nox plymouth plymouth-label plymouth-theme-ubuntu-logo
  plymouth-theme-ubuntu-text python-catkin-pkg python-catkin-pkg-modules python3-louis
  ros-kinetic-cartographer skypeforlinux snapd teamviewer ubuntu-core-launcher usb-pack-efi
36 upgraded, 1 newly installed, 0 to remove and 0 not upgraded.
Need to get 338 MB/339 MB of archives.
After this operation, 21.8 MB disk space will be freed.
Do you want to continue? [Y/n] ^C

Maybe I'm too nervous, but this really threaten me....

It just seem that the explicit dependency at bloom time may go against the compatibility approach.

Thank you for clear explanation, I did not know about compatibility approach and that seems useful.

But with the explicit dependency suggestion, foo will now have bar 1.0.5 in its control file while it shouldnt be necessary and will force users to upgrade bar to use the new version of foo.

In this case, how about 'bar >= 1.0.0' in its control file? That would not force unnecessary upgrade bar

@mikaelarguedas
Copy link
Contributor

That good to know, I usually do not use upgrade with argument. But

Sorry that was an oversight on my part because in the environment I was using all the out of date packages were dependencies of the one I wanted to install. This seems to upgrade everything and installs the new package on top... so this doesn't buy anything compare to an apt upgrade.

In this case, how about 'bar >= 1.0.0' in its control file? That would not force unnecessary upgrade bar

Yes I think that would be the desired outcome 👍

@k-okada
Copy link
Collaborator

k-okada commented Jun 8, 2018 via email

@mikaelarguedas
Copy link
Contributor

I think write "bar >= 1.0.0" in control file,
based on version_gte of packagae.xml might be useful, that was my
original proposal at bloom.

I believe bloom already does it for package.xml that specify version_gte attributes for dependencies.
e.g. roscpp has the following runtime dependency:
<run_depend version_gte="0.3.17">roscpp_traits</run_depend>

And the corresponding Depends in the control file:

Depends: libboost-chrono1.65.1, libboost-filesystem1.65.1, libboost-system1.65.1, libboost-thread1.65.1, libc6 (>= 2.14), libgcc1 (>= 1:3.0), libstdc++6 (>= 5.2), ros-melodic-cpp-common (>= 0.3.17), ros-melodic-message-runtime, ros-melodic-rosconsole, ros-melodic-roscpp-serialization, ros-melodic-roscpp-traits (>= 0.3.17), ros-melodic-rosgraph-msgs (>= 1.10.3), ros-melodic-rostime (>= 0.6.4), ros-melodic-std-msgs, ros-melodic-xmlrpcpp

This includes the version requirement: ros-melodic-roscpp-traits (>= 0.3.17)

Not sure when it was added to bloom though..

@k-okada
Copy link
Collaborator

k-okada commented Jun 11, 2018

Oh, I did not know I have to add to the run_depends, that's good to know. Thanks!!
I that case, we'd better to add version_gte to https://github.com/ros-gbp/cartographer_ros-release/blob/master/kinetic/cartographer_ros/package.xml#L49 ?

@tfoote
Copy link

tfoote commented Jun 11, 2018

The attribute version_gte has been implemented since package.xml format 2 was implemented. http://www.ros.org/reps/rep-0140.html#id14

All dependency tags will accept that attribute so you can just add it to the <depend> tag linked. A depend tag implies <build_depend>, <build_export_depend> and <exec_depend>

@mikaelarguedas
Copy link
Contributor

As I believe this has been addressed in cartographer-project/cartographer_ros#892 I'm closing this issue. Feel free to comment here if it's not the case and we can reopen it

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

No branches or pull requests

4 participants