Update REP3 for Melodic Morenia #139
Conversation
rep-0003.rst
Outdated
| - C++11 | ||
| - Python 2.7 | ||
|
|
||
| - Python 3.6 not required, but testing against it is recommended |
There was a problem hiding this comment.
Some targeted platforms ship Python 3.5 (e.g. debian stretch). So this could be updated to Python >=3.5.
There was a problem hiding this comment.
Oh, thanks. I forgot to double-check that on Stretch. I'll change it to your suggestion.
There was a problem hiding this comment.
Maybe the Python 3 line could be moved into the table below?
There was a problem hiding this comment.
I think the reason it is here and not in the table is because it is a language, and not a library/tool. But if we think moving it into the below table, or a separate table for the languages make sense, I could change to that instead.
|
|
||
| Build System Support: | ||
|
|
||
| - Same as Indigo |
There was a problem hiding this comment.
This may change depending on the decision of supporting the ament build system or not in ROS1
rep-0003.rst
Outdated
| +---------+---------------+----------------+----------------+-----------+ | ||
| | Ogre | 1.9 | 1.9! | 1.9 | 1.9! | | ||
| +---------+---------------+----------------+----------------+-----------+ | ||
| | OpenCV | 3.3* | 3.3* | 3.3* | 3.3* | |
There was a problem hiding this comment.
Last year we forward ported the version of the last released ROS distro to avoid making assumption on what the community will release. I'd suggest putting 3.2 here and see if @vrabaud and/or other in the community releasing OpenCV in Fedora manifest interest in releasing 3.3.
There was a problem hiding this comment.
I was basing the 3.3 version mostly on this discussion: https://discourse.ros.org/t/opencv-3-3/2674/6 . I guess we'll let @vrabaud comment and then we can change it/fix it accordingly.
There was a problem hiding this comment.
3.2 is from Dec 2016. It would be sad to be more than one year behind. 3.3 actually has very serious support for deep learning (not in contrib anymore) and I guess robotics will use more and more of that. 3.4 will be out in December 2017
I hate to say this as it will make me the maintainer for a few more years but we should go with our own 3.4 binaries ....
There was a problem hiding this comment.
@vrabaud Thanks for the response. I'll change the PR to say 3.4 for now based on your recommendation. If things change, we can change this later on.
rep-0003.rst
Outdated
| +---------+---------------+----------------+----------------+-----------+ | ||
| | CMake | 3.9.1 | 3.9.1! | 3.7.2 | 3.9.3! | | ||
| +---------+---------------+----------------+----------------+-----------+ | ||
| | Gazebo | 7.5.0 | 7.5.0! | 7.3.1 | 8.1.1! | |
There was a problem hiding this comment.
@j-rivero do you think Gazebo 8 9 will make it into Ubuntu 18.04 ? If not, will gazebo7 be maintained until 2023 ?
There was a problem hiding this comment.
@mikaelarguedas do you mean to ask about Gazebo 9.0? 8.0 isn't an LTS afaik
There was a problem hiding this comment.
Indeed I meant Gazebo 9 😖, thanks @IanTheEngineer for catching it. I'll edit the previous comment accordingly
There was a problem hiding this comment.
Gazebo 9 has a big number of useful functionality compared to 7. It'd be really sad to stay with 7 in Melodic. But I know it depends on the development status and decision about Gazebo 9 target platforms (which I couldn't find).
There was a problem hiding this comment.
@j-rivero Gentle ping, any thoughts on the versions of Gazebo here, and what we can expect will be supported for the life of Melodic?
There was a problem hiding this comment.
We are working for getting gazebo9 into Ubuntu 18.04 but I can not say yes or no at this moment since there is a good bunch of packages that need to go through the Debian review process, which takes an unpredictable amount of time. The different supporting periods are listed in the gazebo webpage.
rep-0003.rst
Outdated
| Required Support for: | ||
|
|
||
| - Ubuntu Artful (17.10) | ||
| - Ubuntu B (18.04) |
There was a problem hiding this comment.
Ha, nice :). I'll update the documentation accordingly.
|
Nice ! I guess it'll pretty suitable for robotics applications :)
…On Tue, Oct 24, 2017 at 12:18 PM, Dirk Thomas ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In rep-0003.rst
<#139 (comment)>
:
> @@ -281,6 +281,68 @@ Build System Support:
- Same as Indigo
+Melodic Morenia (May 2018 - May 2023)
+--------------------------------------
+Required Support for:
+
+- Ubuntu Artful (17.10)
+- Ubuntu B (18.04)
Bionic Beaver
<http://news.softpedia.com/news/ubuntu-18-04-lts-dubbed-as-the-bionic-beaver-launches-april-26-2018-518186.shtml>
it is.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#139 (review)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AGVh8A-RzU9EIcTDlIK7nJDFIlTeLUhZks5svjgAgaJpZM4QDQtv>
.
|
rep-0003.rst
Outdated
|
|
||
| Targeted Languages: | ||
|
|
||
| - C++11 |
There was a problem hiding this comment.
Should we now change the motivation and/or recommendation about using C++11 in desktop-full packages? Since we have coverage on support, is there harm in starting to use C++11 features in desktop-full?
There was a problem hiding this comment.
Sorry, I'm not really familiar with the history. Is the above note saying that C++11 is targeted not enough? Is there an exception for desktop-full?
There was a problem hiding this comment.
This is saying that all packages should be able to compile with the C++11 flag.
There is a C++ section below that covers use of modern C++ in APIs. I think that's what @wjwwood is referring to.
There are a couple related threads on discourse [1] [2]
There was a problem hiding this comment.
Thanks for the pointers. Now that C++11 (or newer) is going to be the default standard of the compiler for all of the supported platforms, my feeling is that it is safe to allow C++11 features as part of the desktop-full API. That's not to say we will change old APIs, just that new ones are free to use it. Hopefully others from the community can chime in here and express their concerns or support for this change.
There was a problem hiding this comment.
Now that C++11 (or newer) is going to be the default standard of the compiler for all of the supported platforms, my feeling is that it is safe to allow C++11 features as part of the desktop-full API. That's not to say we will change old APIs, just that new ones are free to use it.
That is already exactly the case for Kinetic and Lunar afaik.
There was a problem hiding this comment.
Quoting https://gcc.gnu.org/gcc-6/changes.html
The default mode for C++ is now -std=gnu++14 instead of -std=gnu++98.
Debian Stretch has 6.3.0, Artful has g++ 7.2.0, all others newer
So adding -std=c++11 will be a "downgrade" from default compilation mode for all supported platforms. I vote for having full "permission" for C++11 or even better 14 in all public apis and dropping whatever boost etc is possible
There was a problem hiding this comment.
@simonschmeisser Thanks for the link, that is helpful. I also confirmed empirically that gcc 6+ defaults to having (at least) C++11 features. Given that all of Ubuntu Artful, Bionic, Debian Stretch, and Fedora 28 are going to have versions of gcc newer than 6, that's a pretty good start. One potentially complicating factor here is clang++ on macOS, which, as of High Sierra, prints a warning when using C++11 features without passing -std=c++11. It actually still succeeds compilation, but the warning might be annoying for the macOS-porting portions of the community. If we don't care too much about macOS, then I would suggest we go with option number 1 from @wjwwood's post (just use the compiler default). If we do care about the warnings on macOS, then I think we'll have to go with something like option 2. Thoughts?
There was a problem hiding this comment.
Actually, doing a bit more digging on macOS, it seems like if we have the core packages export a -Wno-c++11-extensions flag, then High Sierra won't complain, but will compile c++11. That suggests to me that we should do 1 (use the compiler defaults), with the additional caveat that the core packages would need to ensure that they export the right flag for downstream users not to complain on macOS. Any further ideas here?
There was a problem hiding this comment.
I actually think this might not be sufficient, because IIRC some include statements will not work without the -std=c++11 flag set, e.g. maybe #include <memory> or #include <functional>?
There was a problem hiding this comment.
Yeah, you are right about that, I just double checked. Without the -std=c++11 flag, most things in clang work, but it doesn't know how to parse functional stuff. So we'll definitely need to provide the flag for macOS to work.
From this, then, I'd suggest that we follow @wjwwood 's option 2, and provide a std=c++11 flag down in one of the core packages so basically everything inherits it. This may be a "downgrade" from the default for certain compilers (the newer gcc ones which have C++14 standard), but packages higher up in the stack can always override it. If everyone is OK with that idea, then I'll update the motivation section to be a lot clearer on this point, and say that we allow C++11 features to be used basically everywhere, including public APIs.
| Recommended Support for: | ||
|
|
||
| - Debian Stretch | ||
| - Fedora 28 |
There was a problem hiding this comment.
Should we also target fedora 27 ?
There was a problem hiding this comment.
I'd suggest not. With their 13 month support cycle. If we release in May, we'll only get ~6 months of support.
There was a problem hiding this comment.
That's accurate.
Following that logic, should we also drop artful from the list?
With the Ubuntu 9-month support cycle, if we release end of may we'll only get ~2 months of support.
There was a problem hiding this comment.
It's certainly something we could consider. Ubuntu being our main development distro is a little different though. Artful will likely be what we do any testing on in the intermediate time.
There was a problem hiding this comment.
I agree with @tfoote about not support F27; the lifecycle is too short. I would be in favor of dropping artful for the same reasons, but I'd like to get opinions from the community on that.
There was a problem hiding this comment.
I think historically the goal was to target all distros and not skip one, but it seems just unnecessary effort to support distros that we don't support for more that a few months. For Lunar we dropped Yakkety before even having the most used packages released. I would be in favor of dropping both F27 and Ubuntu 17.10
There was a problem hiding this comment.
+1 to skip artful, no one will use it anyway
There was a problem hiding this comment.
There is a standing document about the goals and the rational (see http://wiki.ros.org/Distributions/ReleasePolicy). Until that is revised and agreed upon we should stick to it which implies supporting Artful for the time frame it is available.
There was a problem hiding this comment.
We discussed this, and unless there is a strong reason to not support artful, we are going to support it to conform to the Release Policy. As @tfoote suggests above, most of our development will probably be on artful anyway.
rep-0003.rst
Outdated
| - C++11 | ||
| - Python 2.7 | ||
|
|
||
| - Python >= 3.5 not required, but testing against it is recommended |
There was a problem hiding this comment.
Since this is a soft testing requirement anyway, should we recommend Python 3.6? Python 3.6 was released in December 2016, and will be receiving security updates through December 2021.
There was a problem hiding this comment.
Yes, this REP started with 3.6 but given that one of the targeted platforms (Debian Stretch) ships python 3.5 we changed it to 3.5 and above (#139 (comment))
There was a problem hiding this comment.
Ah, fair enough. I hadn't gone through the collapsed comments :/ Thanks!
|
Would this be an appropriate time to update the "Rationale" section? |
rep-0003.rst
Outdated
| - C++11 | ||
| - Python 2.7 | ||
|
|
||
| - Python >= 3.5 not required, but testing against it is recommended |
There was a problem hiding this comment.
Ubuntu Bionic is aiming to have only Python 3.6 in the default install; perhaps testing on Python 3 should be required?
There was a problem hiding this comment.
The current plan is that ROS targets Python 2 so with the ROS Debian packages Python 2.7 would be installed. The fact that only Python 3.6 is present on a bare installation is not relevant here.
There was a problem hiding this comment.
While it would be nice to completely support Python 3, I don't think that is going to be a reasonable amount of effort in the timeframe we have for Melodic. So I agree with @dirk-thomas that we'll just rely on Python 2 being pulled in when doing package installations, and continue to "recommend" support for Python 3.
There was a problem hiding this comment.
In this topic we should consider what Ubuntu is doing with Python 2 support (see http://news.softpedia.com/news/ubuntu-devs-work-on-demoting-python-2-to-universe-repo-for-ubuntu-18-04-lts-518926.shtml). While we can still use it it imo increases the "pressure" to consider moving forward or at least come up with a plan for doing so.
rep-0003.rst
Outdated
| +---------+---------------+----------------+----------------+-----------+ | ||
| | Ogre | 1.9 | 1.9! | 1.9 | 1.9! | | ||
| +---------+---------------+----------------+----------------+-----------+ | ||
| | OpenCV | 3.4* | 3.4* | 3.4* | 3.4* | |
There was a problem hiding this comment.
Actually, just thought about something: having the system version would help with people that have NVidia versions of OpenCV right ? Also, something I've often heard is that the OpenCV dependency is too big as it is compiled with the GUI (Qt), the image/video decoders and that brings in a lot of dependency. Debian/Ubuntu has a proper split of OpenCV.
There was a problem hiding this comment.
As it currently stands, Ubuntu Artful has OpenCV 3.2, Debian Stretch has OpenCV 2.4.9, and Fedora 26 has OpenCV 3.2. Projecting forwards, Ubuntu Bionic will have OpenCV 3.2+, and Fedora 28 will have OpenCV 3.2+. This means that we could say that Melodic packages must target an OpenCV 3.2 "API", but also provide additional, newer packages for the distributions (which would be required on Debian Stretch). That way, users could choose whether they use the system versions of the packages, or the ones provided by ROS. Does that make sense? Are there bad pitfalls in an approach like that?
There was a problem hiding this comment.
@vrabaud Gentle ping, what do you think about my previous proposal?
There was a problem hiding this comment.
The ROS package is actually a bloom/catkin package. Not a .deb.
Therefore, people have to stick to the version given by ROS. We're already at 3.4 almost in Lunar. Getting back to 3.2 would be a downgrade for people.
There was a problem hiding this comment.
@vrabaud Thanks. I think we came to that conclusion as well, so we ended up sticking with 3.4 for Melodic.
Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
Based on feedback from Vincent Rabaud. Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
clalancette
force-pushed
the
rep3_melodic
branch
from
e790523 to
2adef16
Compare
|
I've updated the language here to clarify our stance on C++ and Python. Please take another look and suggest improvements to the wording. |
rep-0003.rst
Outdated
| starting with Melodic Morenia are highly encouraged to support both Python 2.7 | ||
| and Python 3.5. During the development of Melodic there will be work undertaken | ||
| to support rosdep keys for both Python 2 and Python 3 so ROS package developers | ||
| can more easily test with either version of Python. |
There was a problem hiding this comment.
I absolutely agree with the paragraph. Before this can be merged we need to make sure that we are actually committing to do this.
There was a problem hiding this comment.
👍
Not sure we should mention what part of the work will be done during Melodic release cycle, as the infrastructure work for python2 and 3 support will likely be more involved than just providing rosdep keys.
Also I wouldnt restrict to Python 3.5 here as most targeted platform will have 3.6
There was a problem hiding this comment.
Sure, I'll change the wording to "3.5 or greater".
rep-0003.rst
Outdated
| then remove the old APIs in a subsequent release. | ||
|
|
||
| ROS Melodic is also compiler-agnostic, so no use of compiler-specific features is allowed | ||
| without proper use of macros to allow use on other platforms. |
There was a problem hiding this comment.
Does this apply only to ROS Melodic ?
There was a problem hiding this comment.
No, but there is similar wording in the "Lunar and earlier" section below. I'll see about combining them so there is less duplication.
Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
Also change it so that it is more clear that there is work beyond rosdep keys needed to enable Python 3 support. Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
I'm not sure what this implies, does revise means change it for a lower version ? or that we may bump it in the future? |
Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
|
As discussed in chat, d665be4 is a small commit to put one sentence per line. No change in content. |
|
I'm going to merge this now. If there are any further comments, feel free to leave them here, and we can address them in a follow-up PR. Thanks for the feedback and discussion. |
|
@clalancette according to this doc, Melodic will target Gazebo 7.x.x. Is that the case, or will Melodic use Gazebo 9.x.x? |
@IanTheEngineer See #160, where we are updating this to Gazebo 9 :). |
|
aha! Missed that PR. Thanks :) |
The rendered version of this PR can be seen here: https://github.com/ros-infrastructure/rep/blob/rep3_melodic/rep-0003.rst#melodic-morenia-may-2018---may-2023
Signed-off-by: Chris Lalancette clalancette@openrobotics.org