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

mongodb_store: 0.4.2-0 in 'kinetic/distribution.yaml' [bloom] #18103

Merged
merged 1 commit into from
Jun 4, 2018
Merged

mongodb_store: 0.4.2-0 in 'kinetic/distribution.yaml' [bloom] #18103

merged 1 commit into from
Jun 4, 2018

Conversation

hawesie
Copy link
Contributor

@hawesie hawesie commented Jun 4, 2018

Increasing version of package(s) in repository mongodb_store to 0.4.2-0:

libmongocxx_ros

* instead of lsb-release read /etc/os-version from CMake to find OS version
* Contributors: Ferenc Balint-Benczedi

mongodb_log

* instead of lsb-release read /etc/os-version from CMake to find OS version
* Contributors: Ferenc Balint-Benczedi

mongodb_store

* instead of lsb-release read /etc/os-version from CMake to find OS version
* Contributors: Ferenc Balint-Benczedi

mongodb_store_msgs

  • No changes

@mikaelarguedas
Copy link
Member

@hawesie Heads-up: The change from lsb_release to /etc/os-release makes the code not OS independant anymore. For example Debian doesn't provide a VERSION_CODENAME in /etc/os-release.

@hawesie
Copy link
Contributor Author

hawesie commented Jun 4, 2018

@machielbruinink we've been discussing this over in strands-project/mongodb_store#222 (and linked PRs) led by @bbferka. As far as we know we only need this for the build farms since on other platforms else we expect to have ROS_DISTRO available.

@bbferka
Copy link
Contributor

bbferka commented Jun 4, 2018

@mikaelarguedas you are right. I just wanted to see if this check would work on the build farm. Looking at the logs this check solved the previous issue and libmongocxx_ros now build with C++11 support on kinetic and xenial.

I will modify the CMake to check also for Debian. I am aware of the different content of /etc/os-releasefor Debian Jessie, Stretch and Sid and I believe I can distinguish based on the ID field between Ubuntu and Debian, and then figure out which version it is based on another entry. As @hawesie mentioned it, this was only an issue on the build farms since nor lsb_release or ROS_DISTRO are available during the creation of the binary.

@mikaelarguedas
Copy link
Member

Sounds good 👍 , I just wanted to point out the difference in behavior in case you were not aware of it (that bit me recently on another project so I figured it may be useful to others :)).

@dirk-thomas
Copy link
Member

this was only an issue on the build farms since nor lsb_release or ROS_DISTRO are available during the creation of the binary.

If your package would depend on the ros_environment package I would expect the ROS_DISTRO environment variable to be set when your package is being built.

@bbferka
Copy link
Contributor

bbferka commented Jun 5, 2018

@dirk-thomas hmm, good idea, I did not know about the ros_environment package. Is there any way of trying this out without releasing? The current fix works and I would not like to mess that up. Since this problem only occurred on the build farm it is hard for me to debug it properly

@dirk-thomas
Copy link
Member

See my comment strands-project/mongodb_store#224 (comment) why it failed for one of the packages and why it would be fixed with the extra dependency.

You could setup a clean Docker container and only build libmongocxx_ros in it (without the other packages of the same repo and only with the dependencies of that package installed). That will build without c++11 since the environment won't have ROS_DISTRO set. Once you install ros_environment and source the setup again it should set ROS_DISTRO which will make your package build with c++11.

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.

4 participants