Add support for QNX 7.1 build #3360

chachoi · 2023-03-13T19:15:40Z

Description

This PR adds necessary changes which will allow Fast-DDS to build with QNX 7.1:

I added QNX related build files: Makefile, nto folder, common.mk, and qnx.nto.toolchain.cmake.
I added a qnx_patches folder with patches for external libraries i.e. asio, fastcdr, and foonathan_memory_vendor.
I added QNX specific changes to CMakeLists. All of the changes in CMakeLists are wrapped in if(QNX) or if(NOT QNX) so that other builds are not affected. Some google tests are disabled because QNX 7.1 cannot compile the WillOnce function.
I changed the shared memory directory from /dev/shm to /dev/shmem for QNX. This change is wrapped in #ifdef QNXNTO.

Build instructions for QNX 7.1:

# Clone the repo
git clone git@github.com:chachoi/Fast-DDS.git && cd Fast-DDS

# Initialize external repos
vcs import < fastrtps.repos
git submodule update --init ./thirdparty/asio/ ./thirdparty/fastcdr ./thirdparty/tinyxml2/

# Apply QNX patches to external libraries
cd ./thirdparty/asio
git apply ../../build_qnx/qnx_patches/asio_qnx.patch
cd ../fastcdr
git apply ../../build_qnx/qnx_patches/fastcdr_qnx.patch
cd ../tinyxml2
unix2dos ../../build_qnx/qnx_patches/tinyxml2_qnx.patch
git apply ../../build_qnx/qnx_patches/tinyxml2_qnx.patch
cd ../../

# Clone googletest
git clone git@github.com:google/googletest.git
cd googletest
git checkout v1.13.0
git apply ../build_qnx/qnx_patches/googletest_qnx.patch
cd -

# Source the QNX environment script
source <path-to-qnxsdp-env.sh>/qnxsdp-env.sh

# Build Fast-DDS and its dependencies
cd build_qnx && make -j 4

Libraries will be installed to ${QNX_TARGET}/${CPUVARDIR}/usr/lib.

To build examples and tests, change -DEPROSIMA_BUILD_TESTS=OFF and -DCOMPILE_EXAMPLES=OFF in common.mk to -DEPROSIMA_BUILD_TESTS=ON and -DCOMPILE_EXAMPLES=ON .

Examples will be installed to ${QNX_TARGET}/${CPUVARDIR}/usr/bin/Fast-DDS_examples.
Tests will be installed to ${QNX_TARGET}/${CPUVARDIR}/usr/bin/Fast-DDS_test.

Contributor Checklist

Commit messages follow the project guidelines.
The code follows the style guidelines of this project.
N/A Tests that thoroughly check the new feature have been added/Regression tests checking the bug and its fix have been added; the added tests pass locally
N/A Any new/modified methods have been properly documented using Doxygen.
N/A Changes are ABI compatible.
N/A Changes are API compatible.
New feature has been added to the versions.md file (if applicable).
N/A New feature has been documented/Current behavior is correctly described in the documentation.
N/A Applicable backports have been included in the description.

Reviewer Checklist

The PR has a milestone assigned.
Check contributor checklist is correct.
Check CI results: changes do not issue any warning.
Check CI results: failing tests are unrelated with the changes.

jsan-rt

First of all, thanks for all the work done on putting this PR together.

Other than the suggestions and questions already in the relevant files I have some concerns regarding the extensive Makefile usage. Is it required? On the common.mk file it seems like it ends up building all components in the standard CMake way. Also, a user that downloads the repository may be confused by a top level Makefile that fails due to a missing file (since recurse.mk is a QNX feature).

jsan-rt · 2023-03-22T09:35:31Z

.gitignore

+build_fast-dds/
+build_fastcdr/
+build_foonathan_memory/
+build_googletest/
+build_tinyxml2/


Is it possible to have this object folders inside the standard build folder?

jsan-rt · 2023-03-22T09:46:33Z

CMakeLists.txt

@@ -549,6 +567,45 @@ if(INSTALL_TOOLS)
      )
 endif()

+if(COMPILE_EXAMPLES AND QNX)


I think it is better to keep this in line with the rest of the logic. COMPILE_EXAMPLES builds, INSTALL_EXAMPLES moves them to the proper place.

Probably is also a good idea to nest it inside the already existing INSTALL_EXAMPLES check.

Sounds good. Done.

jsan-rt · 2023-03-22T10:44:52Z

CMakeLists.txt

+if(EPROSIMA_BUILD_TESTS AND QNX)
+    install(DIRECTORY ${PROJECT_BINARY_DIR}/test/
+      DESTINATION bin/Fast-DDS_test
+      PATTERN "*.cmake" EXCLUDE
+      PATTERN "*.d" EXCLUDE
+      PATTERN "*.dir" EXCLUDE
+      PATTERN "*.internal" EXCLUDE
+      PATTERN "*.make" EXCLUDE
+      PATTERN "*.marks" EXCLUDE
+      PATTERN "*.o" EXCLUDE
+      PATTERN "*.ts" EXCLUDE
+      PATTERN "*.txt" EXCLUDE
+      PATTERN "CMakeFiles" EXCLUDE
+      PATTERN "Makefile" EXCLUDE
+      PATTERN "cmake" EXCLUDE
+      )
+    install(DIRECTORY ${PROJECT_SOURCE_DIR}/test/certs
+      DESTINATION bin/Fast-DDS_test
+    )
+endif()


Typically ctest runs the test binaries from the build directory. Why do you need to install them explicitly?

Also, this should be moved to the CMakeLists.txt inside the test subfolder.

I install them directly because the test binaries compiled for QNX can only be run on QNX targets, not on your Ubuntu build machine.

I moved it to the CMakeLists.txt inside the test subfolder.

jsan-rt · 2023-03-22T11:49:12Z

versions.md

New commits on master branch are causing conflicts here. Please rebase.

jsan-rt · 2023-03-22T11:56:46Z

qnx_patches/foonathan_memory_qnx.patch

This patch should be its own PR on the foonathan_memory_vendor repository since we are already doing some patching there.

I created a PR on the foonathan_memory_vendor.

chachoi · 2023-03-22T17:20:32Z

First of all, thanks for all the work done on putting this PR together.

Other than the suggestions and questions already in the relevant files I have some concerns regarding the extensive Makefile usage. Is it required? On the common.mk file it seems like it ends up building all components in the standard CMake way. Also, a user that downloads the repository may be confused by a top level Makefile that fails due to a missing file (since recurse.mk is a QNX feature).

Thank you for the feedback @jsantiago-eProsima.

Yes, the Makefiles are required because that is how QNX builds work. I moved all QNX related build files under build_qnx so that users would not be confused.

jsan-rt

Changes look great, thank you. Just a small nitpick that I didn't catch on the first pass left.

Also, it would be really helpful if you could make a Pull Request to Fast DDS' documentation repository detailing the build procedure so it can be referenced there instead of in a Pull Request.

jsan-rt · 2023-03-23T09:35:07Z

test/unittest/rtps/writer/CMakeLists.txt

-    GTest::gmock
-    ${CMAKE_DL_LIBS})
-add_gtest(RTPSWriterTests SOURCES ${RTPSWRITERTESTS_SOURCE})
+if(NOT QNX)


Just a small nitpick. Since we no longer need to build the RTPSWriterTests due to the WillOnce issue, better to move this before the set(RTPSWRITERTESTS_SOURCE RTPSWriterTests.cpp)

chachoi · 2023-03-23T17:04:22Z

Changes look great, thank you. Just a small nitpick that I didn't catch on the first pass left.

Also, it would be really helpful if you could make a Pull Request to Fast DDS' documentation repository detailing the build procedure so it can be referenced there instead of in a Pull Request.

Thanks @jsantiago-eProsima .

That sounds good. I will start to work on documenting the build procedure. Once I am done, I will submit a PR on the documentation repository.

chachoi · 2023-03-24T21:11:32Z

Hello @jsantiago-eProsima,

I opened a PR which adds instructions for building.

Thanks.

Signed-off-by: James Choi <chachoi@blackberry.com>

jsan-rt · 2023-03-27T22:00:13Z

@richiprosima please test this

jsan-rt

LGTM with green CI

jsan-rt · 2023-03-28T05:09:43Z

Failing test is unrelated.

chachoi force-pushed the master branch from b71d02c to 618d55e Compare March 13, 2023 19:23

JLBuenoLopez added this to the v2.10.0 milestone Mar 16, 2023

chachoi force-pushed the master branch 2 times, most recently from d5f9bda to c58675e Compare March 16, 2023 15:45

jsan-rt suggested changes Mar 22, 2023

View reviewed changes

chachoi force-pushed the master branch 6 times, most recently from 8cad29e to 8cf397f Compare March 22, 2023 15:36

chachoi force-pushed the master branch from 8cf397f to 1973e54 Compare March 22, 2023 17:47

chachoi requested a review from jsan-rt March 22, 2023 17:51

chachoi force-pushed the master branch from 1973e54 to 664ec97 Compare March 22, 2023 17:56

jsan-rt suggested changes Mar 23, 2023

View reviewed changes

chachoi force-pushed the master branch 2 times, most recently from b6a8835 to 38c4ca6 Compare March 23, 2023 14:24

jsan-rt added the ci-pending PR which CI is running label Mar 23, 2023

chachoi force-pushed the master branch 5 times, most recently from d2e86fc to c49f53d Compare March 24, 2023 14:30

chachoi mentioned this pull request Mar 24, 2023

Add instructions for building and running Fast-DDS on QNX 7.1 eProsima/Fast-DDS-docs#470

Merged

chachoi requested a review from jsan-rt March 24, 2023 21:11

QNX 7.1 build

e565dc9

Signed-off-by: James Choi <chachoi@blackberry.com>

chachoi force-pushed the master branch from c49f53d to e565dc9 Compare March 27, 2023 15:36

jsan-rt approved these changes Mar 27, 2023

View reviewed changes

jsan-rt added ready-to-merge Ready to be merged. CI and changes have been reviewed and approved. and removed ci-pending PR which CI is running labels Mar 28, 2023

MiguelCompany merged commit 8d2e7dc into eProsima:master Mar 28, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for QNX 7.1 build #3360

Add support for QNX 7.1 build #3360

chachoi commented Mar 13, 2023 •

edited by MiguelCompany

Loading

jsan-rt left a comment

jsan-rt Mar 22, 2023

chachoi Mar 22, 2023

jsan-rt Mar 22, 2023

chachoi Mar 22, 2023

jsan-rt Mar 22, 2023 •

edited

Loading

chachoi Mar 22, 2023

jsan-rt Mar 22, 2023

chachoi Mar 22, 2023

jsan-rt Mar 22, 2023

chachoi Mar 22, 2023

chachoi commented Mar 22, 2023

jsan-rt left a comment

jsan-rt Mar 23, 2023

chachoi commented Mar 23, 2023

chachoi commented Mar 24, 2023

jsan-rt commented Mar 27, 2023

jsan-rt left a comment

jsan-rt commented Mar 28, 2023

Add support for QNX 7.1 build #3360

Add support for QNX 7.1 build #3360

Conversation

chachoi commented Mar 13, 2023 • edited by MiguelCompany Loading

Description

Contributor Checklist

Reviewer Checklist

jsan-rt left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jsan-rt Mar 22, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chachoi commented Mar 22, 2023

jsan-rt left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chachoi commented Mar 23, 2023

chachoi commented Mar 24, 2023

jsan-rt commented Mar 27, 2023

jsan-rt left a comment

Choose a reason for hiding this comment

jsan-rt commented Mar 28, 2023

chachoi commented Mar 13, 2023 •

edited by MiguelCompany

Loading

jsan-rt Mar 22, 2023 •

edited

Loading