Enable Trusty support for lunar-devel #1236
Conversation
|
This commit seems to have added the new function Beside that ROS Lunar is specifically not targeting Ubuntu Trusty. Many other packages will have similar problems. Therefore I am not convinced that it makes sense to add this patch to the code base. |
|
There wasn't a tag; the version number was incremented in a commit without getting tagged. My original commit (and the version we had been using internally) was set to use version 1.7.0 as the version in which the switch occurred, but in verifying that I had the version number right before PR-ing it, I managed to make it wrong. Given that Lunar is not targeting Trusty and that this check would ensure the behavior doesn't change for anyone running on supported platforms, is there danger in merging (once I correct the version number, that is)? We have been using this version of In any case, it's not a big deal if you'd prefer not to merge it. We're just trying to minimize our differences with upstream. |
That is why I am a little hesitant. Basically every change has the potential for regressions. With the recent Kinetic backports we noticed that sadly multiple times that is probably why I am currently extra cautious. I guess this can be merged since it shouldn't effect the targeted platforms. One thing to change beside the exact version is the name of the macro. |
|
I'm supportive of merging. Obviously we can't support old platforms forever, but when it's such a small change, it's certainly nice to provide that migration path forward. I know at Clearpath we were using the Kinetic+ versions of many core repos long before we actually switched our underlying OS to 16.04, and it sure was nice being able to stage the rollout like that. |
|
Sounds good. Will fix. |
utilities/roslz4/src/lz4s.c
Outdated
| // Note that the later versions of LZ4 contain a different macro that does this version math, but the earlier | ||
| // versions lack it. | ||
| #define LZ4_VERSION (LZ4_VERSION_MAJOR * 100 * 100 + LZ4_VERSION_MINOR * 100 + LZ4_VERSION_RELEASE) | ||
| #if LZ4_VERSION >= 10700 |
There was a problem hiding this comment.
That tag identifies itself as version 1.7.0 which is also the same for the earlier tag
r129.
As mentioned before since an older release (r129) identifies itself with this version too I don't think it can be used for the check.
There was a problem hiding this comment.
Oh, apologies, I misunderstood your issue. I will get the first post-1.7.0 tag, which I believe is 1.7.3.
There was a problem hiding this comment.
r131 identifies as 1.7.1. Would that be more precise?
|
Thank you for iterating on this. |
This PR enables us to build the latest version of ros_comm on Trusty. The LZ4 API changed at version 1.7.2 to use
LZ4_compress_defaultinstead ofLZ4_compress_limitedOutput, and this PR defines a version-dependent macro that allows us to build with the older LZ4 API.